Update resize() to be on par with torchvision speed #144

membriux · 2021-11-17T02:47:34Z

Summary

I have read CONTRIBUTING.md to understand how to contribute to this repository :)

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

Augly original (without interpolation)= 0.04475s
Augly revised (with interpolation) = 0.02873s
torchvision (uses interpolation) = 0.02696s

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 the audio tests and paste the output here. Likewise for image, 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.


### Image
```bash
python -m unittest discover -s augly/tests/image_tests/ -p "*_test.py"
# Or `python -m unittest discover -s augly/tests/image_tests/ -p "*.py"` to run pytorch test too (must install `torchvision` to run)

TEST OUTPUT

======================================================================
FAIL: test_Resize (transforms_unit_test.TransformsImageUnitTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/macbookpro/Desktop/Github/AugLy/augly/tests/image_tests/transforms_unit_test.py", line 187, in test_Resize
    self.evaluate_class(imaugs.Resize(), fname="resize")
  File "/Users/macbookpro/Desktop/Github/AugLy/augly/tests/image_tests/base_unit_test.py", line 111, in evaluate_class
    self.assertTrue(
AssertionError: False is not true

----------------------------------------------------------------------
Ran 82 tests in 52.735s

FAILED (failures=1, skipped=5)

zpapakipos

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 in resample=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 new resize code.

augly/image/functional.py

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

Doesn't detect it as updated

Didn't detect it as updated file

zpapakipos

Just a few more things to fix, then we can land this! :)

augly/tests/image_tests/functional_unit_test.py

augly/image/functional.py

membriux · 2021-11-22T18:00:25Z

Use this for updating images on the test cases→
aug_function(self.local_img_path, output_path=f"/tmp/{aug_function.__name__}.png", **kwargs)

Updated metadata, transform, and unit test

membriux

Added!

membriux · 2021-11-23T02:36:20Z

@zpapakipos ready to go! 🚀

zpapakipos

One last nit, then this is good to know!

augly/image/functional.py

augly/image/transforms.py

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

augly/image/functional.py

augly/image/transforms.py

facebook-github-bot · 2021-11-25T08:44:15Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

augly/image/transforms.py

facebook-github-bot · 2021-11-25T08:54:43Z

@membriux has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-11-25T08:55:19Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

augly/image/functional.py

augly/image/transforms.py

facebook-github-bot · 2021-11-25T09:48:20Z

@membriux has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-11-25T09:48:26Z

@membriux has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-11-25T09:48:52Z

@membriux has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-11-25T09:49:00Z

@membriux has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-11-25T09:52:17Z

@zpapakipos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

zpapakipos

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)

Update functional.py

cc72861

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 17, 2021

membriux changed the title ~~Update functional.py~~ Update resize() to be on par with torchvision speed Nov 17, 2021

zpapakipos requested changes Nov 18, 2021

View reviewed changes

augly/image/functional.py Outdated Show resolved Hide resolved

membriux and others added 5 commits November 18, 2021 09:16

Update augly/image/functional.py

4ae193f

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

Update functional.py

e185484

Delete test_resize.png

76f12eb

Doesn't detect it as updated

Create test_resize.png

96f8125

Didn't detect it as updated file

Black linter + updated resize() test param

bc774bb

zpapakipos requested changes Nov 22, 2021

View reviewed changes

augly/tests/image_tests/functional_unit_test.py Show resolved Hide resolved

augly/image/functional.py Outdated Show resolved Hide resolved

membriux added 2 commits November 22, 2021 13:26

resample variable add on

396712c

Updated metadata, transform, and unit test

Black format

865b7da

membriux commented Nov 23, 2021

View reviewed changes

membriux closed this Nov 23, 2021

membriux reopened this Nov 23, 2021

zpapakipos requested changes Nov 24, 2021

View reviewed changes

augly/image/functional.py Outdated Show resolved Hide resolved

augly/image/transforms.py Outdated Show resolved Hide resolved

membriux and others added 2 commits November 24, 2021 13:20

Update augly/image/functional.py

96a78dc

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

Update augly/image/transforms.py

62283ca

Co-authored-by: Zoe Papakipos <zpapakipos@users.noreply.github.com>

zpapakipos reviewed Nov 25, 2021

View reviewed changes

augly/image/functional.py Outdated Show resolved Hide resolved

Update augly/image/functional.py

15216f2

zpapakipos reviewed Nov 25, 2021

View reviewed changes

augly/image/transforms.py Outdated Show resolved Hide resolved

Update augly/image/transforms.py

879d5fd

zpapakipos reviewed Nov 25, 2021

View reviewed changes

augly/image/transforms.py Outdated Show resolved Hide resolved

Update augly/image/transforms.py

58fbcbe

zpapakipos reviewed Nov 25, 2021

View reviewed changes

augly/image/functional.py Outdated Show resolved Hide resolved

augly/image/functional.py Outdated Show resolved Hide resolved

augly/image/transforms.py Outdated Show resolved Hide resolved

augly/image/transforms.py Outdated Show resolved Hide resolved

Update augly/image/functional.py

00ed551

Update augly/image/transforms.py

40d8647

Update augly/image/functional.py

4335255

Update augly/image/transforms.py

7d5d0bf

zpapakipos approved these changes Nov 29, 2021

View reviewed changes

facebook-github-bot closed this in 5edf973 Dec 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update resize() to be on par with torchvision speed #144

Update resize() to be on par with torchvision speed #144

membriux commented Nov 17, 2021 •

edited

zpapakipos left a comment

zpapakipos left a comment

membriux commented Nov 22, 2021

membriux left a comment

membriux commented Nov 23, 2021

zpapakipos left a comment

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

zpapakipos left a comment

Update resize() to be on par with torchvision speed #144

Update resize() to be on par with torchvision speed #144

Conversation

membriux commented Nov 17, 2021 • edited

Summary

Unit Tests

zpapakipos left a comment

Choose a reason for hiding this comment

zpapakipos left a comment

Choose a reason for hiding this comment

membriux commented Nov 22, 2021

membriux left a comment

Choose a reason for hiding this comment

membriux commented Nov 23, 2021

zpapakipos left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

facebook-github-bot commented Nov 25, 2021

zpapakipos left a comment

Choose a reason for hiding this comment

membriux commented Nov 17, 2021 •

edited