Skip to content

Adds Anchor tests #2971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 10, 2020
Merged

Adds Anchor tests #2971

merged 5 commits into from
Nov 10, 2020

Conversation

oke-aditya
Copy link
Contributor

@oke-aditya oke-aditya commented Nov 7, 2020

Followup PR. Adds tests for #2960

Somehow I was not able to run tests locally. I'm unsure if this is fixed quite well.

I'm not sure what and how tests can or should be for this, somehow it simply creates an empty Anchor and no ValueErorr is generated.

@zhiqwang
Copy link
Contributor

zhiqwang commented Nov 8, 2020

Hi, I think that something like this

vision/test/test_onnx.py

Lines 197 to 200 in 052edce

def _init_test_rpn(self):
anchor_sizes = ((32,), (64,), (128,), (256,), (512,))
aspect_ratios = ((0.5, 1.0, 2.0),) * len(anchor_sizes)
rpn_anchor_generator = AnchorGenerator(anchor_sizes, aspect_ratios)

and

vision/test/test_onnx.py

Lines 268 to 284 in 052edce

class RPNModule(torch.nn.Module):
def __init__(self_module):
super(RPNModule, self_module).__init__()
self_module.rpn = self._init_test_rpn()
def forward(self_module, images, features):
images = ImageList(images, [i.shape[-2:] for i in images])
return self_module.rpn(images, features)
images = torch.rand(2, 3, 150, 150)
features = self.get_features(images)
images2 = torch.rand(2, 3, 80, 80)
test_features = self.get_features(images2)
model = RPNModule()
model.eval()
model(images, features)

could be added to the unittest?

@fmassa
Copy link
Member

fmassa commented Nov 9, 2020

Hi

Thanks for the PR!

@oke-aditya Test failures are related, can you have a look?

@zhiqwang yes, we can improve testing of AnchorGenerator to check for correctness as well, and this is just a first PR to start this effort. I think tests which would be useful would be tests for a few known values of a very small feature map size, so that we know the ground-truth.
Would you be willing to help with adding more tests to AnchorGenerator?

@oke-aditya
Copy link
Contributor Author

Yes the failures are related. The reason is I'm not able to generate ValueError which I created with last PR.
Somehow I am not able to figure out why a simple tuple doesn't raise this.
Can someone help me ? 😓

@fmassa
Copy link
Member

fmassa commented Nov 9, 2020

@oke-aditya might be because of

if not isinstance(sizes[0], (list, tuple)):
# TODO change this
sizes = tuple((s,) for s in sizes)
if not isinstance(aspect_ratios[0], (list, tuple)):
aspect_ratios = (aspect_ratios,) * len(sizes)
, which was there to help on those cases.

I think you should add instead a tuple of tuple, but with different sizes

@zhiqwang
Copy link
Contributor

zhiqwang commented Nov 9, 2020

@zhiqwang yes, we can improve testing of AnchorGenerator to check for correctness as well, and this is just a first PR to start this effort. I think tests which would be useful would be tests for a few known values of a very small feature map size, so that we know the ground-truth.
Would you be willing to help with adding more tests to AnchorGenerator?

Hi @fmassa , Yep, It's my pleasure, I will trace this current PR, and check what can I contribute to following up this PR.

@oke-aditya
Copy link
Contributor Author

oke-aditya commented Nov 9, 2020

I'm really unsure what code would generate the error. I have tried various combinations locally but no success yet.

Following is what I tried to generate the ValueError but somehow it isn't getting generated

I think there is some confusion.

assert len(sizes) == len(aspect_ratios)

This assert actually checks if sizes and aspect ratios match.

This can be easily triggered.
E.g. case

incorrect_sizes = ((128, 258, 512), (128, 32))
incorrect_aspects = ((0.5, 1.0, 2.0),),

But this

if not (len(grid_sizes) == len(strides) == len(cell_anchors)):

is a call to very internal code in the forward method which makes it quite hard to get such test cases.

Some test cases that didn't work to fix the ValueError

incorrect_sizes = ((128), (258, 32), )
incorrect_aspects = ((0.5, 2.0), (3.0))

incorrect_sizes = ((128, 128, 128), (258, 32), )
incorrect_aspects = ((0.5, 2.0), (3.0))

Passing only tuple too didn't work. I think that it needs further digging into code.

@fmassa
Copy link
Member

fmassa commented Nov 10, 2020

@oke-aditya you need to set the features to have a different (wrong?) number of feature maps

@oke-aditya
Copy link
Contributor Author

Ah, now I get it. I have fixed the test. 😄

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fmassa fmassa merged commit 481ef51 into pytorch:master Nov 10, 2020
@oke-aditya oke-aditya deleted the anchor_tests branch November 10, 2020 11:26
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* tries adding anchor tests

* fixes lint

* tries fixing

* tries one more time

* fixes the test
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
* tries adding anchor tests

* fixes lint

* tries fixing

* tries one more time

* fixes the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants