-
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
Support Mean in DDP Sync #2568
Support Mean in DDP Sync #2568
Conversation
Hello @justusschock! Thanks for updating this PR.
Comment last updated at 2020-08-04 15:57:36 UTC |
Codecov Report
@@ Coverage Diff @@
## master #2568 +/- ##
=======================================
- Coverage 88% 86% -3%
=======================================
Files 78 78
Lines 7069 6921 -148
=======================================
- Hits 6250 5932 -318
- Misses 819 989 +170 |
@Borda are TPU tests still to be neglected? Edit: Also the failing GPU Test seems unrelated to the metrics (which is the only thing I touched here) |
TPU shall be fine... EDIT: ok we have some problem with TPU tests :{ |
1a724e5
to
d2cfd16
Compare
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 @justusschock, this will be very useful
it seems that the test acc dropped significantly...
|
@Borda @williamFalcon @SkafteNicki can we get this done? |
@SkafteNicki you're right. This is fixed now. |
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?
Add support for mean in DDP Sync
Fixes # (issue)
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 🙃