-
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
Fix Multi-GPU join for horovod #6954
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6954 +/- ##
=======================================
- Coverage 92% 87% -5%
=======================================
Files 194 194
Lines 12329 12333 +4
=======================================
- Hits 11325 10673 -652
- Misses 1004 1660 +656 |
Thanks for the fix <3 |
Thank you for the reply. I do not think it related to the skipped tests, but seems related to this one, wonder why the issue was not caught by this test before. |
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.
shall we reenable Horovod tests back?
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 ! Thanks for your contribution.
* fixjoin * fix join on cpu * fix typo * try to undo horovod skip * undo * Try removing skip * Update CHANGELOG * add back skip for test_horovod_multi_optimizer * Add back skip Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Carlos Mocholi <[email protected]>
* fixjoin * fix join on cpu * fix typo * try to undo horovod skip * undo * Try removing skip * Update CHANGELOG * add back skip for test_horovod_multi_optimizer * Add back skip Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Carlos Mocholi <[email protected]>
Horovd's join required local rank as argument for multi-gpu cases, code here
The default join only works for cpu.
Partly fixes #6935edit: does not seem like any skipped tests were fixed
Before submitting
PR review