-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Introduce Stateful
DataModule
#11637
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a follow up, please deprecate CheckpointHooks off the DataModule interface: https://github.com/PyTorchLightning/pytorch-lightning/blob/86b177ebe5427725b35fde1a8808a7b59b8a277a/pytorch_lightning/core/datamodule.py#L26
It is no longer needed following the Stateful change and should be removed in v1.8
Did we want to deprecate |
I should have clarified that we can follow it up in a separate issue/PR. It is not a blocker for this one. I initially proposed using the CheckpointHooks to DataModule back in #3544 . It was to capture dataloader state that had proprietary APIs. I wish at the time I had recognized the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
What does this PR do?
part of / Fixes #11429: introduce
Stateful
DataModule (see Appendix code changes outlined in issue)Does your PR introduce any breaking changes? If yes, please list them.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃