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

Unify Tensor and PIL transforms #2292

Closed
16 tasks done
fmassa opened this issue Jun 5, 2020 · 19 comments
Closed
16 tasks done

Unify Tensor and PIL transforms #2292

fmassa opened this issue Jun 5, 2020 · 19 comments

Comments

@fmassa
Copy link
Member

fmassa commented Jun 5, 2020

Now that most of the tensor transforms have been implemented #1375, it is time to make unify the implementations for PIL and Tensor so that torchvision.transforms and torchvision.transforms.functional works seamlessly between both datatypes.

As such, we would like that the following work with the same interface:

# functional interface
torchvision.transforms.functional.hflip(pil_image)
torchvision.transforms.functional.hflip(tensor_image)

# class interface
transform = torchvision.transforms.RandomHorizontalFlip(0.3)
transform(pil_image)
transform(tensor_image)

# torchscript support for tensor_image
script_transform = torch.jit.script(transform)
script_transform(tensor_image)

Example PRs adding support for hflip and vflip: #2282 #2283

We would like to add support for torchscript to the torchvision.transforms interface as well, which for simplicity might require a few changes. Most notably we would need the transforms to inherit from nn.Module.

Here is a list of transforms that can be readily converted:

@clemkoa
Copy link
Contributor

clemkoa commented Jun 7, 2020

Hi! I would be glad to work on a PR for this issue. I can start with the color jitter if that's okay with you

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 22, 2020

@fmassa I can work on some of these listed transformation if it is OK.

@vfdev-5 vfdev-5 mentioned this issue Jun 23, 2020
1 task
@dimimal
Copy link

dimimal commented Jun 23, 2020

In the documentation of pytorch it is mentioned that functional module works with both tensors and pil objects. This is about torch version 1.5.1 and torchvision 0.6.1. However, when I try to use them with torch.tensor it throws an error that the object is not PIL. I checked the source code and the snippet shown in the docs is missing. I use the functional.hflip() function of the docs. I thought that the unification has been completed, hasn't it?

@fmassa
Copy link
Member Author

fmassa commented Jun 23, 2020

@dimimal the unification is not yet complete

Can you point out where in the documentation you found the reference that all functions work with both tensors and PIL Images?

@fmassa
Copy link
Member Author

fmassa commented Jun 23, 2020

@vfdev-5 sure go for it!

@dimimal
Copy link

dimimal commented Jun 23, 2020

@fmassa
Copy link
Member Author

fmassa commented Jun 23, 2020

@dimimal hflip supports both tensors and PIL Images since #2282 , but only on master and only a handful of functions support it for now

@dimimal
Copy link

dimimal commented Jun 23, 2020

@fmassa I wasn't using the master and the docs confused me with the noted version. Thank you. Is there anything I can do to help in contribution?

@fmassa
Copy link
Member Author

fmassa commented Jun 24, 2020

@dimimal sure, contributions are more than welcome! There are two steps here: finishing up #1375 , and then implementing the glue code, similarly to the example PRs #2282 #2283, or more recently #2345 I think implementing adjust_gamma would be a good issue to start with.

Note that one of the biggest difficulties will be making the implementation work with torchscript, as not everything that we use is natively supported.

vfdev-5 added a commit to Quansight/vision that referenced this issue Jul 3, 2020
- RandomErasing is not scriptable
fmassa pushed a commit that referenced this issue Jul 3, 2020
* Related to #2292
- RandomErasing is not scriptable

* Fixed code according to review comments
- added additional checking of value vs img num_channels
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jul 24, 2020

@fmassa if ops should also work on tensors on non-cpu devices, a supplementary check of certain geometrical ops will be needed. Should we setup some CI tests for that ?

@fmassa
Copy link
Member Author

fmassa commented Jul 29, 2020

Should we setup some CI tests for that ?

Yes, please!

de-vri-es pushed a commit to fizyr-forks/torchvision that referenced this issue Aug 4, 2020
* Related to pytorch#2292
- RandomErasing is not scriptable

* Fixed code according to review comments
- added additional checking of value vs img num_channels
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Aug 6, 2020

@nairbv would you be interested in working on unifying adjust_hue ?

@fmassa
Copy link
Member Author

fmassa commented Aug 7, 2020

@vfdev-5 go for it!

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Aug 10, 2020

According to a discussion with @fmassa , we also have to make sure that all transformations work with a batch of images.

EDIT: requested as a another issue : #2583

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Sep 3, 2020

The issue can be closed as all mentioned transformations now support tensor and pil image as input.

@vfdev-5 vfdev-5 closed this as completed Sep 3, 2020
@voldemortX
Copy link
Contributor

Hi! Very exciting codes! Wondering whether future commits will support fill color other than 0 for tensor affine transformations? It is kind of important for tasks like semantic segmentation when transforming labels. @vfdev-5

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Oct 25, 2020

@voldemortX thanks for the feedback !

Wondering whether future commits will support fill color other than 0 for tensor affine transformations? It is kind of important for tasks like semantic segmentation when transforming labels.

Could you please open an issue for that such we could track this feature request and see what could be possible to do.
Currently, the blocker is with pytorch grid_sample which has

padding_mode="zeros": use 0 for out-of-bound grid locations

@voldemortX
Copy link
Contributor

@voldemortX thanks for the feedback !

Wondering whether future commits will support fill color other than 0 for tensor affine transformations? It is kind of important for tasks like semantic segmentation when transforming labels.

Could you please open an issue for that such we could track this feature request and see what could be possible to do.
Currently, the blocker is with pytorch grid_sample which has

padding_mode="zeros": use 0 for out-of-bound grid locations

New issue opened #2887

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

No branches or pull requests

5 participants