[docs] RandomResizeCrop signature quirks

[vadimkantorov]

1. Why do we need to have so much precision: 1.333333333333? 1.3 would suffice and be more readable
2. <InterpolationMode.BILINEAR: 'bilinear'>. It's not clear from this if the enum should be preferred or the constant (and it's also not copy-pastable in the code). It's also not clear which type is used for InterpolationMode, especially given that for a long time only PIL.Image.InterpolationMode was supported, hence even new codebases are using the legacy PIL types: https://github.com/facebookresearch/dino/blob/58aabc0/main_dino.py#L420
3. Why does InterpolationMode live in torchvision.transforms.functional whereas the type using it lives in torchvision.transforms? This enum is basic enough to deserve placement in torchvision.transforms directly IMHO.
4. The doc text mentions torchvision.transforms.InterpolationMode, but it's incorrect since the type currently lives in transforms.functional: https://pytorch.org/vision/master/transforms.html?highlight=randomresizedcrop#torchvision.transforms.functional.InterpolationMode

[vadimkantorov]

Also, string constant (despite ths string constant in the signature may be suggestive) does not work. Maybe it should work? That's the interface supported by the underlying F.interpolate?

Currently the error is also not very clear:

  File "/specific/netapp5_2/gamir/lab/vadim/prefix/miniconda/lib/python3.8/site-packages/torchvision/transforms/functional.py", line 371, in resize
    raise TypeError("Argument interpolation should be a InterpolationMode")
TypeError: Argument interpolation should be a InterpolationMode

It would be nice if the error at least provdided a fully-qualified type: torchvision.transforms.functional.InterpolationMode, and maybe mentioned explicitly that the strings are not supported

[NicolasHug]

@vadimkantorov thanks for the report, please feel free to open a PR to fix these (except for 3 maybe, which should be discussed in a different issue / PR since it's not related to the docs)

[vadimkantorov]

I think (1) and (2) actually are about configuration of Sphinx / rendering ...

[NicolasHug]

Yes, and they are still doc-related. My point is that 3) is about the API, and will require more discussions than the rest of the items: we can't move stuff around easily when they're public. So it's best not to address all of your points together in the same PR

[vadimkantorov]

I propose to also have strings supported as interpolation mode (as Kornia does)...

[datumbox]

@vadimkantorov The use of "enum-style" modes was a deliberate choice in order to make the code easier to read/navigate/refactor.

[fmassa]

@vadimkantorov about points 3 and 4:
The enum is visible from transforms.py in

vision/torchvision/transforms/transforms.py

Line 17 in 00ea88c

from .functional import InterpolationMode, _interpolation_modes_from_int

and is needed to be in functional.py because transforms.py import functional.py, otherwise we would have cyclic dependencies (except if we extract it to its own file, which I would be fine with).

About supporting strings for InterpolationMode, I remember discussing this with @datumbox back then and he mentioned that Enums were better for discoverability, specially with smart IDEs that provide auto-completion features.

About points 1 and 2, we should improve the documentation there, and maybe provide a better __repr__ to InterpolationMode

How do you think we could improve the docs so that new projects can know which version they should be using?

[vadimkantorov]

I don't mind enum existing, I'm only advocating in favor of supporting strings as well. These interpolation modes naming are quite standard across opencv / pil / kornia etc. So it's easier when changing backends. As is in my case: I went from torchvision+pil to kornia

[vadimkantorov]

Since enums have appeared in Python relatively not long time ago, not everyone (including me) have adjusted to their strange-y repr. So some clearness / examples would be great - and copy-pasteability, that is the enum type should be fully qualified (despite being verbose)

[vadimkantorov]

Probably also to just be supporting exactly the same interface as F.interpolate does, to not force users have different interoplate mode typing in different parts of pytorch

[fmassa]

These interpolation modes naming are quite standard across opencv / pil / kornia

Well, each of those libraries have different names / conventions for each method, but both PIL and OpenCV use some sort of enums (and not strings) for representing the mode.
From this perspective, using a proper Python enum seems adapted to me. But I would be ok making it support a string as well (which is what TF does for tf.image.resize )

So some clearness / examples would be great - and copy-pasteability, that is the enum type should be fully qualified (despite being verbose)

That sounds ok to me.

Probably also to just be supporting exactly the same interface as F.interpolate does, to not force users have different interoplate mode typing in different parts of pytorch

This is something which is worth further discussion. Now that torchvision transforms support both Tensor and PIL, there is indeed a question on wether we should follow PIL (BC) or lean towards PyTorch.

IMO, the current situation with PyTorch interpolate is a bit messy, with interpolation modes being linear / bilinear / trilinear / etc, while only one single option actually works for a given input shape. I would lean towards not following what PyTorch does wrt that, but instead focus on providing more generic abstractions in PyTorch for interpolating over any arbitrary number of dimensions, which is what I proposed in pytorch/pytorch#10482 and that we are slowly getting there by unifying the CPU implementations of all those methods, see pytorch/pytorch#54500