Do we need to update AbstractArray for duck arrays?

[dcherian]

What happened?

I'm calling cupy.round on a DataArray wrapping a cupy array and it raises an error here:

xarray/xarray/core/common.py

Lines 155 to 156 in 3f7cc2d

	def __array__(self: Any, dtype: DTypeLike = None) -> np.ndarray:
	return np.asarray(self.values, dtype=dtype)

Traceback below:

--> 25     a = _core.array(a, copy=False)
     26     return a.round(decimals, out=out)
     27 

cupy/_core/core.pyx in cupy._core.core.array()

cupy/_core/core.pyx in cupy._core.core.array()

cupy/_core/core.pyx in cupy._core.core._array_default()

~/miniconda3/envs/gpu/lib/python3.7/site-packages/xarray/core/common.py in __array__(self, dtype)
    146 
    147     def __array__(self: Any, dtype: DTypeLike = None) -> np.ndarray:
--> 148         return np.asarray(self.values, dtype=dtype)
    149 
    150     def __repr__(self) -> str:

~/miniconda3/envs/gpu/lib/python3.7/site-packages/xarray/core/dataarray.py in values(self)
    644         type does not support coercion like this (e.g. cupy).
    645         """
--> 646         return self.variable.values
    647 
    648     @values.setter

~/miniconda3/envs/gpu/lib/python3.7/site-packages/xarray/core/variable.py in values(self)
    517     def values(self):
    518         """The variable's data as a numpy.ndarray"""
--> 519         return _as_array_or_item(self._data)
    520 
    521     @values.setter

~/miniconda3/envs/gpu/lib/python3.7/site-packages/xarray/core/variable.py in _as_array_or_item(data)
    257     TODO: remove this (replace with np.asarray) once these issues are fixed
    258     """
--> 259     data = np.asarray(data)
    260     if data.ndim == 0:
    261         if data.dtype.kind == "M":

cupy/_core/core.pyx in cupy._core.core.ndarray.__array__()

TypeError: Implicit conversion to a NumPy array is not allowed. Please use `.get()` to construct a NumPy array explicitly.

What did you expect to happen?

Not an error? I'm not sure what's expected

np.round(dataarray) does actually work successfully.

My question is : Do we need to update AbstractArray.__array__ to return the underlying duck array instead of always a numpy array?

Minimal Complete Verifiable Example

No response

MVCE confirmation

• Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
• Complete example — the example is self-contained, including all data and the text of any traceback.
• Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
• New issue — a search of GitHub Issues suggests this is not a duplicate.

Relevant log output

No response

Anything else we need to know?

No response

Environment

xarray v2022.6.0

cupy 10.6.0

[Illviljan]

I believe so. The other ones that uses .values will fail as well with sparse.

[keewis]

Not sure, but maybe cupy.round is not supposed to be called on anything other than cupy (or numpy.ndarray)? If I understand correctly, it is converting everything to cupy (or numpy) using _core.array before actually doing the computation.

Also, my impression of __array__ is that it should only be used to convert custom objects to np.ndarray (usually using np.array or np.asarray)

[dcherian]

cupy.round is not supposed to be called on anything other than cupy (or numpy.ndarray

Yeah I'm not sure what the expectation is but I was calling cp.round on a DataArray that wrapped a cupy array. Which is why the __array__ triggered an error. I'll update the first post to clarify.

my impression of array is that it should only be used to convert custom objects to np.ndarray (usually using np.array or np.asarray)

OK that would suggest that the current behaviour is correct

[jakirkham]

Probably out of my depth here (so please forgive me), but one thing that might be worth looking at is Array API support, which CuPy 10+ supports and Dask is working on support for ( dask/dask#8750 ). Believe XArray is taking some initial steps in this direction recently ( #6804 ), but could easily be misunderstanding the scope/intended usage of the changes there.

[TomNicholas]

My understanding is that if __array_function__ is working correctly you should never need to call cupy.round on your dataarray. Instead you should always be able to call np.round and trust that the __array_function__ implementation will dispatch to cupy's equivalent of round automatically.

Is there actually a case where we need the library-specific version of a numpy function to work too?

Do we need to update AbstractArray.array to return the underlying duck array instead of always a numpy array?

(Having said all that we might still want to make this change anyway, this was just an argument for the current behaviour being "good enough".)

[dcherian]

My understanding is that if array_function is working correctly you should never need to call cupy.round on your dataarray. Instead you should always be able to call np.round and trust that the array_function implementation will dispatch to cupy's equivalent of round automatically.

👍 Question is whether we are expected to also make cp.round work.

Is there actually a case where we need the library-specific version of a numpy function to work too?

Someone's going to try it =) . At least we should document what's expected.