Skip to content

[proto] Ported all transforms to the new API #6305

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 20 commits into from
Jul 28, 2022
Merged

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Jul 22, 2022

Description:

  • Ported all transforms to the new API
  • Added tests

Please merge #6272 before this PR

What remains

  • Polish sharp edges:
    • Search and fix for TODOs in torchvision/prototype/transforms and test/test_prototype_*. Mostly those to implement a missing functionality.
  • Better testing:
    • check if we test all ops and transforms
    • functional ops have to be tested on result correctness, jit/eager, devices etc. Philip was proposing to provide a simplified way to test that.
    • transforms have to be tested on parameter generation and checking if functional counterpart is correctly called via mocking (this is already done for majority of transforms in test_prototype_transforms.py)
    • we need to test specific Feature.op for example BoundingBox.rotate as they change metadata like image_size.

CI Failures to take into account:

  • FAILED test/test_prototype_transforms_functional.py::test_eager_vs_scripted[gaussian_blur_image_tensor-26]
____________ test_eager_vs_scripted[gaussian_blur_image_tensor-26] _____________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 740, in test_eager_vs_scripted
    torch.testing.assert_close(eager, scripted)
  File "/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/torch/testing/_comparison.py", line 1359, in assert_close
    msg=msg,
  File "/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/torch/testing/_comparison.py", line 1093, in assert_equal
    raise error_metas[0].to_error(msg)
AssertionError: Tensor-likes are not equal!

vfdev-5 added 11 commits July 12, 2022 22:50
* Added supported/unsupported data checks in the tests for cutmix/mixup

* Added RandomRotation, RandomAffine transforms tests

* Added tests for RandomZoomOut, Pad

* Update test_prototype_transforms.py
* Added GaussianBlur transform and tests

* Fixing code format

* Copied correctness test
* Added random color transforms and tests

* Disable smoke test for RandomSolarize, RandomAdjustSharpness
- replaced real image creation by mocks for other tests
* WIP [proto] Added functional elastic transform with tests

* Added more functional tests

* WIP on elastic op

* Added elastic transform and tests

* Added tests

* Added tests for ElasticTransform
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Thanks @vfdev-5, looks good overall. I've added a few comments, let me know what you think. I'm not going to check the tests because as you indicate more discussions will be required over the testing strategy.

@vfdev-5 vfdev-5 force-pushed the proto-transforms-api branch from a5a66d7 to d226e16 Compare July 28, 2022 10:51
@vfdev-5 vfdev-5 requested a review from datumbox July 28, 2022 11:42
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, I haven't checked the testing strategy but I know @pmeier's plan is to revise it thoroughly so I'm happy to go with whatever you prefer for now.



def has_all(sample: Any, *types: Type) -> bool:
return not bool(set(types) - set(_extract_types(sample)))
flat_sample, _ = tree_flatten(sample)
return not bool(set(types) - set([type(obj) for obj in flat_sample]))
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail if we subclass one of the types (say we subclass Feature.Image) but since that's not the intention now, we are OK to keep it as-is.

@@ -260,7 +260,7 @@ def _parse_fill(
) -> Dict[str, Optional[Union[float, List[float], Tuple[float, ...]]]]:

# Process fill color for affine transforms
num_bands = len(img.getbands())
num_bands = get_image_num_channels(img)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Instead of num_bands use num_channels?

@vfdev-5 vfdev-5 merged commit 77c8c91 into main Jul 28, 2022
@vfdev-5 vfdev-5 deleted the proto-transforms-api branch July 28, 2022 13:58
facebook-github-bot pushed a commit that referenced this pull request Aug 2, 2022
Summary:
* [proto] Added few transforms tests, part 1 (#6262)

* Added supported/unsupported data checks in the tests for cutmix/mixup

* Added RandomRotation, RandomAffine transforms tests

* Added tests for RandomZoomOut, Pad

* Update test_prototype_transforms.py

* Added RandomCrop transform and tests (#6271)

* [proto] Added GaussianBlur transform and tests (#6273)

* Added GaussianBlur transform and tests

* Fixing code format

* Copied correctness test

* [proto] Added random color transforms and tests (#6275)

* Added random color transforms and tests

* Disable smoke test for RandomSolarize, RandomAdjustSharpness

* Added RandomPerspective and tests (#6284)

- replaced real image creation by mocks for other tests

* Added more functional tests (#6285)

* [proto] Added elastic transform and tests (#6295)

* WIP [proto] Added functional elastic transform with tests

* Added more functional tests

* WIP on elastic op

* Added elastic transform and tests

* Added tests

* Added tests for ElasticTransform

* Try to format code as in #5106

* Fixed bug in affine get_params test

* Implemented RandomErase on PIL input as fallback to tensors (#6309)

Added tests

* Added image_size computation for BoundingBox.rotate if expand (#6319)

* Added image_size computation for BoundingBox.rotate if expand

* Added tests

* Added erase_image_pil and eager/jit erase_image_tensor test (#6320)

* Updates according to the review

Reviewed By: NicolasHug

Differential Revision: D38351755

fbshipit-source-id: 4b52b530f93c3dfa92326e01803665cb44003a63

Co-authored-by: Vasilis Vryniotis <[email protected]>
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.

3 participants