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

Deprecate {train/val/test}_transforms, dims, and size from the DataModule #8728

Closed
ananthsub opened this issue Aug 5, 2021 · 4 comments · Fixed by #8851
Closed

Deprecate {train/val/test}_transforms, dims, and size from the DataModule #8728

ananthsub opened this issue Aug 5, 2021 · 4 comments · Fixed by #8851
Assignees
Labels
data handling Generic data-related topic design Includes a design discussion feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on

Comments

@ananthsub
Copy link
Contributor

🚀 Feature

Deprecate these properties off the LightningDataModule interface:
https://github.com/PyTorchLightning/pytorch-lightning/blob/963c26764682fa4cf64c93c5a7572ae0040e9c32/pytorch_lightning/core/datamodule.py#L94-L147

Motivation

We are auditing the Lightning components and APIs to assess opportunities for improvements:

train_transforms, val_transforms, test_transforms, dims, and size are entirely optional to use, yet they’re on the DataModule interface. The Trainer does not rely on these, nor is the user forced to implement them. In reality, these are internal implementation details of individual datamodules. As a result, we ought to deprecate these off the DataModule interface.

Of course, users retain the ability to implement these properties in their LightningModules if they find these abstractions helpful.

Pitch

We can follow a similar approach as what was done for #7301

  • Mark the properties as deprecated in v1.5. Warn if users are passing non-None values to the DataModule constructor and if they read/set their corresponding properties
  • Remove the properties in v1.7

Alternatives

Keep as is

Additional context


If you enjoy Lightning, check out our other projects! ⚡

  • Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

  • Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, finetuning and solving problems with deep learning

  • Bolts: Pretrained SOTA Deep Learning models, callbacks and more for research and production with PyTorch Lightning and PyTorch

  • Lightning Transformers: Flexible interface for high performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.

@ananthsub ananthsub added feature Is an improvement or enhancement help wanted Open to be worked on data handling Generic data-related topic design Includes a design discussion labels Aug 5, 2021
@ananthsub ananthsub self-assigned this Aug 5, 2021
@ananthsub ananthsub added the good first issue Good for newcomers label Aug 5, 2021
Tshimanga added a commit to Tshimanga/pytorch-lightning that referenced this issue Aug 11, 2021
…tamodule methods: train_transforms, val_transforms, test_transforms, dums, and size
@Tshimanga
Copy link
Contributor

@ananthsub I've submitted a PR for review that adds the v1.5 deprecation warnings. I didn't find any tests that needed to be (re)moved. By the way, is there a reason why rank_zero_deprecation isn't a decorator?

@ananthsub
Copy link
Contributor Author

By the way, is there a reason why rank_zero_deprecation isn't a decorator?

rank_zero_deprecation can be used within functions. It doesn't always have to wrap a function to print a warning. Something like rank_zero_only is a decorator as it applies to the function as a whole.

Tshimanga added a commit to Tshimanga/pytorch-lightning that referenced this issue Aug 11, 2021
@ajdillhoff
Copy link

If these are being deprecated, what will be the best practice for modifying transforms on datasets coming from pl_bolts?

https://lightning-bolts.readthedocs.io/en/latest/datamodules/vision.html

@akihironitta
Copy link
Contributor

akihironitta commented Apr 9, 2022

Hi @ajdillhoff, I believe it's still free for users to define the methods. They're just deprecated and will be removed from the base class. If you have a further issue/question, please use GitHub Discussions: https://github.com/PyTorchLightning/pytorch-lightning/discussions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data handling Generic data-related topic design Includes a design discussion feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on
Projects
None yet
4 participants