-
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
[feat] Add restore to base loop #8247
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8247 +/- ##
=======================================
- Coverage 93% 88% -5%
=======================================
Files 212 212
Lines 13695 13710 +15
=======================================
- Hits 12735 12068 -667
- Misses 960 1642 +682 |
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.
- what's the difference between this and load_state_dict?
- why do we need the is_restarting boolean?
On restart, loading the state dict isn't enough, you would need to do multiple things depending on your loop such as fast forwarding samplers, skip progress tracking reset, etc... Linked to this PR: #8131 Best, |
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!
self._restarting = False | ||
|
||
@property | ||
def restarting(self) -> bool: | ||
return self._restarting | ||
|
||
@restarting.setter | ||
def restarting(self, restarting: bool) -> None: | ||
self._restarting = restarting |
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.
Is there a reason why you added a getter/setter considering they don't do anything custom?
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.
Not a strong need for them right now. Can be removed later on.
What does this PR do?
This PR adds restore to base loop and is_restarting boolean.
Fixes #<issue_number>
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 🙃