-
Notifications
You must be signed in to change notification settings - Fork 93
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
Unify device_memory_limit
parsing and set default to 0.8
#439
Unify device_memory_limit
parsing and set default to 0.8
#439
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.
Thank you @pentschev . Should we also update documentation ?
I mean, do you want to call out the default setting in the docs? I'm happy to do this if you'd like |
Are you referring to some docstrings or https://dask-cuda.readthedocs.io/en/latest/ ? |
Sure, we can update that next week. I just found out that we can't mix types with |
Based off of distributed.worker.parse_memory_limit
Hopefully this is fixed in latest commits. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #439 +/- ##
===============================================
+ Coverage 90.57% 90.79% +0.21%
===============================================
Files 14 14
Lines 796 804 +8
===============================================
+ Hits 721 730 +9
+ Misses 75 74 -1
Continue to review full report at Codecov.
|
Co-authored-by: Benjamin Zaitlen <[email protected]>
Thanks @pentschev ! |
This should address the request to set a default value for
device_memory_limit
, as well as allowing fractions and unifying howLocalCUDACluster
anddask-cuda-worker
parse its value. Fixes #334 .cc @beckernick @rjzamora