-
Notifications
You must be signed in to change notification settings - Fork 94
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
Add arguments to enable cuDF spilling and set statistics #1362
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.
Thanks for this @pentschev ! Only one minor comment/question from me.
Side thought: We have a lot of options at this point. Do you think it makes sense to deprecate anything?
.. warning:: | ||
This should NOT be used together with JIT-Unspill. |
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.
Is there any way to detect if the user is doing this and warn/raise? This doc-string is good, but I'm sure some users will never read it :)
Warning added in d7ffa0a . @madsbk do you think we should just raise a warning or raise a warning and disable JIT-Unspill regardless of the user enabling both together, i.e.,
Probably @madsbk also has a better sense of that as well, do you have thoughts on this? |
I think we should raise a warning only.
I think we should wait a bit with deprecating until we get a good solution from the NO-OOM work. |
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.
Thanks @pentschev, looks good
/merge |
Add arguments to enable cuDF spilling and set statistics in
dask cuda worker
/LocalCUDACluster
. This is implemented as a Dask plugin, and does not require users anymore to rely onclient.run
to do that.Closes #1280