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

expand has_any and has_all to also accept check callables #6447

Merged
merged 10 commits into from
Aug 18, 2022

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Aug 18, 2022

This is useful if the type checking involves more than plain types. A common example is to check for an image in the sample which is done with the types features.Image and PIL.Image.Image as well as the check function is_simple_tensor. With this PR, we can pass all three to has_any and has_all:

import PIL.Image
import torch
from torchvision.prototype import features
from torchvision.prototype.transforms._utils import has_any, is_simple_tensor
from torchvision.prototype.transforms.functional import to_image_pil

image_tensor = torch.rand(3, 16, 16)
image_pil = to_image_pil(image_tensor)
image = features.Image(image_tensor)

for input in [image_tensor, image_pil, image]:
    assert has_any(input, PIL.Image.Image, features.Image, is_simple_tensor)

This PR also uses this new idiom to add support for simple tensors to CutMix and MixUp as well as RandomIoUCrop as discussed in #6401 (comment).

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.

@pmeier This might be a matter of styles and preferences, but I have hard time reading code that is so densely written as this. I understand that some people like it as it's more pythonic.

@vfdev-5 thoughts?

test/test_prototype_transforms_utils.py Outdated Show resolved Hide resolved
torchvision/prototype/transforms/_utils.py Outdated Show resolved Hide resolved
torchvision/prototype/transforms/_utils.py Outdated Show resolved Hide resolved
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've added a few optional comments above, I haven't thoroughly checked it as I am in between meetings. Feel free to ignore.

flat_sample, _ = tree_flatten(sample)
return not bool(set(types) - set([type(obj) for obj in flat_sample]))
for type_or_check in types_or_checks:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are asymmetric because the behavior we want for has_any is any(any(...)) and for has_all is any(all(...)). If has_all would be all(all(...)) they would be symmetric.

@pmeier pmeier merged commit 330b6c9 into pytorch:main Aug 18, 2022
@pmeier pmeier deleted the expand-has-utils branch August 18, 2022 14:58
@pmeier pmeier mentioned this pull request Aug 22, 2022
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2022
…6447)

Summary:
* expand has_any and has_all to also accept check callables

* add test and fix has_all

* add support for simple tensor images to CutMix, MixUp and RandomIoUCrop

* remove TODO

* remove pythonic syntax sugar

* simplify

* use concreate examples in test rather than abstract ones

* simplify further

Reviewed By: datumbox

Differential Revision: D39013675

fbshipit-source-id: 6cd68d471b7cf94192284cdf7948c87ed570e6af
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