support inplace ops in array_ufunc#394
Conversation
|
|
||
| def convert_ndarray_to_np_ndarray(x): | ||
| if isinstance(x, ndarray): | ||
| return np.ndarray(x.shape, x.dtype, x) |
There was a problem hiding this comment.
I am not sure this is sufficient. Is this doing the right thing for non-contiguous array x?
In [1]: import numpy as np
In [2]: X = np.empty((7, 5))
In [3]: Y = X[1::2, -1::-2]
In [4]: Y.flags
Out[4]:
C_CONTIGUOUS : False
F_CONTIGUOUS : False
OWNDATA : False
WRITEABLE : True
ALIGNED : True
WRITEBACKIFCOPY : False
UPDATEIFCOPY : False
In [5]: np.ndarray(Y.shape, Y.dtype, Y)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-5-86e1bd43f835> in <module>
----> 1 np.ndarray(Y.shape, Y.dtype, Y)
ValueError: ndarray is not contiguous
This code will need to change to use np.array(x, copy=False, subok=False).
In [8]: Z = np.array(Y, copy=False, subok=False)
In [10]: (Z.strides , Y.strides)
Out[10]: ((80, -16), (80, -16))
In [11]: (Z.shape , Y.shape)
Out[11]: ((3, 3), (3, 3))
In [12]: (Y.dtype, X.dtype)
Out[12]: (dtype('float64'), dtype('float64'))
In [15]: Z.ctypes.data
Out[15]: 94827342751896
In [16]: Y.ctypes.data
Out[16]: 94827342751896
There was a problem hiding this comment.
@oleksandr-pavlyk Thanks again for noticing the potential issue. There is a bigger issue here I think. If np.array does make a copy and this is used for the out argument, then it is the copy that would be updated and not the original, right? There's a line about 20 above in the source code where for non-out arguments the conversion to np.ndarray is done and this one should also use your np.array idea but we don't have the issue there where there is a copy because it is input-only. Do you think that is right?
There was a problem hiding this comment.
Perhaps for out keyword arguments we have to post-check that for y = np.array(x, copy=False, subok=False) we have x is an ndarray subclass and x.ctypes.data == y.ctypes.data and raise an error if it is not.
There was a problem hiding this comment.
In fact, I think we must insist that the out be only ndarray and raise an error otherwise, sort of like NumPy already does:
In [1]: import numpy as np
In [2]: np.sin([1.,2.,3.], out=[1.,2.,3.])
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-2-d866583ee0d2> in <module>
----> 1 np.sin([1.,2.,3.], out=[1.,2.,3.])
TypeError: return arrays must be of ArrayType
There was a problem hiding this comment.
In fact, I think we must insist that the
outbe onlyndarrayand raise an error otherwise, sort of like NumPy already does:In [1]: import numpy as np In [2]: np.sin([1.,2.,3.], out=[1.,2.,3.]) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-2-d866583ee0d2> in <module> ----> 1 np.sin([1.,2.,3.], out=[1.,2.,3.]) TypeError: return arrays must be of ArrayType
@oleksandr-pavlyk My understanding is that for inplace operations you get a tuple of (np.ndarray,) but for non in-place operations, we get a non-tuple np.ndarray. Is that your understanding?
There was a problem hiding this comment.
No exactly. For out-of-place operations out=None, for in-place, it is either an array, or a tuple of arrays of required length:
In [1]: import numpy as np
In [2]: res = np.empty((4,))
In [3]: X = np.array([1., 1.1, 1.2, 1.3])
In [4]: np.sin(X, out=res)
Out[4]: array([0.84147098, 0.89120736, 0.93203909, 0.96355819])
In [5]: res
Out[5]: array([0.84147098, 0.89120736, 0.93203909, 0.96355819])
In [6]: np.sin(2*X, out=(res,))
Out[6]: array([0.90929743, 0.8084964 , 0.67546318, 0.51550137])
In [7]: res
Out[7]: array([0.90929743, 0.8084964 , 0.67546318, 0.51550137])
There are u-functions in NumPy that return more than one argument, e.g. np.frexp. For them
In [10]: X = np.array([3.4, 1.2])
In [11]: mant = np.empty((2,), dtype='d')
In [12]: exp = np.empty((2,), dtype='i4')
In [13]: np.frexp(X, out=(mant, exp))
Out[13]: (array([0.85, 0.6 ]), array([2, 1], dtype=int32))
In [14]: mant
Out[14]: array([0.85, 0.6 ])
In [15]: exp
Out[15]: array([2, 1], dtype=int32)
|
Running this branch locally: also, running a local example, the code runs into an infinite recursion, but I have not triaged to pinpoint the cause: |
|
I fixed issues I have found and am about to push a fix |
…n PR. Added few more tests pertaining to out keyword use
ad97d07 to
ada9420
Compare
|
@oleksandr-pavlyk Thanks Sasha. |
No description provided.