Skip to content

Revamp prototype features and transforms #5407

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 19 commits into from
Feb 11, 2022

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Feb 11, 2022

This PR is a large overhaul of what we currently have for prototype features and transforms. For larger changes, individual PRs were send to this branch.

pmeier added 18 commits January 26, 2022 15:36
* 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
* 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 functional transforms after review

* address features review
* 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
* 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
@facebook-github-bot
Copy link

facebook-github-bot commented Feb 11, 2022

💊 CI failures summary and remediations

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


  • 1/1 failures introduced in this PR

1 failure not recognized by patterns:

Job Step Action
CircleCI cmake_macos_cpu curl -o conda.sh https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh
sh conda.sh -b
source $HOME/miniconda3/bin/activate
conda install -yq conda-build cmake
packaging/build_cmake.sh
🔁 rerun

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.

@pmeier pmeier force-pushed the revamp-prototype-features-transforms branch from 1490767 to f795349 Compare February 11, 2022 10:10
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.

All changes up until f795349 commit are from PRs that were already reviewed. Any commit after needs to be reviewed.

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, let's merge when CI is green.

@datumbox datumbox merged commit d32bc4b into main Feb 11, 2022
@datumbox
Copy link
Contributor

This PR reimplements the New Transforms API and is the result of offline discussions with @pmeier and @vfdev-5. It's not the final adopted solution but the latest candidate. More ideas will be discussed soon.

Here I summarize the main point of this solution:

  • There are 2 types of kernels. The high-level ones (such as resize()) which can handle all types of input and the low-level ones (such as resize_image()) which handle specific types.
  • High-level kernels are located here. They receive inputs of various types (such Image, BoundingBox etc) and are responsible for dispatching to the correct low-level kernel by using the rich meta-data available on the custom types. Usually they are a thin layer that mostly maps types to operations but sometimes additional massaging is required prior calling the low-level kernel. High-level kernels are not JIT-scriptable.
  • Low-level kernels are located here. They are specialized for each type of input and operate on raw PyTorch Tensors (or PIL Images) and thus they require passing to them all the necessary meta-data required to perform the calculations. The low-level kernels are JIT-scriptable and can be used by downstream libraries without requiring using the proposed Feature types of TorchVision.
  • The dispatching is currently performed a simple annotation mechanism. This targets to remove most of the boilerplate code for 90% of simple cases that just map an input to a kernel. Thus in most cases, the main body of the high-level kernel is omitted. Nevertheless, there are some cases where extra massaging is needed for specific types. When this is needed, we pass None to the mapping and the actual handling of the dispatch happens on the body of the kernel.
  • It's worth noting that kernels of the same operation (such as resize) might require different input parameters for different types (example 1 vs 2). Thus currently, the dispatcher just receives args/kwargs and forwards them to the low-level kernels. Occasionally we enhance these parameters with available meta-data to avoid explicitly requiring them (TBD if overwriting meta-data will be allowed). In the documentation, we link to the low-level kernels to show which parameters are needed for the operation.

@pmeier pmeier deleted the revamp-prototype-features-transforms branch February 11, 2022 14:50
facebook-github-bot pushed a commit that referenced this pull request Feb 16, 2022
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
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