-
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
Added getstate/setstate method for torch.save serialization #4127
Conversation
…ditional Optional Typing to results object
Codecov Report
@@ Coverage Diff @@
## master #4127 +/- ##
=======================================
+ Coverage 89% 92% +3%
=======================================
Files 103 103
Lines 7781 7792 +11
=======================================
+ Hits 6919 7147 +228
+ Misses 862 645 -217 |
Hello @SeanNaren! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-10-13 20:22:57 UTC |
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, just minor srestructure :]
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.
seems like some tests are hanging
… epochs for speed
|
||
|
||
@pytest.mark.skipif(torch.cuda.device_count() < 2, reason="test requires multi-GPU machine") | ||
def test_model_torch_save_ddp_cuda(tmpdir): |
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.
seems like this guy is hanging...
What does this PR do?
Fixes #4114 and Fixes #4120
Still requires test, and to see if there is a less verbose way to fix serialization in the accelerator class.
Also note that this will omit any additional state variables of accelerators that inherit from this; it assumes custom accelerator will override
__getstate__
and__setstate__
, or ensure that other objects are setup again.PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃