-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Deprecate legacy low level optimizers #11458
Comments
I'm generally ok with removing optimizers if we think they aren't useful for performance. I'd want to verify that things like In general, I don't have conviction that this is a bad idea, so no blockers from me (but I can't say if it's a good idea with any confidence) |
Also, it's probably worth checking with a few of the downstream collections like xarray, rapids, geopandas |
With #11525 the first couple low level optimizers are removed from the default optimization chain for arrays. Once we remove the legacy dataframe I believe most of the low level optimizers are dead |
We have a scenario where two delayed stages which we want to combine/fuse, so we were exploring how two levels can be fused together. If low level optimisers are going to be deprecated, what are the other ways of achieving similar optimisations like task fusion. |
We added an option for this a few releases ago: dask.config.set({"optimization.fuse.delayed": True}) Could you try this? |
We're offering some functionality for low level task graph rewrites/optimizations, see https://docs.dask.org/en/stable/optimize.html
I propose to deprecate and remove most, if not all, of this functionality. With #9969 most of this will break. Some/all of this could be migrated but I am not entirely convinced this is worth it.
Explicitly, I propose to remove
(Re)Moving these things also means that magical kwargs to compute like
fuse_keys
,fast_functions
,inline_functions_fast_functions
would vanish.For now, we'd likely keep the fuse functions but keep it pretty much internal. It is currently only executed for the array code so keeping it there in place still makes sense (for now) but I don't believe this is a valid function to expose.
I'm a bit on the fence regarding custom optimizers, e.g. the possibility to register an optimizer function with something like
dask.config.set(array_optimize=my_optimizer)
, see also https://docs.dask.org/en/stable/optimize.html#customizing-optimization I suspect there are very few power users even capable of writing this kind of code. It will certainly break and I'm not entirely convinced this is worth restoring.cc @mrocklin in case you have insights or an opinion on any of this
The text was updated successfully, but these errors were encountered: