-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support for duck Dask Arrays #4208
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
Comments
Maybe something like this would work?
|
That would be a straightforward solution to both problems! A Pint Quantity containing a Dask Array passes along the chunks attribute from the Dask Array, and a Pint Quantity containing something else will raise an AttributeError. Unless there are other objections, I'll see what it will take to swap out the existing Dask checks for this in the xarray internals and hopefully get around to a PR (after I get some MetPy stuff done first). |
It might also make sense to check for one or more of the special dask collection attributes ( |
I think there are already enough headaches with
We should ask the dask team to formalize what defines a "dask-array-like", like they already did with dask collections, and implement their definition in xarray. |
Since Pint wraps Dask, in order to leverage Dask Array functionality on Pint Quantities, we need to have the Dask collection interface available. In a sense, Pint needs special handling for Dask like xarray Variables do since they both can be upcast types of Dask Array. Implicitly passing through attributes (how Pint handles special methods/attributes of downcast types in general) from the wrapped Dask Array is not sufficient, however, because the finalizers have to rewrap with Quantity (see https://github.com/hgrecco/pint/pull/1129/files#diff-d9924213798d0fc092b8cff13928d747R1947-R1950), hence the explicit awareness of Dask being needed in Pint.
Done! See dask/dask#6385. |
Based on @mrocklin's comment in dask/dask#6385, the plan will be to check for duck Dask Arrays with |
I can go ahead with putting together a PR for this. Before I do so, I'd like to clarify what is expected.
I searched for existing duck array checks in xarray and nothing immediately obvious to me is showing up. It looks like a check for |
We have https://github.com/pydata/xarray/blob/master/xarray/core/pycompat.py which defines re duck array check: @keewis added this recently Lines 250 to 253 in f3ca63a
|
@rpmanser That sounds like a good plan to me at least, but it would be great if any of the xarray maintainers would be able to chime in. Also, thanks again for being willing to work on this while I try working on dask/dask#4583. The hidden 4th step is of course testing--primarily that this doesn't break existing functionality, but also that it works for duck Dask Arrays other than Dask Arrays themselves (not sure if Pint Quantities in upcoming v0.15 or a mocked class would be better). Also, thank you @dcherian for pointing out those checks, you found them faster than I did! |
Does/should any of this also consider #4212 (CuPy)? |
Only indirectly, since this deals with duck Dask arrays (things like Pint that go between xarray and Dask) rather than Dask chunks, which CuPy would be. But, once this, #4212, hgrecco/pint#964, and dask/dask#6393 are all in place, then we can test if xarray( pint( dask( cupy ))) works automatically from it all or not. |
A couple of things came up in #4221
|
In Xarray we implemented the Dask collection spec. https://docs.dask.org/en/latest/custom-collections.html#the-dask-collection-interface We might want to do that with Pint as well, if they're going to contain Dask things. That way Dask operations like |
My guess is that we could steal the xarray.DataArray implementations over to Pint without causing harm. |
That's exactly what's been done in Pint (see hgrecco/pint#1129)! @dcherian's points go beyond just that and address what Pint hasn't covered yet through the standard collection interface. |
Ah, great. My bad.
I think that you would want to make a pint array rechunk method that called down to the dask array rechunk method. My guess is that this might come up in other situations as well.
I think that implementing the It's also possible that we could look at the |
Re:rechunk, this should be part of the spec I guess. We need this for xarray does do some automatic rechunking in
suggest that we could delete that automatic rechunking today.
ah yes, we can rely on the underlying array library to optimize this. |
Dask collections tokenize quickly. We just use the name I think. |
* Change isinstance checks to duck Dask Array checks #4208 * Use is_dask_collection in is_duck_dask_array * Use is_dask_collection in is_duck_dask_array * Revert to isinstance checks according to review discussion * Move is_duck_dask_array to pycompat.py and use tokenize for comparisons * isort * Implement `is_duck_array` to replace `is_array_like` * Rename `is_array_like` to `is_duck_array` * `is_duck_array` checks for `__array_function__` and `__array_ufunc__` in addition to previous checks * Replace checks for `is_duck_dask_array` and `__array_function__` with `is_duck_array` * Skip numpy duck array tests when NEP18 is not active * Use utils.is_duck_array in xarray/core/formatting.py * Replace locally defined `is_duck_array` in _diff_mapping_repr * Replace `"__array_function__"` and `is_duck_dask_array` check in `short_data_repr` * Revert back to isinstance check for iris cube * Add is_duck_array_or_ndarray function to utils * Use is_duck_array_or_ndarray for duck array checks without NEP18 * Remove is_duck_dask_array_or_ndarray, replace checks with is_duck_array * Add explicit check for NumPy array to is_duck_array * Replace is_duck_array_or_ndarray checks with is_duck_array * Remove is_duck_array check for deep copy Co-authored-by: keewis <[email protected]> * Use is_duck_array check in load * Move duck dask array tokenize tests from test_units.py to test_dask.py * Use _importorskip to require pint >=0.15 instead of pytest.mark.skipif Co-authored-by: Deepak Cherian <[email protected]> Co-authored-by: keewis <[email protected]>
#525 (comment) raised the idea of adding "duck Dask Array" support to xarray as a way to handle xarray > Pint Quantity > Dask Array wrapping in a way that still allowed for most of xarray's Dask integration to work properly. With @rpmanser working on implementing the Dask collection interface in Pint (hgrecco/pint#1129), I thought it best to elevate this to its own issue to track progress and discuss implementation on xarray's side (since hopefully @rpmanser or I can get started on it soon).
Two initial (and intertwined) discussion points that I'd like to bring up (xref hgrecco/pint#1129 (comment)):
cc @keewis, @shoyer, @crusaderky
The text was updated successfully, but these errors were encountered: