Skip to content
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

img_size attribute for TensorPoint is not updated properly #2799

Merged

Conversation

IRailean
Copy link
Contributor

@IRailean IRailean commented Sep 15, 2020

Solving the issue reported in #2786
As resized image size will be saved in PointScaler self.sz variable, and 'img_size' attribute for TensorPoint is not updated properly -> prioritize self.sz over sz in PointScaler _get_sz function.

Copy link
Member

@jph00 jph00 left a comment

Choose a reason for hiding this comment

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

Could you please add a test in the notebook that previously would have failed, but this change makes pass?

@IRailean
Copy link
Contributor Author

I have updated the last test from 09_vision.augment.ipynb with a new transform Resize(256) - it is clearly seen that previously bboxes are not covering the objects.
Note: With the old implementation everything works fine if run only this notebook. If run after installing fastai (pip | from git | using local setup.py), bboxes are not shown correctly.

@IRailean IRailean requested a review from jph00 September 16, 2020 13:43
@jph00
Copy link
Member

jph00 commented Sep 16, 2020

With the old implementation everything works fine if run only this notebook. If run after installing fastai (pip | from git | using local setup.py), bboxes are not shown correctly.

Oh interesting - can you tell me more about this? Is it a fastcore change or fastai change which broke things? Do you know what version the breakage occurred from?

@marii-moe
Copy link
Collaborator

marii-moe commented Sep 17, 2020

I think I was the one who originally stated that a previous version existed that did not have the issue. I tried to track down the issue, but didn't get to the bottom of it yet. This notebook shows the behavior I found that was suspicious: https://gist.github.com/marii-moe/a8e7a330fbd4d9b0dc326e396ef17e36

commit that this is from is 94e1568, though I have no idea what version actually broke it yet.

I was moving my last project off of the old environment when I noticed the discrepancy between the old and new fastai.

Just noticed gist has display issues with displaying the results from the previous version of fastai, leaving this here as I am having trouble getting its formatting correct in the notebook gist:

Pipeline: BBoxLabeler -> PointScaler -> Resize -> ToTensor
{'img_size': None}
<function <lambda> at 0x7f6d1da1a2f0>
{'img_size': None}
<function <lambda> at 0x7f6d1da1a268>
{'img_size': None}
Pipeline: IntToFloatTensor
{'img_size': None}
Pipeline: IntToFloatTensor
{'img_size': None}
<function <lambda> at 0x7f6d1da1a268>
{'img_size': None}
<bound method DataLoader.create_batch of <fastai2.data.core.TfmdDL object at 0x7f6d28af1320>>
{'img_size': None}
Pipeline: bb_pad
{'img_size': None}
<function <lambda> at 0x7f6d1da1a2f0>
{'img_size': None}
Pipeline: BBoxLabeler -> PointScaler -> Resize -> ToTensor
{'img_size': [256, 256]}

NotE: this was found in a debugger, I am just trying to make it more obvious what is happening by breaking open the dataloader.

All of the 'Nones' is what makes me suspect that the previous version of fastai had something that was broken, but a new version fixed something else, that actually ended up breaking bounding boxes, by making img_size not null. Which is why I think IRailean's fix may work. This logic is a bit convoluted though.

@jph00
Copy link
Member

jph00 commented Sep 17, 2020

Thanks to you both! I'll just go ahead and merge this then, but I don't actually know if it's the right fix, so I'd love it if you folks could take a look at some image augmentation outputs and confirm visually that it all looks ok to you! :)

@jph00 jph00 changed the title Prioritize self.sz over sz in _get_sz function img_size attribute for TensorPoint is not updated properly Sep 17, 2020
@jph00 jph00 added the bug label Sep 17, 2020
@jph00 jph00 merged commit efd4640 into fastai:master Sep 17, 2020
@muellerzr
Copy link
Contributor

Can confirm this is working. Also fixes any issues with aug_transforms along with it. Nice work guys :)

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

Successfully merging this pull request may close these issues.

None yet

4 participants