-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Move optimizer creation after device placement for ddp backends. #2904
Move optimizer creation after device placement for ddp backends. #2904
Conversation
Seems like the tests that previously passed still work. I did some digging into why On one of the #GPU>2 machines I have access to, this test fails (without this PR) with |
this is awesome. can we get the GPU tests to pass? |
it is an unrelated issue, as the failing case is happening because we set too high test acc value and sometimes it just did not reach it... it shall be fixed with setting fixed seed @awaelchli |
There is a |
Codecov Report
@@ Coverage Diff @@
## master #2904 +/- ##
=======================================
+ Coverage 89% 90% +1%
=======================================
Files 80 80
Lines 7531 7825 +294
=======================================
+ Hits 6738 7041 +303
+ Misses 793 784 -9 |
I say it already several times this week on multiple PRs, kind of randomly... |
Hm, how would I go about fixing the codecov/patch? I'm not familiar with this tool and don't find the details link too helpful ;) |
What does this PR do?
This PR moves the optimizer creation after the device placements for ddp backends. This allows the user to access the actual parameters used for training when creating the optimizers in the
configure_optimizers
function.See discussion in #2902.
Fixes #2902.
Before submitting
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 🙃