-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Adapt Dask on Ray to the new Dask Task class #52589
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Hiromu Hota <hiromu.hota@gmail.com>
Since CI doesn't run unit tests with a newer version of dask, this is the result of unit tests that I locally ran.
|
d1aae96
to
0947309
Compare
What do you think is the right approach here? At first glance, it seems to make sense to optimize for the latest version. |
I think it'd be safer for Dask on Ray users to phase the migration because of the changes in Dask side:
So, I'd suggest to split the migration into two: ship this PR as is, wait Ray to be released at least once (e.g., 2.44.2), and then do another migration without #13951 to be released in a later version of Ray. This way, it'd be easier to identify the root cause if some issue happens. |
Just my own two cents, but I think that Dask-on-ray has been in a quasi maintained state long enough that I don't think many (or any?) are using it in production. So I think it is safe to optimize for the latest version. Selfishly, we are keen to use the latest version of Dask and try Dask on Ray with it. There's been some crucial optimizations made after 2025.1.0 https://docs.dask.org/en/stable/changelog.html#reducing-memory-pressure-for-xarray-workloads However I'm just happy that this is seeing some activity again and looking forward to trying whenever it is available. |
Signed-off-by: Hiromu Hota <hiromu.hota@gmail.com>
I made a further change to support dask>=2025.1.0.
|
Why are these changes needed?
"Dask on Ray" (DoR) is broken in dask==2024.11.0 or later as reported in #48689 because Dask removed a private function in dask/dask#11378 that DoR has been relying on. Not only dask/dask#11378, Dask has migrated their task data structure to a new format (the high-level motivation is described in dask/dask#9969). Since this migration spans across a series of PRs between 2024.11.0 and 2025.1.0, it's not realistic to copy what's been removed and paste them in Ray.
This PR adapts Dask on Ray to the change to keep its functionality.
The current PR as of 7c9def3 is compatible only with
dask>=2024.11.0,<2025.1.0
because Dask made another major change in 2025.1.0, breaking the shuffle optimization introduced in #13951.I can make further changes in this PR if we want to support
dask>=2025.1.0
and drop supportdask<2025.1.0
. Supporting both would technically be possible but that would make codebase complicated. Let me know how Ray would like to approach this.Related issue number
Closes #48689
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.