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

πŸš€ Replace albumentations with torchvision transforms #1706

Merged
merged 114 commits into from
Feb 29, 2024

Conversation

djdameln
Copy link
Contributor

@djdameln djdameln commented Feb 2, 2024

πŸ“ Description

PR for replacing albumentations transforms with torchvision transforms

✨ Changes

  • Use torchvision v2 transforms instead of albumentations transforms throughout the code base.
  • Load images as torch tensors instead of numpy images in dataset __getitem__ methods.
  • Use tv_tensors Mask format for GT masks
  • Transforms are included in the exported model. This is done by wrapping the transform and the torch model in the new InferenceModel class before exporting.
  • The datamodules take a Transform object as input to directly set the transform that is applied to the input images.
  • The model implementer can define a default model-specific transform using the configure_transforms method of the Lit model. When no transform is passed to the datamodule, this default transform will be used. The default model transform may use the image_size parameter from the datamodule to resize the images to the size specified by the user.

To be addressed:

  • Inferencers still use albumentations
  • Current design does not support model-specific transforms (i.e. defining a default transform from the lightning model).
  • Still need to implement the model-specific default transforms for most models. This can be done later as they now just use the default one from the base module.
  • Docstrings in the data classes need to be updated to reflect the new signatures.
  • Patchcore fails to export because of the CenterCrop transform. We may have to implement our own version of this transform to allow exporting.
  • Some models still have the input_size parameter, which needs to be removed.
  • CLI tests are failing because the export API has changed.
  • The ConfigAdapter needs to be updated to reflect the new datamodules API.

Select what type of change your PR is:

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • πŸ”¨ Refactor (non-breaking change which refactors the code base)
  • πŸš€ New feature (non-breaking change which adds functionality)
  • πŸ’₯ Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • πŸ“š Documentation update

βœ… Checklist

Before you submit your pull request, please make sure you have completed the following steps:

  • πŸ“‹ I have summarized my changes in the CHANGELOG and followed the guidelines for my type of change (skip for minor changes, documentation updates, and test enhancements).
  • πŸ“š I have made the necessary updates to the documentation (if applicable).
  • πŸ§ͺ I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).

Copy link
Contributor

@samet-akcay samet-akcay left a comment

Choose a reason for hiding this comment

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

Thanks for creating this pr. I've added my initial comments.. Will go for another round

src/anomalib/data/base/dataset.py Outdated Show resolved Hide resolved
src/anomalib/data/base/dataset.py Show resolved Hide resolved
src/anomalib/data/utils/transforms.py Show resolved Hide resolved
Copy link
Collaborator

@ashwinvaidya17 ashwinvaidya17 left a comment

Choose a reason for hiding this comment

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

I didn't run this yet but overall I like this over albumentations. It will be one less dependency.

src/anomalib/data/base/dataset.py Outdated Show resolved Hide resolved
src/anomalib/data/base/video.py Show resolved Hide resolved
src/anomalib/data/utils/augmenter.py Outdated Show resolved Hide resolved
src/anomalib/data/utils/transforms.py Outdated Show resolved Hide resolved
Copy link

Check out this pull request onΒ  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added Dependencies Pull requests that update a dependency file Notebooks labels Feb 28, 2024
Copy link
Contributor

@samet-akcay samet-akcay left a comment

Choose a reason for hiding this comment

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

Looking great! Thanks a lot!

@samet-akcay samet-akcay merged commit 80c0756 into openvinotoolkit:main Feb 29, 2024
7 checks passed
@djdameln djdameln deleted the torchvision-transforms-main branch December 4, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file Inference Notebooks Tests Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants