-
Notifications
You must be signed in to change notification settings - Fork 296
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
Update resize() to be on par with torchvision speed #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR & for adding the speed stats, @membriux! Please fix the nits I added in the file comments. Also, a few things:
- Note that this augmentation isn't always faster with this change; when an interpolation method is not passed in, the default one used was bicubic resampling, which is slower but also results in a higher-quality image than bilinear resampling, which you made the new default. So now the default behavior of
resize
will be faster than before, but the resuting image will be lower quality and if the user chooses to pass inresample=Image.BICUBIC
then the behavior & runtime will be the same as before. - Because as I mentioned the resampling method is different from before & results in a lower-quality image, you need to regenerate the output image
test_resize.png
by running the newresize
code.
Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>
Doesn't detect it as updated
Didn't detect it as updated file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more things to fix, then we can land this! :)
Use this for updating images on the test cases→ |
Updated metadata, transform, and unit test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added!
@zpapakipos ready to go! 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit, then this is good to know!
Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>
Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>
@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@membriux has updated the pull request. You must reimport the pull request before landing. |
@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@membriux has updated the pull request. You must reimport the pull request before landing. |
@membriux has updated the pull request. You must reimport the pull request before landing. |
@membriux has updated the pull request. You must reimport the pull request before landing. |
@membriux has updated the pull request. You must reimport the pull request before landing. |
@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just waiting for @jbitton to accept this diff internally so we can land!
…-merge-video-audio * 'main' of https://github.com/Adib234/AugLy: Update resize() to be on par with torchvision speed (facebookresearch#144) Change local image paths to links (facebookresearch#162) Text Augmentation Optimizations (facebookresearch#147) chore(typo) (facebookresearch#154) Downloads badge (facebookresearch#158) Add error messages for easier debugging (facebookresearch#151)
Summary
Summary: Refactored
resize()
from image/functional.py to be on par with torchvision. However, I just have one minor failure in my code. Please advise on where I should look :)Test results
The following results were acquired from my machine
Test code → https://colab.research.google.com/drive/14-KZdSGaOaz73OgIS0DZZY4RsS3cJ0rg#scrollTo=xVI_h-1v49lC
Unit Tests
If your changes touch the
audio
module, please run all of theaudio
tests and paste the output here. Likewise forimage
,text
, &video
. If your changes could affect behavior in multiple modules, please run the tests for all potentially affected modules. If you are unsure of which modules might be affected by your changes, please just run all the unit tests.TEST OUTPUT