You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Polyfit performance on large datasets - Suboptimal dask task graph #5629
I am trying to calculate a linear trend over a large climate model simulation. I use rechunker to chunk the data along the horizontal dimensions and make sure that the time dimension (along which I want to calculate the fit) is not chunked.
In my realistic example, this blows up the memory of my workers.
What you expected to happen:
I expected this to work very smoothly because it should be embarassingly parallel (no information of sourrounding chunks is needed and the time dimension is complete in each chunk).
Minimal Complete Verifiable Example:
I think this minimal example shows that the task graph created is not ideal
importxarrayasxrimportdask.arrayasdsada=xr.DataArray(dsa.random.random((4,6, 100), chunks=(1,2,100)), dims=['x','y', 'time'])
da
Note that the original data array has 12 chunks
When I apply polyfit I get this
poly=da.polyfit('time', 1)
poly
Now the number of chunks has decreased to 4? I am not sure why, but this indicates to me that my problem might be related to #4554.
When I look at the task graph it seems that this explains why for very large dataset the computation blows up:
As I said before I would expect this calculation to be fully parallelizable, but there seems to be some aggregation/rechunking steps in the bottom layer. How 'bad' these get (e.g. how many input chunks get lumped together in the rechunk/merge step, depends on the chunking structure of the horizontal axes.
It's the same thing: stack is a reshape which can be very inefficient with dask. I think we want to replace the stack+apply_along_axis approach with xr.apply_ufunc(np.polyfit, ..., vectorize=True).
Thanks for the clear example @jbusecke . I think your example is helpful enough that we should keep this open (even if the same PR will hopefully close both this and #4554)
Was about to say the same @dcherian! @aulemahal what do you think about how easy it might be to change polyfit to use apply_ufunc?
Potentially a useful comparison: here is the task graph for the same problem posed to da.curvefit, which uses xr.apply_ufunc(scipy.optimize.curvefit, ..., vectorize=True). Looks a lot cleaner, and I think xr.polyfit could be re-implemented this way without much trouble.
importxarrayasxrimportdask.arrayasdsadeflinear(x, m, b):
returnm*x+bda=xr.DataArray(dsa.random.random((4,6, 100), chunks=(1,2,100)), dims=['x','y', 'time'])
fit=da.curvefit('time', linear).curvefit_coefficients
Reacted by Tom Nicholas, Deepak Cherian and Julius Busecke
@aulemahal what do you think about how easy it might be to change polyfit to use apply_ufunc?
Should be easy! The code was written without apply_ufunc for a few reasons, one being that using apply_ufunc+vectorize was slower than the apply_along_axis trick it currently uses. But, with the issue raised here, that might not be true...
apply_ufunc was refactored to use dask.array.apply_gufunc (which moves the vectorization to dask) after implementation of polyfit.
So those concerns about slowness of apply_ufunc + vectorization where based on the formerly used _apply_blockwise approach and might not be an issue any more with the current implementation of apply_ufunc .
In reading through #5933, I realized there's an alternate solution that preserves the desirable feature of fitting along chunked dimensions (at least for skipna=False)
Since we are only ever fitting along one dimension, we can apply the reshape-other-dimensions operation blockwise. As long as we reshape back in a consistent manner (again blockwise) this is identical to our current stacking approach, but substantially more efficient. In particular, it avoids merging chunks.
defreshape_block(arr, output_chunks):
""" Blockwise reshape array to match output_chunks This method applies a reshape operation blockwise, so it only makes sense if you do not care about the order of elements (along reshaped axes) after reshaping. The order of elements will depend on the chunking of ``arr``. It is substantially more efficient than a usual reshape, which makes sure array elements appear in a numpy-like order regardless of chunking. Based on https://github.com/dask/dask/issues/4855#issuecomment-497062002 """fromitertoolsimportproductimportnumpyasnpfromdask.arrayimportArrayfromdask.baseimporttokenizefromdask.highlevelgraphimportHighLevelGraphname="reshape-block-"+tokenize(arr)
ichunks=tuple(range(len(chunks_v)) forchunks_vinarr.chunks)
ochunks=tuple(range(len(chunks_v)) forchunks_vinoutput_chunks)
dsk= {
(name, *ochunk): (np.reshape, (arr.name, *ichunk), oshape)
forichunk, ochunk, oshapeinzip(
product(*ichunks), product(*ochunks), product(*output_chunks)
)
}
graph=HighLevelGraph.from_collections(name, dsk, dependencies=[arr])
res=Array(graph, name, output_chunks, arr.dtype, meta=arr._meta)
returnresdefpolyfit(da, dim, skipna=False):
importitertoolsimportnumpyasnpda=da.transpose(..., dim)
arr=da.datado_blockwise_reshape=arr.ndim>1andany(
len(chunks) >1forchunksinarr.chunks
)
ifdo_blockwise_reshape:
output_chunks= (tuple(map(np.product, product(*arr.chunks[:-1]))),) + (
arr.chunks[-1],
)
other_dims=tuple(dim_fordim_inda.dimsifdim_!=dim)
reshaped=xr.DataArray(
reshape_block(arr, output_chunks), dims=("__reshaped__", dim)
)
else:
reshaped=dafit=reshaped.polyfit(dim, 1, skipna=skipna)
ifdo_blockwise_reshape:
result=xr.Dataset()
forvarinfit:
result[var] = (
("degree",) +other_dims,
reshape_block(fit[var].data, ((2,),) +arr.chunks[:-1]),
)
fordim_inother_dims:
result[dim_] =da[dim_]
result["degree"] =fit["degree"]
returnresultelse:
returnfit
Here's the graph 1 chunk along time (like @jbusecke's example, and skipna=True)
And when the time dimension is chunked (and skipna=False)
older version
Details
fromitertoolsimportproductimportnumpyasnpfromdask.arrayimportArrayfromdask.array.utilsimportassert_eqfromdask.baseimporttokenizefromdask.highlevelgraphimportHighLevelGraphdefreshape_block(arr, output_chunks):
""" Blockwise reshape array to match output_chunks This method applies a reshape operation blockwise, so it only makes sense if you do not care about the order of elements (along reshaped axes) after reshaping. The order of elements will depend on the chunking of ``arr``. It is substantially more efficient than a usual reshape, which makes sure array elements appear in a numpy-like order regardless of chunking. Based on https://github.com/dask/dask/issues/4855#issuecomment-497062002 """name="reshape-block-"+tokenize(arr)
ichunks=tuple(range(len(chunks_v)) forchunks_vinarr.chunks)
ochunks=tuple(range(len(chunks_v)) forchunks_vinoutput_chunks)
dsk= {
(name, *ochunk): (np.reshape, (arr.name, *ichunk), oshape)
forichunk, ochunk, oshapeinzip(
product(*ichunks), product(*ochunks), product(*output_chunks)
)
}
graph=HighLevelGraph.from_collections(name, dsk, dependencies=[arr])
res=Array(graph, name, output_chunks, arr.dtype, meta=arr._meta)
returnres# assumes we're collapsing axes (0,1) and preserving axis 2output_chunks= (tuple(map(np.product, product(*arr.chunks[:2]))),) + (arr.chunks[-1],)
reshaped=reshape_block(arr, output_chunks)
# make sure that we roundtripactual=reshape_block(reshaped, arr.chunks)
assert_eq(actual, arr) # True
The problem is that there is no general solution here. You have to write a chunk-aware function and then use apply_ufunc(..., dask="allowed"). This suggestion only works because dask's lstsq can work with chunked dimensions.
The problem is that there is no general solution here. You have to write a chunk-aware function and then use apply_ufunc(..., dask="allowed"). This suggestion only works because dask's lstsq can work with chunked dimensions.
Sorry, what case does your suggestion not handle? It would be really nice to support chunked core dims if we are going to the effort of rewriting polyfit.
Is that something that could be implemented in apply_ufunc?
I interpreted this as asking whether this general idea could be implemented in apply-ufunc, to which the answer is "no" (I think) because it's pretty specific. At best we put this logic in a wrapper function and call that with apply_ufunc(..., dask="allowed").
This should handle all current use-cases of polyfit, we just change the call to stack.
I was thinking the general idea of reshape_block seems like a clever trick to get around the apply_ufunc constraints and could be used for some other functions as well. But maybe there aren't that many functions that could make use of it.
I was thinking the general idea of reshape_block seems like a clever trick to get around the apply_ufunc constraints and could be used for some other functions as well. But maybe there aren't that many functions that could make use of it.
The idea seems similar to what we used in xhistogram, which uses blockwise to apply a function that reshapes the data in that block to be 1D.
EDIT: But in that case our algorithm was one that could be applied blockwise, apply_ufunc was not used.
a clever trick to get around the apply_ufunc constraints
To clarify, the constraint here is that lstsq expects 2D arrays, that's why the reshaping was needed in the first place.
apply_ufunc(..., dask="parallelized") automatically applies your dask-unaware function blockwise which only makes sense if there is a single chunk along the core dimension, hence that constraint.
apply_ufunc(..., dask="allowed") has no constraints, your function has to know how to deal with chunked data though.
uses blockwise to apply a function that reshapes the data in that block to be 1D.
Yes, same idea. Though when you do it that way, you have to implement the function you're applying (that xhistogram PR reimplements dask.array.bincount). Reimplementing lstsq is a lot more complicated, hence this approach.
Very nice! yeah I think @jbusecke's example is the case where the "blockwise reshaping" isn't really needed because the outer most axis has chunk size 1.
let's leave it open for the more general solution.
Activity
dcherian commentedon Jul 22, 2021
It's the same thing:
stackis a reshape which can be very inefficient with dask. I think we want to replace the stack+apply_along_axis approach withxr.apply_ufunc(np.polyfit, ..., vectorize=True).jbusecke commentedon Jul 22, 2021
Should I close this one then?
TomNicholas commentedon Jul 22, 2021
Thanks for the clear example @jbusecke . I think your example is helpful enough that we should keep this open (even if the same PR will hopefully close both this and #4554)
Was about to say the same @dcherian! @aulemahal what do you think about how easy it might be to change
polyfitto useapply_ufunc?slevang commentedon Jul 22, 2021
Potentially a useful comparison: here is the task graph for the same problem posed to
da.curvefit, which usesxr.apply_ufunc(scipy.optimize.curvefit, ..., vectorize=True). Looks a lot cleaner, and I thinkxr.polyfitcould be re-implemented this way without much trouble.aulemahal commentedon Jul 26, 2021
Should be easy! The code was written without apply_ufunc for a few reasons, one being that using
apply_ufunc+vectorizewas slower than theapply_along_axistrick it currently uses. But, with the issue raised here, that might not be true...dcherian commentedon Jul 26, 2021
You could write a wrapper that does
reshape+apply_along_axisand call that wrapper inapply_ufuncwithoutvectorizekmuehlbauer commentedon Jul 27, 2021
apply_ufuncwas refactored to usedask.array.apply_gufunc(which moves the vectorization to dask) after implementation ofpolyfit.So those concerns about slowness of
apply_ufunc+ vectorization where based on the formerly used_apply_blockwiseapproach and might not be an issue any more with the current implementation ofapply_ufunc.dcherian commentedon Nov 3, 2021
In reading through #5933, I realized there's an alternate solution that preserves the desirable feature of fitting along chunked dimensions (at least for skipna=False)
Since we are only ever fitting along one dimension, we can apply the reshape-other-dimensions operation blockwise. As long as we reshape back in a consistent manner (again blockwise) this is identical to our current stacking approach, but substantially more efficient. In particular, it avoids merging chunks.
Here's the graph 1 chunk along time (like @jbusecke's example, and skipna=True)

And when the time dimension is chunked (and skipna=False)
older version
Details
Illviljan commentedon Nov 4, 2021
@dcherian Is that something that could be implemented in apply_ufunc? Not being able to apply along a chunked dim I think happens quite often, right?
dcherian commentedon Nov 4, 2021
The problem is that there is no general solution here. You have to write a chunk-aware function and then use
apply_ufunc(..., dask="allowed"). This suggestion only works because dask'slstsqcan work with chunked dimensions.TomNicholas commentedon Nov 4, 2021
Sorry, what case does your suggestion not handle? It would be really nice to support chunked core dims if we are going to the effort of rewriting polyfit.
dcherian commentedon Nov 4, 2021
I interpreted this as asking whether this general idea could be implemented in apply-ufunc, to which the answer is "no" (I think) because it's pretty specific. At best we put this logic in a wrapper function and call that with
apply_ufunc(..., dask="allowed").This should handle all current use-cases of
polyfit, we just change the call tostack.Illviljan commentedon Nov 4, 2021
I was thinking the general idea of reshape_block seems like a clever trick to get around the apply_ufunc constraints and could be used for some other functions as well. But maybe there aren't that many functions that could make use of it.
TomNicholas commentedon Nov 4, 2021
The idea seems similar to what we used in xhistogram, which uses blockwise to apply a function that reshapes the data in that block to be 1D.
EDIT: But in that case our algorithm was one that could be applied blockwise,
apply_ufuncwas not used.dcherian commentedon Nov 4, 2021
To clarify, the constraint here is that
lstsqexpects 2D arrays, that's why the reshaping was needed in the first place.apply_ufunc(..., dask="parallelized")automatically applies your dask-unaware function blockwise which only makes sense if there is a single chunk along the core dimension, hence that constraint.apply_ufunc(..., dask="allowed")has no constraints, your function has to know how to deal with chunked data though.Yes, same idea. Though when you do it that way, you have to implement the function you're applying (that xhistogram PR reimplements
dask.array.bincount). Reimplementinglstsqis a lot more complicated, hence this approach.phofl commentedon Aug 14, 2024
The explosion in chunk sizes will be fixed in the next dask release, task graph is still relatively complex though
phofl commentedon Aug 19, 2024
Looking at this again, I was incorrect. Here is the graph of the current implementation:
This is basically embarrassingly parallel, so I think we can close this one?
dcherian commentedon Aug 19, 2024
Very nice! yeah I think @jbusecke's example is the case where the "blockwise reshaping" isn't really needed because the outer most axis has chunk size 1.
let's leave it open for the more general solution.
Optimize polyfit
Optimize polyfit
Optimize polyfit
Optimize polyfit (#9766)