-
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
set device to root gpu in single GPU case #3042
Conversation
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 😄
If possible, mind adding quick test for this? Approving so we don't block this bugfix either way.
@awaelchli weren't the random failing tests fixed? |
@ananyahjha93 I think a simple test would be to check |
@ananyahjha93 I thought so. Can we rerun the CI to check if it's the same accuracy value. I want to see if it's deterministic. |
@awaelchli should pass the GPU tests, the failure was ddp_spawn test on the accuracy metric. |
the test passes now in a second run, which means despite setting the deterministic flag in the Trainer and seed_everything(1234), something in that test is still random. |
Codecov Report
@@ Coverage Diff @@
## master #3042 +/- ##
======================================
- Coverage 90% 90% -0%
======================================
Files 81 81
Lines 7672 7685 +13
======================================
+ Hits 6906 6912 +6
- Misses 766 773 +7 |
still no test, right? |
Fixes #3030
are there suggested tests for this PR?