-
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
[2 / 3] improvements to saving and loading callback state #7187
Conversation
Co-authored-by: Carlos Mocholí <[email protected]>
37244b5
to
e0559fe
Compare
Hello @awaelchli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-08-13 11:48:25 UTC |
Codecov Report
@@ Coverage Diff @@
## master #7187 +/- ##
=======================================
- Coverage 92% 88% -4%
=======================================
Files 178 178
Lines 14694 14702 +8
=======================================
- Hits 13526 12925 -601
- Misses 1168 1777 +609 |
f16af3a
to
77e7027
Compare
for more information, see https://pre-commit.ci
Co-authored-by: Jirka Borovec <[email protected]>
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.
for a followup, we should use the state_key to disambiguate callbacks here too: https://github.com/PyTorchLightning/pytorch-lightning/blob/f87b2ef21f01d7bbda666088b8cdae3dcbe12010/pytorch_lightning/trainer/connectors/callback_connector.py#L134-L159
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.
LGTM !
What does this PR do?
Fixes #6467
It is now possible to save and resume with multiple callbacks of the same time, provided they have a suitable
state_id
property that defines the equivalence class. See the added docs section for more information of how it works.This is now possible without sacrifice:
and resuming from an existing checkpoint:
will load the internal state of BOTH these callbacks, where previously this wouldn't work with stateful callbacks.
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
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 🙃