readd functional transforms API to prototype #5295

pmeier · 2022-01-27T08:40:43Z

This re-adds the functional API to the prototype transforms. They are not yet compatible with the transforms. That will come in a follow-up PR.

Design principles:

Every kernel has to be jit.scriptable
Pillow is no longer supported. Note this only applies to the low level kernels. There will be another layer on top, i.e. a higher level functional API, that will preserve BC.
Every kernel needs to work on vanilla tensors
Kernels can make assumptions on the tensor data, i.e. requiring bounding boxes in XYXY format
Kernels that implement the same transform for different feature types should use the transform name suffixed by the feature type in snake case, e.g. horizontal_flip_image and horizontal_flip_bounding_box.
Kernels that implement the same transform for different feature types should use the same parameters as much as possible.

In the current state, this PR only adds a few non-image kernels and imports most image kernels from the stable implementation.

facebook-github-bot · 2022-01-27T08:40:49Z

💊 CI failures summary and remediations

As of commit a77bf1f (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

torchvision/prototype/transforms/functional/utils.py

NicolasHug

Thanks @pmeier , only took a very brief look

torchvision/prototype/transforms/functional/utils.py

NicolasHug · 2022-01-27T13:38:40Z

test/test_prototype_transforms_functional.py

+            for kernel_info in KERNEL_INFOS
+            for idx, sample_input in enumerate(kernel_info.sample_inputs())


instead of the 2 manual for loops could we simply use 2 @parametrize statements?

Also, is there stark difference between passing idx instead of relying on pytest's default ids?

instead of the 2 manual for loops could we simply use 2 @parametrize statements?

We can't use two separate parametrizations, since the inner loop depends on the outer.

Also, is there stark difference between passing idx instead of relying on pytest's default ids?

Let's look into this more after #5295 (comment).

NicolasHug · 2022-01-27T13:41:11Z

test/test_prototype_transforms_functional.py

+        yield make_bounding_box(format=format, extra_dims=extra_dims_)
+
+
+class SampleInput:


Could you explain why we need this class?

The other option would be for a simple inputs function to return ((...), dict(...)) to bundle args and kwargs. IMO this is not as convenient as having a single structure to hold everything. To me

yield SampleInput(bounding_box, old_image_size=bounding_box.image_size, new_image_size=new_image_size)

is more readable than

yield (bounding_box,), dict(old_image_size=bounding_box.image_size, new_image_size=new_image_size)

given that it resembles the actual call signature.

Plus, I was looking into lazily loading the samples. This is not implemented yet, so we are generating all samples at test collection time. This can become an issue real quick if we go along this automated tests direction.

vfdev-5 · 2022-01-27T13:46:31Z

torchvision/prototype/features/_bounding_box.py

+        # import at runtime to avoid cyclic imports
+        from torchvision.prototype.transforms.functional import convert_bounding_box_format


Maybe, this is a sign to redesign the structure and put this method here or in utils module ?

It was a design choice to have all transforming functions in torchvision.prototype.transforms.functional. This is not implemented yet, but for the automatic dispatch to work, we need to depend on torchvision.prototype.features. Thus, if we want to honor the design choice, we can't get around the cyclic imports if we want to provide these convenience conversion methods.

If we relax the design choice to all transforming functions need to be present in torchvision.prototype.transforms.functional, but can be imported from somewhere else, I fully agree, and this should be refactored.

torchvision/prototype/transforms/functional/_geometry.py

…eadd-functional

NicolasHug

Thanks @pmeier

pmeier · 2022-02-04T13:50:26Z

@datumbox reviewed in #5375.

* revamp prototype features (#5283) * remove decoding from prototype datasets (#5287) * remove decoder from prototype datasets * remove unused imports * cleanup * fix readme * use OneHotLabel in SEMEION * improve voc implementation * revert unrelated changes * fix semeion mock data * fix pcam * readd functional transforms API to prototype (#5295) * readd functional transforms * cleanup * add missing imports * remove __torch_function__ dispatch * readd repr * readd empty line * add test for scriptability * remove function copy * change import from functional tensor transforms to just functional * fix import * fix test * fix prototype features and functional transforms after review (#5377) * fix prototype functional transforms after review * address features review * make mypy more strict on prototype features * make mypy more strict for prototype transforms * fix annotation * fix kernel tests * add automatic feature type dispatch to functional transforms (#5323) * add auto dispatch * fix missing arguments error message * remove pil kernel for erase * automate feature specific parameter detection * fix typos * cleanup dispatcher call * remove __torch_function__ from transform dispatch * remove auto-generation * revert unrelated changes * remove implements decorator * change register parameter order * change order of transforms for readability * add documentation for __torch_function__ * fix mypy * inline check for support * refactor kernel registering process * refactor dispatch to be a regular decorator * split kernels and dispatchers * remove sentinels * replace pass with ... * appease mypy * make single kernel dispatchers more concise * make dispatcher signatures more generic * make kernel checking more strict * revert doc changes * address Franciscos comments * remove inplace * rename kernel test module * fix inplace * remove special casing for pil and vanilla tensors * address comments * update docs * cleanup features / transforms feature branch (#5406) * mark candidates for removal * align signature of resize_bounding_box with corresponding image kernel * fix documentation of Feature * remove interpolation mode and antialias option from resize_segmentation_mask * remove or privatize functionality in features / datasets / transforms

Summary: * revamp prototype features (#5283) * remove decoding from prototype datasets (#5287) * remove decoder from prototype datasets * remove unused imports * cleanup * fix readme * use OneHotLabel in SEMEION * improve voc implementation * revert unrelated changes * fix semeion mock data * fix pcam * readd functional transforms API to prototype (#5295) * readd functional transforms * cleanup * add missing imports * remove __torch_function__ dispatch * readd repr * readd empty line * add test for scriptability * remove function copy * change import from functional tensor transforms to just functional * fix import * fix test * fix prototype features and functional transforms after review (#5377) * fix prototype functional transforms after review * address features review * make mypy more strict on prototype features * make mypy more strict for prototype transforms * fix annotation * fix kernel tests * add automatic feature type dispatch to functional transforms (#5323) * add auto dispatch * fix missing arguments error message * remove pil kernel for erase * automate feature specific parameter detection * fix typos * cleanup dispatcher call * remove __torch_function__ from transform dispatch * remove auto-generation * revert unrelated changes * remove implements decorator * change register parameter order * change order of transforms for readability * add documentation for __torch_function__ * fix mypy * inline check for support * refactor kernel registering process * refactor dispatch to be a regular decorator * split kernels and dispatchers * remove sentinels * replace pass with ... * appease mypy * make single kernel dispatchers more concise * make dispatcher signatures more generic * make kernel checking more strict * revert doc changes * address Franciscos comments * remove inplace * rename kernel test module * fix inplace * remove special casing for pil and vanilla tensors * address comments * update docs * cleanup features / transforms feature branch (#5406) * mark candidates for removal * align signature of resize_bounding_box with corresponding image kernel * fix documentation of Feature * remove interpolation mode and antialias option from resize_segmentation_mask * remove or privatize functionality in features / datasets / transforms Reviewed By: sallysyw Differential Revision: D34265747 fbshipit-source-id: 569ed9f74ac0c026391767c3b422ca0147f55ead

pmeier added 6 commits January 27, 2022 08:58

readd functional transforms

4b327ba

cleanup

eaac4f2

add missing imports

079a005

remove __torch_function__ dispatch

c78f6f7

readd repr

45ce1f9

readd empty line

0b81a23

pmeier added module: transforms prototype labels Jan 27, 2022

pytorch-bot bot added the ciflow/default label Jan 27, 2022

facebook-github-bot added the cla signed label Jan 27, 2022

pmeier changed the title ~~Transforms/readd functional~~ readd functional transforms API to prototype Jan 27, 2022

pmeier commented Jan 27, 2022

View reviewed changes

torchvision/prototype/transforms/functional/utils.py Outdated Show resolved Hide resolved

pmeier requested a review from NicolasHug January 27, 2022 08:49

add test for scriptability

265b3d4

NicolasHug reviewed Jan 27, 2022

View reviewed changes

vfdev-5 reviewed Jan 27, 2022

View reviewed changes

torchvision/prototype/transforms/functional/_geometry.py Outdated Show resolved Hide resolved

pmeier added 6 commits January 27, 2022 15:07

remove function copy

2a7983e

change import from functional tensor transforms to just functional

1392e78

fix import

249cfb3

Merge branch 'revamp-prototype-features-transforms' into transforms/r…

e455810

…eadd-functional

Merge branch 'revamp-prototype-features-transforms' into transforms/r…

8c96fc0

…eadd-functional

fix test

a77bf1f

NicolasHug approved these changes Jan 31, 2022

View reviewed changes

pmeier merged commit 466b845 into pytorch:revamp-prototype-features-transforms Jan 31, 2022

pmeier deleted the transforms/readd-functional branch January 31, 2022 15:07

pmeier mentioned this pull request Jan 31, 2022

add automatic feature type dispatch to functional transforms #5323

Merged

pmeier mentioned this pull request Feb 4, 2022

fix prototype features and functional transforms after review #5377

Merged

pmeier mentioned this pull request Feb 7, 2022

[NOMERGE] Review Features prototype #5379

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

readd functional transforms API to prototype #5295

readd functional transforms API to prototype #5295

pmeier commented Jan 27, 2022 •

edited

Loading

facebook-github-bot commented Jan 27, 2022 •

edited

Loading

NicolasHug left a comment

NicolasHug Jan 27, 2022

pmeier Jan 27, 2022

NicolasHug Jan 27, 2022

pmeier Jan 27, 2022

pmeier Jan 27, 2022

vfdev-5 Jan 27, 2022

pmeier Jan 27, 2022

NicolasHug left a comment

pmeier commented Feb 4, 2022 •

edited

Loading

		for kernel_info in KERNEL_INFOS
		for idx, sample_input in enumerate(kernel_info.sample_inputs())

		yield make_bounding_box(format=format, extra_dims=extra_dims_)


		class SampleInput:

		# import at runtime to avoid cyclic imports
		from torchvision.prototype.transforms.functional import convert_bounding_box_format

readd functional transforms API to prototype #5295

readd functional transforms API to prototype #5295

Conversation

pmeier commented Jan 27, 2022 • edited Loading

facebook-github-bot commented Jan 27, 2022 • edited Loading

💊 CI failures summary and remediations

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug Jan 27, 2022

Choose a reason for hiding this comment

pmeier Jan 27, 2022

Choose a reason for hiding this comment

NicolasHug Jan 27, 2022

Choose a reason for hiding this comment

pmeier Jan 27, 2022

Choose a reason for hiding this comment

pmeier Jan 27, 2022

Choose a reason for hiding this comment

vfdev-5 Jan 27, 2022

Choose a reason for hiding this comment

pmeier Jan 27, 2022

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

pmeier commented Feb 4, 2022 • edited Loading

pmeier commented Jan 27, 2022 •

edited

Loading

facebook-github-bot commented Jan 27, 2022 •

edited

Loading

pmeier commented Feb 4, 2022 •

edited

Loading