-
Notifications
You must be signed in to change notification settings - Fork 423
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
New argument compute_on_cpu
#867
Conversation
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #867 +/- ##
=======================================
+ Coverage 70% 95% +25%
=======================================
Files 175 175
Lines 7468 7476 +8
=======================================
+ Hits 5249 7102 +1853
+ Misses 2219 374 -1845 |
…htning/metrics into features/store_on_cpu
A few questions:
|
@maximsch2 I think the device should indicate the input and output device of a metric. E.g. even if the metic states are on cpu, it would require the devices of the inputs to be cuda and handle the device-transfer logic accordingly and the output would also be on gpu. |
@SkafteNicki mind resolve conflicts 🐰 |
@SkafteNicki @justusschock pls ^^ 🐰 |
seems there is a DDP issue... :/ |
@Borda all unrelated to the PR, trying to run tests again :] |
What does this PR do?
Rework of #851
Fixes #848
General solution that allows all metrics where the metric state is a list to store the state on CPU instead of GPU through a new keyword argument
compute_on_cpu=True/False
(defaultFalse
). This can beneficial in situations where the user do not have that much VRAM. Consequence of enabling this feature is that code incompute
will be executed on CPU (code inupdate
will still be executed on GPU).Fixes #866
Related as the MAP metric currently moves all metric states to CPU for faster computations. As the issue points out this can overload the CPU. Feature in this PR essentially also fixes this by making it an option to move to CPU by setting
MeanAveragePrecision(compute_on_cpu=True)
.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 🙃