From 0e5817e75c1bb4da07277f76cbb770fc2ea117d0 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 11:00:07 +0530 Subject: [PATCH 01/37] keepdims added to np.argmin,np.argmax --- numpy/core/fromnumeric.py | 53 ++++++++++++++++++++++++----- numpy/core/tests/test_multiarray.py | 22 ++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index 65a42eb1ee72..e1f08db21295 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1114,12 +1114,12 @@ def argsort(a, axis=-1, kind=None, order=None): return _wrapfunc(a, 'argsort', axis=axis, kind=kind, order=order) -def _argmax_dispatcher(a, axis=None, out=None): +def _argmax_dispatcher(a, axis=None, out=None, keepdims=None): return (a, out) @array_function_dispatch(_argmax_dispatcher) -def argmax(a, axis=None, out=None): +def argmax(a, axis=None, out=None, keepdims=False): """ Returns the indices of the maximum values along an axis. @@ -1133,12 +1133,18 @@ def argmax(a, axis=None, out=None): out : array, optional If provided, the result will be inserted into this array. It should be of the appropriate shape and dtype. + keepdims : bool, optional + If this is set to True, the axes which are reduced are left + in the result as dimensions with size one. With this option, + the result will broadcast correctly against the array. Returns ------- index_array : ndarray of ints Array of indices into the array. It has the same shape as `a.shape` - with the dimension along `axis` removed. + with the dimension along `axis` removed. If `keepdims` is set to True, + then the size of `axis` will be 1 with the resulting array having same + shape as `a.shape`. See Also -------- @@ -1191,16 +1197,28 @@ def argmax(a, axis=None, out=None): >>> np.take_along_axis(x, np.expand_dims(index_array, axis=-1), axis=-1).squeeze(axis=-1) array([4, 3]) + Setting `keepdims` to `True`, + + >>> x = np.arange(24).reshape((2, 3, 4)) + >>> res = np.argmax(x, axis=1, keepdims=True) + >>> res.shape + (2, 1, 4) """ - return _wrapfunc(a, 'argmax', axis=axis, out=out) + res = _wrapfunc(a, 'argmax', axis=axis, out=out) + if keepdims: + curr_shape = a.shape + new_shape = curr_shape[:axis] + (1,) + curr_shape[axis + 1:] + return res.reshape(new_shape) + + return res -def _argmin_dispatcher(a, axis=None, out=None): +def _argmin_dispatcher(a, axis=None, out=None, keepdims=None): return (a, out) @array_function_dispatch(_argmin_dispatcher) -def argmin(a, axis=None, out=None): +def argmin(a, axis=None, out=None, keepdims=False): """ Returns the indices of the minimum values along an axis. @@ -1214,12 +1232,18 @@ def argmin(a, axis=None, out=None): out : array, optional If provided, the result will be inserted into this array. It should be of the appropriate shape and dtype. + keepdims : bool, optional + If this is set to True, the axes which are reduced are left + in the result as dimensions with size one. With this option, + the result will broadcast correctly against the array. Returns ------- index_array : ndarray of ints Array of indices into the array. It has the same shape as `a.shape` - with the dimension along `axis` removed. + with the dimension along `axis` removed. If `keepdims` is set to True, + then the size of `axis` will be 1 with the resulting array having same + shape as `a.shape`. See Also -------- @@ -1272,8 +1296,21 @@ def argmin(a, axis=None, out=None): >>> np.take_along_axis(x, np.expand_dims(index_array, axis=-1), axis=-1).squeeze(axis=-1) array([2, 0]) + Setting `keepdims` to `True`, + + >>> x = np.arange(24).reshape((2, 3, 4)) + >>> res = np.argmin(x, axis=1, keepdims=True) + >>> res.shape + (2, 1, 4) """ - return _wrapfunc(a, 'argmin', axis=axis, out=out) + res = _wrapfunc(a, 'argmin', axis=axis, out=out) + + if keepdims: + curr_shape = a.shape + new_shape = curr_shape[:axis] + (1,) + curr_shape[axis + 1:] + return res.reshape(new_shape) + + return res def _searchsorted_dispatcher(a, v, side=None, sorter=None): diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 25dd76256663..0ec68b1a7c7e 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4333,6 +4333,17 @@ def test_object_argmax_with_NULLs(self): assert_equal(a.argmax(), 3) a[1] = 30 assert_equal(a.argmax(), 1) + + def test_np_argmax_keepdims(self): + + sizes = [(3,), (3, 2), (2, 3), + (3, 3), (2, 3, 4), (4, 3, 2)] + for size in sizes: + arr = np.random.normal(size=size) + for axis in range(len(size)): + res = np.argmax(arr, axis=axis, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape[axis] == 1) class TestArgmin: @@ -4489,6 +4500,17 @@ def test_object_argmin_with_NULLs(self): assert_equal(a.argmin(), 3) a[1] = 10 assert_equal(a.argmin(), 1) + + def test_np_argmin_keepdims(self): + + sizes = [(3,), (3, 2), (2, 3), + (3, 3), (2, 3, 4), (4, 3, 2)] + for size in sizes: + arr = np.random.normal(size=size) + for axis in range(len(size)): + res = np.argmin(arr, axis=axis, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape[axis] == 1) class TestMinMax: From 5d664271ef802e70b98ff718158ac576f6bae23f Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 11:07:10 +0530 Subject: [PATCH 02/37] Added release notes entry --- doc/release/upcoming_changes/19211.new_feature.rst | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 doc/release/upcoming_changes/19211.new_feature.rst diff --git a/doc/release/upcoming_changes/19211.new_feature.rst b/doc/release/upcoming_changes/19211.new_feature.rst new file mode 100644 index 000000000000..1833d533e15f --- /dev/null +++ b/doc/release/upcoming_changes/19211.new_feature.rst @@ -0,0 +1,9 @@ +`keepdims` optional argument added to `numpy.argmin`, `numpy.argmax` +-------------------------------------------------------------------- + +`keepdims` argument has been added to `numpy.argmin`, `numpy.argmax`. +By default, it is `False` and hence no behaviour change should be expected +in existing codes using `numpy.argmin`, `numpy.argmax`. If set to `True`, +the axes which are reduced are left in the result as dimensions with size one. +In simple words, the resulting array will be having same dimensions +as the input array. \ No newline at end of file From 4d8f3af6ac92faae5548c3522b28306ad3ceb9b6 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 13:39:14 +0530 Subject: [PATCH 03/37] tested for axis=None,keepdims=True --- numpy/core/fromnumeric.py | 17 +++++++++++++---- numpy/core/tests/test_multiarray.py | 10 +++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index e1f08db21295..b7c254764fb5 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1205,9 +1205,14 @@ def argmax(a, axis=None, out=None, keepdims=False): (2, 1, 4) """ res = _wrapfunc(a, 'argmax', axis=axis, out=out) + if keepdims: - curr_shape = a.shape - new_shape = curr_shape[:axis] + (1,) + curr_shape[axis + 1:] + if axis is None: + new_shape = (1,)*a.ndim + else: + curr_shape = a.shape + new_shape = list(curr_shape) + new_shape[axis] = 1 return res.reshape(new_shape) return res @@ -1306,8 +1311,12 @@ def argmin(a, axis=None, out=None, keepdims=False): res = _wrapfunc(a, 'argmin', axis=axis, out=out) if keepdims: - curr_shape = a.shape - new_shape = curr_shape[:axis] + (1,) + curr_shape[axis + 1:] + if axis is None: + new_shape = (1,)*a.ndim + else: + curr_shape = a.shape + new_shape = list(curr_shape) + new_shape[axis] = 1 return res.reshape(new_shape) return res diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 0ec68b1a7c7e..a8e0b15e5667 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4345,6 +4345,10 @@ def test_np_argmax_keepdims(self): assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) + # Testing for axis=None, keepdims=True + res = np.argmin(arr, axis=None, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape == (1,)*arr.ndim) class TestArgmin: @@ -4511,7 +4515,11 @@ def test_np_argmin_keepdims(self): res = np.argmin(arr, axis=axis, keepdims=True) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) - + + # Testing for axis=None, keepdims=True + res = np.argmin(arr, axis=None, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape == (1,)*arr.ndim) class TestMinMax: From aa2adc98854b10a1bd19eb53fa202e1b3eb64140 Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Thu, 10 Jun 2021 13:41:30 +0530 Subject: [PATCH 04/37] Apply suggestions from code review --- numpy/core/fromnumeric.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index b7c254764fb5..674616380713 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1210,8 +1210,7 @@ def argmax(a, axis=None, out=None, keepdims=False): if axis is None: new_shape = (1,)*a.ndim else: - curr_shape = a.shape - new_shape = list(curr_shape) + new_shape = list(a.shape) new_shape[axis] = 1 return res.reshape(new_shape) @@ -1314,8 +1313,7 @@ def argmin(a, axis=None, out=None, keepdims=False): if axis is None: new_shape = (1,)*a.ndim else: - curr_shape = a.shape - new_shape = list(curr_shape) + new_shape = list(a.shape) new_shape[axis] = 1 return res.reshape(new_shape) From f7d4df429de2c524994ff53980cb110c79f7eb12 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 13:50:23 +0530 Subject: [PATCH 05/37] updated interface --- numpy/core/fromnumeric.pyi | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/numpy/core/fromnumeric.pyi b/numpy/core/fromnumeric.pyi index 3342ec3ac47b..775e44a68533 100644 --- a/numpy/core/fromnumeric.pyi +++ b/numpy/core/fromnumeric.pyi @@ -130,12 +130,14 @@ def argmax( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., + keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmax( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., + keepdims: bool = ..., ) -> Any: ... @overload @@ -143,12 +145,14 @@ def argmin( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., + keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmin( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., + keepdims: bool = ..., ) -> Any: ... @overload From 1e725c7127897ec560920e0d878bd7a4ee04b844 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 14:32:10 +0530 Subject: [PATCH 06/37] updated interface --- numpy/__init__.pyi | 8 +++++++- numpy/core/fromnumeric.py | 6 +++--- numpy/ma/core.py | 8 ++++---- numpy/ma/core.pyi | 4 ++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/numpy/__init__.pyi b/numpy/__init__.pyi index ec38d49439a0..2442d1b4ebf5 100644 --- a/numpy/__init__.pyi +++ b/numpy/__init__.pyi @@ -1299,18 +1299,21 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., + keepdims: L[False] = ..., ) -> intp: ... @overload def argmax( self, axis: _ShapeLike = ..., out: None = ..., + keepdims: bool = ..., ) -> Any: ... @overload def argmax( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., + keepdims: bool = ..., ) -> _NdArraySubClass: ... @overload @@ -1318,18 +1321,21 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., + keepdims: L[False] = ..., ) -> intp: ... @overload def argmin( self, axis: _ShapeLike = ..., - out: None = ..., + out: None = ..., + keepdims: bool = ..., ) -> Any: ... @overload def argmin( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., + keepdims: bool = ..., ) -> _NdArraySubClass: ... def argsort( diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index 674616380713..15e04e6583f6 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1115,7 +1115,7 @@ def argsort(a, axis=-1, kind=None, order=None): def _argmax_dispatcher(a, axis=None, out=None, keepdims=None): - return (a, out) + return (a, out, keepdims) @array_function_dispatch(_argmax_dispatcher) @@ -1205,7 +1205,7 @@ def argmax(a, axis=None, out=None, keepdims=False): (2, 1, 4) """ res = _wrapfunc(a, 'argmax', axis=axis, out=out) - + if keepdims: if axis is None: new_shape = (1,)*a.ndim @@ -1218,7 +1218,7 @@ def argmax(a, axis=None, out=None, keepdims=False): def _argmin_dispatcher(a, axis=None, out=None, keepdims=None): - return (a, out) + return (a, out, keepdims) @array_function_dispatch(_argmin_dispatcher) diff --git a/numpy/ma/core.py b/numpy/ma/core.py index 82e5e7155322..033bd50ca392 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -5491,7 +5491,7 @@ def argsort(self, axis=np._NoValue, kind=None, order=None, filled = self.filled(fill_value) return filled.argsort(axis=axis, kind=kind, order=order) - def argmin(self, axis=None, fill_value=None, out=None): + def argmin(self, axis=None, fill_value=None, out=None, keepdims=False): """ Return array of indices to the minimum values along the given axis. @@ -5534,9 +5534,9 @@ def argmin(self, axis=None, fill_value=None, out=None): if fill_value is None: fill_value = minimum_fill_value(self) d = self.filled(fill_value).view(ndarray) - return d.argmin(axis, out=out) + return d.argmin(axis, out=out, keepdims=keepdims) - def argmax(self, axis=None, fill_value=None, out=None): + def argmax(self, axis=None, fill_value=None, out=None, keepdims=False): """ Returns array of indices of the maximum values along the given axis. Masked values are treated as if they had the value fill_value. @@ -5571,7 +5571,7 @@ def argmax(self, axis=None, fill_value=None, out=None): if fill_value is None: fill_value = maximum_fill_value(self._data) d = self.filled(fill_value).view(ndarray) - return d.argmax(axis, out=out) + return d.argmax(axis, out=out, keepdims=keepdims) def sort(self, axis=-1, kind=None, order=None, endwith=True, fill_value=None): diff --git a/numpy/ma/core.pyi b/numpy/ma/core.pyi index e7e3f1f36818..139b583265c1 100644 --- a/numpy/ma/core.pyi +++ b/numpy/ma/core.pyi @@ -270,8 +270,8 @@ class MaskedArray(ndarray[_ShapeType, _DType_co]): def std(self, axis=..., dtype=..., out=..., ddof=..., keepdims=...): ... def round(self, decimals=..., out=...): ... def argsort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... - def argmin(self, axis=..., fill_value=..., out=...): ... - def argmax(self, axis=..., fill_value=..., out=...): ... + def argmin(self, axis=..., fill_value=..., out=..., keepdims=...): ... + def argmax(self, axis=..., fill_value=..., out=..., keepdims=...): ... def sort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... def min(self, axis=..., out=..., fill_value=..., keepdims=...): ... # NOTE: deprecated From 4d2bfab1428bcb4b41d35880178b8ab74f151217 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 10 Jun 2021 15:08:49 +0530 Subject: [PATCH 07/37] API changed, implementation to be done --- doc/source/reference/c-api/array.rst | 3 ++- numpy/__init__.cython-30.pxd | 2 +- numpy/__init__.pxd | 2 +- numpy/core/fromnumeric.py | 13 ++----------- numpy/core/src/multiarray/calculation.c | 3 ++- numpy/core/src/multiarray/calculation.h | 3 ++- numpy/core/src/multiarray/methods.c | 4 +++- 7 files changed, 13 insertions(+), 17 deletions(-) diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index cb2f4b645af2..955208990eb6 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2014,7 +2014,8 @@ Calculation .. c:function:: PyObject* PyArray_ArgMax( \ - PyArrayObject* self, int axis, PyArrayObject* out) + PyArrayObject* self, int axis, PyArrayObject* out, \ + int keepdims) Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the largest element of *self* along *axis*. diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index 42a46d0b832b..c1653af22b90 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -643,7 +643,7 @@ cdef extern from "numpy/arrayobject.h": int PyArray_Sort (ndarray, int, NPY_SORTKIND) object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) - object PyArray_ArgMax (ndarray, int, ndarray) + object PyArray_ArgMax (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index 97f3da2e5673..fadaf37574e0 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -601,7 +601,7 @@ cdef extern from "numpy/arrayobject.h": int PyArray_Sort (ndarray, int, NPY_SORTKIND) object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) - object PyArray_ArgMax (ndarray, int, ndarray) + object PyArray_ArgMax (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index 15e04e6583f6..d986223ebcce 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1204,17 +1204,8 @@ def argmax(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ - res = _wrapfunc(a, 'argmax', axis=axis, out=out) - - if keepdims: - if axis is None: - new_shape = (1,)*a.ndim - else: - new_shape = list(a.shape) - new_shape[axis] = 1 - return res.reshape(new_shape) - - return res + return _wrapfunc(a, 'argmax', axis=axis, out=out, + keepdims=keepdims) def _argmin_dispatcher(a, axis=None, out=None, keepdims=None): diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index de67b35b53d6..ad779251b626 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -38,7 +38,8 @@ power_of_ten(int n) * ArgMax */ NPY_NO_EXPORT PyObject * -PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) +PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out, + int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index 34bc31f69806..bc7fab7218d9 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -2,7 +2,8 @@ #define _NPY_CALCULATION_H_ NPY_NO_EXPORT PyObject* -PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out); +PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out, + int keepdims); NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index 251e527a6b96..59839432f865 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -284,16 +284,18 @@ array_argmax(PyArrayObject *self, { int axis = NPY_MAXDIMS; PyArrayObject *out = NULL; + int keepdims = -1; NPY_PREPARE_ARGPARSER; if (npy_parse_arguments("argmax", args, len_args, kwnames, "|axis", &PyArray_AxisConverter, &axis, "|out", &PyArray_OutputConverter, &out, + "|keepdims", &PyArray_BoolConverter, &keepdims, NULL, NULL, NULL) < 0) { return NULL; } - PyObject *ret = PyArray_ArgMax(self, axis, out); + PyObject *ret = PyArray_ArgMax(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { From 09433d6202f0151c382ebc9d3e997ed5a721afcd Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 11 Jun 2021 12:10:30 +0530 Subject: [PATCH 08/37] Added reshape approach to C implementation --- doc/source/reference/c-api/array.rst | 10 +++++-- numpy/__init__.cython-30.pxd | 3 +- numpy/__init__.pxd | 3 +- numpy/core/src/multiarray/calculation.c | 40 +++++++++++++++++++++++-- numpy/core/src/multiarray/calculation.h | 6 ++-- numpy/core/src/multiarray/methods.c | 9 ++++-- numpy/core/src/umath/ufunc_object.c | 1 + numpy/core/tests/test_multiarray.py | 38 +++++++++++++---------- 8 files changed, 85 insertions(+), 25 deletions(-) diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index 955208990eb6..456e7115f9bc 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2014,12 +2014,18 @@ Calculation .. c:function:: PyObject* PyArray_ArgMax( \ - PyArrayObject* self, int axis, PyArrayObject* out, \ - int keepdims) + PyArrayObject* self, int axis, PyArrayObject* out) Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the largest element of *self* along *axis*. +.. c:function:: PyObject* PyArray_ArgMaxKeepdims( \ + PyArrayObject* self, int axis, PyArrayObject* out) + + Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of + the largest element of *self* along *axis*. The number of dimensions of the result matches with the + that of the input object. + .. c:function:: PyObject* PyArray_ArgMin( \ PyArrayObject* self, int axis, PyArrayObject* out) diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index c1653af22b90..f5a5214b142a 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -643,7 +643,8 @@ cdef extern from "numpy/arrayobject.h": int PyArray_Sort (ndarray, int, NPY_SORTKIND) object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) - object PyArray_ArgMax (ndarray, int, ndarray, int) + object PyArray_ArgMax (ndarray, int, ndarray) + object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index fadaf37574e0..fd0f4ed96a5f 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -601,7 +601,8 @@ cdef extern from "numpy/arrayobject.h": int PyArray_Sort (ndarray, int, NPY_SORTKIND) object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) - object PyArray_ArgMax (ndarray, int, ndarray, int) + object PyArray_ArgMax (ndarray, int, ndarray) + object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index ad779251b626..afa58b959fb8 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -17,6 +17,8 @@ #include "calculation.h" #include "array_assign.h" +#include "alloc.h" + static double power_of_ten(int n) { @@ -38,8 +40,7 @@ power_of_ten(int n) * ArgMax */ NPY_NO_EXPORT PyObject * -PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out, - int keepdims) +PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; @@ -151,6 +152,41 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out, return NULL; } +NPY_NO_EXPORT PyObject* +PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { + PyArrayObject* ret = (PyArrayObject*)PyArray_ArgMax(op, axis, NULL); + PyArray_Dims newdims; + newdims.len = PyArray_NDIM(op); + newdims.ptr = npy_alloc_cache_dim(newdims.len); + npy_intp* currdim = PyArray_DIMS(op); + for( int dim = 0; dim < newdims.len; dim++ ) { + if( dim == axis || axis == NPY_MAXDIMS ) { + newdims.ptr[dim] = 1; + } else { + newdims.ptr[dim] = currdim[dim]; + } + } + + // If ret is same as out, then should we reshape or raise an error? + ret = (PyArrayObject*)PyArray_Newshape(ret, &newdims, NPY_CORDER); + if (out != NULL) { + if ((PyArray_NDIM(out) != PyArray_NDIM(ret)) || + !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ret), + PyArray_NDIM(out))) { + PyErr_SetString(PyExc_ValueError, + "output array dimensions do not match with that of result from np.argmax."); + Py_XDECREF(ret); + return NULL; + } + PyArray_CopyInto(out, ret); + Py_DECREF(ret); + ret = out; + Py_INCREF(ret); + } + npy_free_cache_dim_obj(newdims); + return (PyObject*)ret; +} + /*NUMPY_API * ArgMin */ diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index bc7fab7218d9..6ca3610efbad 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -2,8 +2,10 @@ #define _NPY_CALCULATION_H_ NPY_NO_EXPORT PyObject* -PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out, - int keepdims); +PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out); + +NPY_NO_EXPORT PyObject* +PyArray_ArgMaxKeepdims(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index 59839432f865..be9dd74f90eb 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -284,7 +284,7 @@ array_argmax(PyArrayObject *self, { int axis = NPY_MAXDIMS; PyArrayObject *out = NULL; - int keepdims = -1; + npy_bool keepdims = NPY_FALSE; NPY_PREPARE_ARGPARSER; if (npy_parse_arguments("argmax", args, len_args, kwnames, @@ -295,7 +295,12 @@ array_argmax(PyArrayObject *self, return NULL; } - PyObject *ret = PyArray_ArgMax(self, axis, out, keepdims); + PyObject *ret; + if( keepdims ) { + ret = PyArray_ArgMaxKeepdims(self, axis, out); + } else { + ret = PyArray_ArgMax(self, axis, out); + } /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { diff --git a/numpy/core/src/umath/ufunc_object.c b/numpy/core/src/umath/ufunc_object.c index 0644a28c011b..c949504d1f59 100644 --- a/numpy/core/src/umath/ufunc_object.c +++ b/numpy/core/src/umath/ufunc_object.c @@ -4001,6 +4001,7 @@ static PyObject * PyUFunc_GenericReduction(PyUFuncObject *ufunc, PyObject *const *args, Py_ssize_t len_args, PyObject *kwnames, int operation) { + int i, naxes=0, ndim; int axes[NPY_MAXDIMS]; diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index a8e0b15e5667..5f2c7c6dcde2 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4344,11 +4344,18 @@ def test_np_argmax_keepdims(self): res = np.argmax(arr, axis=axis, keepdims=True) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) + outarray = np.empty(res.shape, dtype=res.dtype) + np.argmax(arr, axis=axis, out=outarray, + keepdims=True) + assert_equal(res, outarray) # Testing for axis=None, keepdims=True - res = np.argmin(arr, axis=None, keepdims=True) + res = np.argmax(arr, axis=None, keepdims=True) assert_(res.ndim == arr.ndim) assert_(res.shape == (1,)*arr.ndim) + outarray = np.empty(res.shape, dtype=res.dtype) + np.argmax(arr, axis=None, out=outarray, keepdims=True) + assert_equal(res, outarray) class TestArgmin: @@ -4505,21 +4512,22 @@ def test_object_argmin_with_NULLs(self): a[1] = 10 assert_equal(a.argmin(), 1) - def test_np_argmin_keepdims(self): - - sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2)] - for size in sizes: - arr = np.random.normal(size=size) - for axis in range(len(size)): - res = np.argmin(arr, axis=axis, keepdims=True) - assert_(res.ndim == arr.ndim) - assert_(res.shape[axis] == 1) + # Avoid until np.argmin is ready to support keepdims + # def test_np_argmin_keepdims(self): + + # sizes = [(3,), (3, 2), (2, 3), + # (3, 3), (2, 3, 4), (4, 3, 2)] + # for size in sizes: + # arr = np.random.normal(size=size) + # for axis in range(len(size)): + # res = np.argmin(arr, axis=axis, keepdims=True) + # assert_(res.ndim == arr.ndim) + # assert_(res.shape[axis] == 1) - # Testing for axis=None, keepdims=True - res = np.argmin(arr, axis=None, keepdims=True) - assert_(res.ndim == arr.ndim) - assert_(res.shape == (1,)*arr.ndim) + # # Testing for axis=None, keepdims=True + # res = np.argmin(arr, axis=None, keepdims=True) + # assert_(res.ndim == arr.ndim) + # assert_(res.shape == (1,)*arr.ndim) class TestMinMax: From 8ea07b4f1fbd21091fd91baf9c00f48c0b58e9eb Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 11 Jun 2021 16:16:10 +0530 Subject: [PATCH 09/37] buggy implementation without reshape --- doc/source/reference/c-api/array.rst | 7 ++ numpy/__init__.cython-30.pxd | 1 + numpy/__init__.pxd | 1 + numpy/core/fromnumeric.py | 12 +- numpy/core/src/multiarray/calculation.c | 155 ++++++++++++++++++++++++ numpy/core/src/multiarray/calculation.h | 3 + numpy/core/src/multiarray/methods.c | 11 +- numpy/core/tests/test_multiarray.py | 31 ++--- 8 files changed, 194 insertions(+), 27 deletions(-) diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index 456e7115f9bc..b591b3fe037d 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2032,6 +2032,13 @@ Calculation Equivalent to :meth:`ndarray.argmin` (*self*, *axis*). Return the index of the smallest element of *self* along *axis*. +.. c:function:: PyObject* PyArray_ArgMinKeepdims( \ + PyArrayObject* self, int axis, PyArrayObject* out) + + Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of + the smallest element of *self* along *axis*. The number of dimensions of the result matches with the + that of the input object. + .. c:function:: PyObject* PyArray_Max( \ PyArrayObject* self, int axis, PyArrayObject* out) diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index f5a5214b142a..70aaf9f0a744 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -646,6 +646,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgMax (ndarray, int, ndarray) object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) + object PyArray_ArgMinKeepdims (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index fd0f4ed96a5f..edd8cc156001 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -604,6 +604,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgMax (ndarray, int, ndarray) object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) + object PyArray_ArgMinKeepdims (ndarray, int, ndarray) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index d986223ebcce..d7f35d57e945 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1298,17 +1298,7 @@ def argmin(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ - res = _wrapfunc(a, 'argmin', axis=axis, out=out) - - if keepdims: - if axis is None: - new_shape = (1,)*a.ndim - else: - new_shape = list(a.shape) - new_shape[axis] = 1 - return res.reshape(new_shape) - - return res + return _wrapfunc(a, 'argmin', axis=axis, out=out, keepdims=keepdims) def _searchsorted_dispatcher(a, v, side=None, sorter=None): diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index afa58b959fb8..21c771d875a3 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -303,6 +303,161 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) return NULL; } +NPY_NO_EXPORT PyObject* +PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { + PyArrayObject *ap = NULL, *rp = NULL; + PyArray_ArgFunc* arg_func; + char *ip; + npy_intp *rptr; + npy_intp i, n, m; + int elsize; + NPY_BEGIN_THREADS_DEF; + + if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { + return NULL; + } + + /* + * We need to permute the array so that axis is placed at the end. + * And all other dimensions are shifted left. + */ + PyArray_Dims newaxes; + npy_intp dims[NPY_MAXDIMS]; + if (axis != PyArray_NDIM(ap)-1) { + int i; + + newaxes.ptr = dims; + newaxes.len = PyArray_NDIM(ap); + for (i = 0; i < axis; i++) { + dims[i] = i; + } + for (i = axis; i < PyArray_NDIM(ap) - 1; i++) { + dims[i] = i + 1; + } + dims[PyArray_NDIM(ap) - 1] = axis; + op = (PyArrayObject *)PyArray_Transpose(ap, &newaxes); + Py_DECREF(ap); + if (op == NULL) { + return NULL; + } + } + else { + op = ap; + } + + if( out && PyArray_NDIM(out) == PyArray_NDIM(ap) ) { + newaxes.len = PyArray_NDIM(out); + for (i = 0; i < axis; i++) { + dims[i] = i; + } + for (i = axis; i < PyArray_NDIM(out) - 1; i++) { + dims[i] = i + 1; + } + dims[PyArray_NDIM(out) - 1] = axis; + out = (PyArrayObject *)PyArray_Transpose(out, &newaxes); + } + + /* Will get native-byte order contiguous copy. */ + ap = (PyArrayObject *)PyArray_ContiguousFromAny((PyObject *)op, + PyArray_DESCR(op)->type_num, 1, 0); + Py_DECREF(op); + if (ap == NULL) { + return NULL; + } + arg_func = PyArray_DESCR(ap)->f->argmin; + if (arg_func == NULL) { + PyErr_SetString(PyExc_TypeError, + "data type not ordered"); + goto fail; + } + elsize = PyArray_DESCR(ap)->elsize; + m = PyArray_DIMS(ap)[PyArray_NDIM(ap) - 1]; + if (m == 0) { + PyErr_SetString(PyExc_ValueError, + "attempt to get argmin of an empty sequence"); + goto fail; + } + + if (!out) { + npy_intp newdims[PyArray_NDIM(ap)]; + for( int dim = 0; dim < PyArray_NDIM(ap) - 1; dim++ ) { + if( axis == NPY_MAXDIMS ) { + newdims[dim] = 1; + } else { + newdims[dim] = PyArray_DIMS(ap)[dim]; + } + } + newdims[PyArray_NDIM(ap) - 1] = 1; + rp = (PyArrayObject *)PyArray_NewFromDescr( + Py_TYPE(ap), PyArray_DescrFromType(NPY_INTP), + PyArray_NDIM(ap), newdims, NULL, NULL, + 0, (PyObject *)ap); + if (rp == NULL) { + goto fail; + } + } + else { + if ((PyArray_NDIM(out) != PyArray_NDIM(ap)) || + !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), + PyArray_NDIM(out) - 1) || + (PyArray_DIMS(out)[PyArray_NDIM(out) - 1] != 1)) { + PyErr_SetString(PyExc_ValueError, + "output array does not match result of np.argmin."); + goto fail; + } + rp = (PyArrayObject *)PyArray_FromArray(out, + PyArray_DescrFromType(NPY_INTP), + NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); + if (rp == NULL) { + goto fail; + } + } + + NPY_BEGIN_THREADS_DESCR(PyArray_DESCR(ap)); + n = PyArray_SIZE(ap)/m; + rptr = (npy_intp *)PyArray_DATA(rp); + for (ip = PyArray_DATA(ap), i = 0; i < n; i++, ip += elsize*m) { + arg_func(ip, m, rptr, ap); + rptr += 1; + } + NPY_END_THREADS_DESCR(PyArray_DESCR(ap)); + + Py_DECREF(ap); + /* Trigger the UPDATEIFCOPY/WRITEBACKIFCOPY if necessary */ + if (out != NULL && out != rp) { + PyArray_ResolveWritebackIfCopy(rp); + Py_DECREF(rp); + rp = out; + Py_INCREF(rp); + } + + newaxes.ptr = dims; + newaxes.len = PyArray_NDIM(rp); + npy_intp k; + for (i = 0, k = 0; i < PyArray_NDIM(rp); i++ ) { + if( i == axis ) { + continue; + } + dims[i] = k; + k++; + } + dims[axis] = PyArray_NDIM(rp) - 1; + op = (PyArrayObject *)PyArray_Transpose(rp, &newaxes); + if (op == NULL) { + Py_DECREF(rp); + return NULL; + } + Py_DECREF(rp); + rp = op; + Py_INCREF(rp); + return (PyObject *)rp; + + fail: + Py_DECREF(ap); + Py_XDECREF(rp); + return NULL; +} + /*NUMPY_API * Max */ diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index 6ca3610efbad..e3762c8a91df 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -10,6 +10,9 @@ PyArray_ArgMaxKeepdims(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); +NPY_NO_EXPORT PyObject* +PyArray_ArgMinKeepdims(PyArrayObject* self, int axis, PyArrayObject *out); + NPY_NO_EXPORT PyObject* PyArray_Max(PyArrayObject* self, int axis, PyArrayObject* out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index be9dd74f90eb..9b4e2749e843 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -317,16 +317,23 @@ array_argmin(PyArrayObject *self, { int axis = NPY_MAXDIMS; PyArrayObject *out = NULL; + npy_bool keepdims; NPY_PREPARE_ARGPARSER; if (npy_parse_arguments("argmin", args, len_args, kwnames, "|axis", &PyArray_AxisConverter, &axis, "|out", &PyArray_OutputConverter, &out, + "|keepdims", &PyArray_BoolConverter, &keepdims, NULL, NULL, NULL) < 0) { return NULL; } - - PyObject *ret = PyArray_ArgMin(self, axis, out); + + PyObject *ret; + if( keepdims ) { + ret = PyArray_ArgMinKeepdims(self, axis, out); + } else { + ret = PyArray_ArgMin(self, axis, out); + } /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 5f2c7c6dcde2..fa7db8ea069b 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4513,21 +4513,24 @@ def test_object_argmin_with_NULLs(self): assert_equal(a.argmin(), 1) # Avoid until np.argmin is ready to support keepdims - # def test_np_argmin_keepdims(self): - - # sizes = [(3,), (3, 2), (2, 3), - # (3, 3), (2, 3, 4), (4, 3, 2)] - # for size in sizes: - # arr = np.random.normal(size=size) - # for axis in range(len(size)): - # res = np.argmin(arr, axis=axis, keepdims=True) - # assert_(res.ndim == arr.ndim) - # assert_(res.shape[axis] == 1) + def test_np_argmin_keepdims(self): + + sizes = [(3,), (3, 2), (2, 3), + (3, 3), (2, 3, 4), (4, 3, 2)] + for size in sizes: + arr = np.random.normal(size=size) + for axis in range(len(size)): + res = np.argmin(arr, axis=axis, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape[axis] == 1) - # # Testing for axis=None, keepdims=True - # res = np.argmin(arr, axis=None, keepdims=True) - # assert_(res.ndim == arr.ndim) - # assert_(res.shape == (1,)*arr.ndim) + # Testing for axis=None, keepdims=True + res = np.argmin(arr, axis=None, keepdims=True) + assert_(res.ndim == arr.ndim) + assert_(res.shape == (1,)*arr.ndim) + outarray = np.empty(res.shape, dtype=res.dtype) + np.argmin(arr, axis=None, out=outarray, keepdims=True) + assert_equal(res, outarray) class TestMinMax: From aa7390719c0389097f015bc6ae584b4393978532 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 12 Jun 2021 11:54:15 +0530 Subject: [PATCH 10/37] TestArgMax, TestArgMin fixed, comments added --- numpy/core/src/multiarray/calculation.c | 117 ++++++++++++++++++++---- numpy/core/src/multiarray/methods.c | 2 +- numpy/core/tests/test_multiarray.py | 8 +- 3 files changed, 104 insertions(+), 23 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 21c771d875a3..d8bc2369722f 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -154,19 +154,37 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) NPY_NO_EXPORT PyObject* PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { - PyArrayObject* ret = (PyArrayObject*)PyArray_ArgMax(op, axis, NULL); + /* + * `axis_copy` retains a copy of original axis value because, `axis` + * itself is changed by the call in the following line. This is done + * for two reasons, first, `axis_copy` helps in checking whether axis + * was `None` or not as received from Python interface and second, axis + * values are consistent between `PyArray_ArgMax` and `PyArray_ArgMaxKeepdims`. + */ + int axis_copy = axis; + if ((PyArrayObject *)PyArray_CheckAxis(op, &axis, 0) == NULL) { + return NULL; + } + PyArrayObject* ret = (PyArrayObject*)PyArray_ArgMax(op, axis_copy, NULL); PyArray_Dims newdims; newdims.len = PyArray_NDIM(op); - newdims.ptr = npy_alloc_cache_dim(newdims.len); + npy_intp dims[newdims.len]; + newdims.ptr = dims; npy_intp* currdim = PyArray_DIMS(op); for( int dim = 0; dim < newdims.len; dim++ ) { - if( dim == axis || axis == NPY_MAXDIMS ) { + /* + * Here `axis` is converted to 0 when `axis = NPY_MAXDIMS` + * but `axis_copy` retains that value and hence, all values + * in `newdims.ptr` can be set to 1 if axis is received as `None` + * from Python interface. + */ + if( dim == axis || axis_copy == NPY_MAXDIMS ) { newdims.ptr[dim] = 1; } else { newdims.ptr[dim] = currdim[dim]; } } - + // If ret is same as out, then should we reshape or raise an error? ret = (PyArrayObject*)PyArray_Newshape(ret, &newdims, NPY_CORDER); if (out != NULL) { @@ -183,7 +201,6 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { ret = out; Py_INCREF(ret); } - npy_free_cache_dim_obj(newdims); return (PyObject*)ret; } @@ -309,10 +326,18 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { PyArray_ArgFunc* arg_func; char *ip; npy_intp *rptr; - npy_intp i, n, m; + npy_intp i, n, m, axis_copy; + npy_intp op_NDIM = PyArray_NDIM(op); int elsize; NPY_BEGIN_THREADS_DEF; + /* + * `axis_copy` retains a copy of original axis value because, `axis` + * itself is changed by the call in the following line. This is done + * for two reasons, because `axis_copy` helps in checking whether axis + * was `None` or not as received from Python interface. + */ + axis_copy = axis; if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { return NULL; } @@ -322,8 +347,8 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { * And all other dimensions are shifted left. */ PyArray_Dims newaxes; - npy_intp dims[NPY_MAXDIMS]; - if (axis != PyArray_NDIM(ap)-1) { + if (axis != PyArray_NDIM(ap) - 1) { + npy_intp dims[NPY_MAXDIMS]; int i; newaxes.ptr = dims; @@ -345,8 +370,16 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { op = ap; } + /* + * We need to permute the out array so that its dimensions, + * other than the axis, are in line with the input array. + */ if( out && PyArray_NDIM(out) == PyArray_NDIM(ap) ) { + npy_intp dims[NPY_MAXDIMS]; + + int i; newaxes.len = PyArray_NDIM(out); + newaxes.ptr = dims; for (i = 0; i < axis; i++) { dims[i] = i; } @@ -356,10 +389,11 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { dims[PyArray_NDIM(out) - 1] = axis; out = (PyArrayObject *)PyArray_Transpose(out, &newaxes); } - + /* Will get native-byte order contiguous copy. */ ap = (PyArrayObject *)PyArray_ContiguousFromAny((PyObject *)op, PyArray_DESCR(op)->type_num, 1, 0); + Py_DECREF(op); if (ap == NULL) { return NULL; @@ -379,28 +413,63 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { } if (!out) { - npy_intp newdims[PyArray_NDIM(ap)]; - for( int dim = 0; dim < PyArray_NDIM(ap) - 1; dim++ ) { - if( axis == NPY_MAXDIMS ) { + /* + * If out array is NULL then we create a new array + * such that when `axis` is not `None` then all the + * dimensions are same as the input array except the + * `axis` which is of size 1. When `axis` is `None`, + * then all the dimensions are of size 1. Note that, + * `axis_copy` is used to check whether `axis` was `None` + * in the input because `axis` itself has changed due to previous + * code. + */ + int newdims_len; + if( axis_copy == NPY_MAXDIMS ) { + newdims_len = op_NDIM; + } else { + newdims_len = PyArray_NDIM(ap); + } + npy_intp newdims[newdims_len]; + for( int dim = 0; dim < newdims_len - 1; dim++ ) { + if( axis_copy == NPY_MAXDIMS ) { newdims[dim] = 1; } else { newdims[dim] = PyArray_DIMS(ap)[dim]; } } - newdims[PyArray_NDIM(ap) - 1] = 1; + newdims[newdims_len - 1] = 1; rp = (PyArrayObject *)PyArray_NewFromDescr( Py_TYPE(ap), PyArray_DescrFromType(NPY_INTP), - PyArray_NDIM(ap), newdims, NULL, NULL, + newdims_len, newdims, NULL, NULL, 0, (PyObject *)ap); if (rp == NULL) { goto fail; } - } - else { - if ((PyArray_NDIM(out) != PyArray_NDIM(ap)) || - !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), - PyArray_NDIM(out) - 1) || - (PyArray_DIMS(out)[PyArray_NDIM(out) - 1] != 1)) { + } else { + int shape_check; + if( axis_copy == NPY_MAXDIMS ) { + /* + * When axis is `None` in the input then all dimensions + * of out array should be 1. Note that for comparing number + * of dimensions we have used `op_NDIM` because it is the actual + * number of dimensions in the input. `ap` looses one dimension + * due to previous code. + */ + shape_check = PyArray_NDIM(out) == op_NDIM; + for( int dim = 0; shape_check && dim < PyArray_NDIM(out); dim++ ) { + shape_check = PyArray_DIMS(out)[dim] == 1; + } + } else { + /* + * Standard case, the out array dimensions should be same + * as input array except the last one which should be 1. + */ + shape_check = (PyArray_NDIM(out) == PyArray_NDIM(ap)) && + PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), + PyArray_NDIM(out) - 1) && + (PyArray_DIMS(out)[PyArray_NDIM(out) - 1] == 1); + } + if (!shape_check) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); goto fail; @@ -431,6 +500,14 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { Py_INCREF(rp); } + /* + * The following code takes the axis back to its original + * position which was placed in the last dimension earlier. + * This can be interpreted as reversing the previously taken + * transpose of output, which was taken to travel through + * the desired axis by just increasing the pointer by 1. + */ + npy_intp dims[NPY_MAXDIMS]; newaxes.ptr = dims; newaxes.len = PyArray_NDIM(rp); npy_intp k; diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index 9b4e2749e843..ece301a0d318 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -317,7 +317,7 @@ array_argmin(PyArrayObject *self, { int axis = NPY_MAXDIMS; PyArrayObject *out = NULL; - npy_bool keepdims; + npy_bool keepdims = NPY_FALSE; NPY_PREPARE_ARGPARSER; if (npy_parse_arguments("argmin", args, len_args, kwnames, diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index fa7db8ea069b..c2164511cdcd 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4340,7 +4340,7 @@ def test_np_argmax_keepdims(self): (3, 3), (2, 3, 4), (4, 3, 2)] for size in sizes: arr = np.random.normal(size=size) - for axis in range(len(size)): + for axis in range(-len(size), len(size)): res = np.argmax(arr, axis=axis, keepdims=True) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) @@ -4519,10 +4519,14 @@ def test_np_argmin_keepdims(self): (3, 3), (2, 3, 4), (4, 3, 2)] for size in sizes: arr = np.random.normal(size=size) - for axis in range(len(size)): + for axis in range(-len(size), len(size)): res = np.argmin(arr, axis=axis, keepdims=True) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) + outarray = np.empty(res.shape, dtype=res.dtype) + np.argmin(arr, axis=axis, out=outarray, + keepdims=True) + assert_equal(res, outarray) # Testing for axis=None, keepdims=True res = np.argmin(arr, axis=None, keepdims=True) From 22fbb06f678ac5d76f47dec02044f4766722086c Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 12 Jun 2021 14:31:01 +0530 Subject: [PATCH 11/37] Fixed for matrix --- numpy/core/fromnumeric.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index d7f35d57e945..26747fa78d1a 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1204,6 +1204,8 @@ def argmax(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ + if isinstance(a, np.matrix): + return _wrapfunc(a, 'argmax', axis=axis, out=out) return _wrapfunc(a, 'argmax', axis=axis, out=out, keepdims=keepdims) @@ -1298,6 +1300,8 @@ def argmin(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ + if isinstance(a, np.matrix): + return _wrapfunc(a, 'argmin', axis=axis, out=out) return _wrapfunc(a, 'argmin', axis=axis, out=out, keepdims=keepdims) From d04946fe4288fa9f4faa2ed0fff79477d443fb35 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 12 Jun 2021 14:57:34 +0530 Subject: [PATCH 12/37] removed unrequired changes --- numpy/core/src/multiarray/calculation.c | 1 - numpy/core/src/umath/ufunc_object.c | 1 - numpy/core/tests/test_multiarray.py | 1 - numpy/ma/core.py | 6 ++++-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index d8bc2369722f..c42ea7e462f9 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -17,7 +17,6 @@ #include "calculation.h" #include "array_assign.h" -#include "alloc.h" static double power_of_ten(int n) diff --git a/numpy/core/src/umath/ufunc_object.c b/numpy/core/src/umath/ufunc_object.c index c949504d1f59..0644a28c011b 100644 --- a/numpy/core/src/umath/ufunc_object.c +++ b/numpy/core/src/umath/ufunc_object.c @@ -4001,7 +4001,6 @@ static PyObject * PyUFunc_GenericReduction(PyUFuncObject *ufunc, PyObject *const *args, Py_ssize_t len_args, PyObject *kwnames, int operation) { - int i, naxes=0, ndim; int axes[NPY_MAXDIMS]; diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index c2164511cdcd..1b95f36cfc4a 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4512,7 +4512,6 @@ def test_object_argmin_with_NULLs(self): a[1] = 10 assert_equal(a.argmin(), 1) - # Avoid until np.argmin is ready to support keepdims def test_np_argmin_keepdims(self): sizes = [(3,), (3, 2), (2, 3), diff --git a/numpy/ma/core.py b/numpy/ma/core.py index 033bd50ca392..a10538b49e80 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -5491,7 +5491,7 @@ def argsort(self, axis=np._NoValue, kind=None, order=None, filled = self.filled(fill_value) return filled.argsort(axis=axis, kind=kind, order=order) - def argmin(self, axis=None, fill_value=None, out=None, keepdims=False): + def argmin(self, axis=None, fill_value=None, out=None, keepdims=np._NoValue): """ Return array of indices to the minimum values along the given axis. @@ -5534,9 +5534,10 @@ def argmin(self, axis=None, fill_value=None, out=None, keepdims=False): if fill_value is None: fill_value = minimum_fill_value(self) d = self.filled(fill_value).view(ndarray) + keepdims = False if keepdims is np._NoValue else bool(keepdims) return d.argmin(axis, out=out, keepdims=keepdims) - def argmax(self, axis=None, fill_value=None, out=None, keepdims=False): + def argmax(self, axis=None, fill_value=None, out=None, keepdims=np._NoValue): """ Returns array of indices of the maximum values along the given axis. Masked values are treated as if they had the value fill_value. @@ -5571,6 +5572,7 @@ def argmax(self, axis=None, fill_value=None, out=None, keepdims=False): if fill_value is None: fill_value = maximum_fill_value(self._data) d = self.filled(fill_value).view(ndarray) + keepdims = False if keepdims is np._NoValue else bool(keepdims) return d.argmax(axis, out=out, keepdims=keepdims) def sort(self, axis=-1, kind=None, order=None, From 512b35941892d6965b3c086711a2ff1cb8aa893e Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 12 Jun 2021 15:15:52 +0530 Subject: [PATCH 13/37] fixed CI failure --- numpy/core/src/multiarray/calculation.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index c42ea7e462f9..f322a268db80 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -167,7 +167,7 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { PyArrayObject* ret = (PyArrayObject*)PyArray_ArgMax(op, axis_copy, NULL); PyArray_Dims newdims; newdims.len = PyArray_NDIM(op); - npy_intp dims[newdims.len]; + npy_intp dims[NPY_MAXDIMS]; newdims.ptr = dims; npy_intp* currdim = PyArray_DIMS(op); for( int dim = 0; dim < newdims.len; dim++ ) { @@ -428,7 +428,7 @@ PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { } else { newdims_len = PyArray_NDIM(ap); } - npy_intp newdims[newdims_len]; + npy_intp newdims[NPY_MAXDIMS]; for( int dim = 0; dim < newdims_len - 1; dim++ ) { if( axis_copy == NPY_MAXDIMS ) { newdims[dim] = 1; From da3df5b5f62ba9f122d1c6babf08619205558a7c Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 12 Jun 2021 15:21:20 +0530 Subject: [PATCH 14/37] fixed linting issue --- numpy/ma/core.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/numpy/ma/core.py b/numpy/ma/core.py index a10538b49e80..9cae4cf7ac83 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -5491,7 +5491,8 @@ def argsort(self, axis=np._NoValue, kind=None, order=None, filled = self.filled(fill_value) return filled.argsort(axis=axis, kind=kind, order=order) - def argmin(self, axis=None, fill_value=None, out=None, keepdims=np._NoValue): + def argmin(self, axis=None, fill_value=None, out=None, + keepdims=np._NoValue): """ Return array of indices to the minimum values along the given axis. @@ -5537,7 +5538,8 @@ def argmin(self, axis=None, fill_value=None, out=None, keepdims=np._NoValue): keepdims = False if keepdims is np._NoValue else bool(keepdims) return d.argmin(axis, out=out, keepdims=keepdims) - def argmax(self, axis=None, fill_value=None, out=None, keepdims=np._NoValue): + def argmax(self, axis=None, fill_value=None, out=None, + keepdims=np._NoValue): """ Returns array of indices of the maximum values along the given axis. Masked values are treated as if they had the value fill_value. From 828df3b857df418b301468d7a1898fe5ff4b2c58 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 18 Jun 2021 11:20:36 +0530 Subject: [PATCH 15/37] PyArray_ArgMaxKeepdims now only modifies shape and strides --- numpy/core/src/multiarray/calculation.c | 125 ++++++++++++++++-------- numpy/core/tests/test_multiarray.py | 12 ++- 2 files changed, 91 insertions(+), 46 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index f322a268db80..8353fc5e8e93 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -17,6 +17,8 @@ #include "calculation.h" #include "array_assign.h" +#include "alloc.h" + static double power_of_ten(int n) @@ -153,52 +155,91 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) NPY_NO_EXPORT PyObject* PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { - /* - * `axis_copy` retains a copy of original axis value because, `axis` - * itself is changed by the call in the following line. This is done - * for two reasons, first, `axis_copy` helps in checking whether axis - * was `None` or not as received from Python interface and second, axis - * values are consistent between `PyArray_ArgMax` and `PyArray_ArgMaxKeepdims`. - */ - int axis_copy = axis; - if ((PyArrayObject *)PyArray_CheckAxis(op, &axis, 0) == NULL) { - return NULL; - } - PyArrayObject* ret = (PyArrayObject*)PyArray_ArgMax(op, axis_copy, NULL); - PyArray_Dims newdims; - newdims.len = PyArray_NDIM(op); - npy_intp dims[NPY_MAXDIMS]; - newdims.ptr = dims; - npy_intp* currdim = PyArray_DIMS(op); - for( int dim = 0; dim < newdims.len; dim++ ) { - /* - * Here `axis` is converted to 0 when `axis = NPY_MAXDIMS` - * but `axis_copy` retains that value and hence, all values - * in `newdims.ptr` can be set to 1 if axis is received as `None` - * from Python interface. - */ - if( dim == axis || axis_copy == NPY_MAXDIMS ) { - newdims.ptr[dim] = 1; + PyArrayObject* ret = NULL; + if( out == NULL ) { + ret = (PyArrayObject*) PyArray_ArgMax(op, axis, NULL); + if( ret == NULL ) { + return NULL; + } + ((PyArrayObject_fields*)ret)->nd = PyArray_NDIM(op); + npy_intp* ret_DIMS = npy_alloc_cache_dim(NPY_MAXDIMS); + npy_intp* ret_STRIDES = npy_alloc_cache_dim(NPY_MAXDIMS); + if( axis == NPY_MAXDIMS ) { + for( int i = 0; i < PyArray_NDIM(op); i++ ) { + ret_DIMS[i] = 1; + ret_STRIDES[i] = PyArray_DESCR(ret)->elsize; + } } else { - newdims.ptr[dim] = currdim[dim]; + check_and_adjust_axis(&axis, PyArray_NDIM(op)); + for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { + ret_DIMS[i] = PyArray_DIMS(op)[i]; + if( i != axis ) { + ret_STRIDES[i] = PyArray_STRIDES(ret)[k]; + k++; + } + } + ret_DIMS[axis] = 1; + if( axis < PyArray_NDIM(op) - 1 ) { + ret_STRIDES[axis] = ret_STRIDES[axis + 1]*ret_DIMS[axis + 1]; + } else { + ret_STRIDES[axis] = PyArray_DESCR(ret)->elsize; + } } - } - - // If ret is same as out, then should we reshape or raise an error? - ret = (PyArrayObject*)PyArray_Newshape(ret, &newdims, NPY_CORDER); - if (out != NULL) { - if ((PyArray_NDIM(out) != PyArray_NDIM(ret)) || - !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ret), - PyArray_NDIM(out))) { - PyErr_SetString(PyExc_ValueError, - "output array dimensions do not match with that of result from np.argmax."); - Py_XDECREF(ret); - return NULL; + ((PyArrayObject_fields*)ret)->dimensions = ret_DIMS; + ((PyArrayObject_fields*)ret)->strides = ret_STRIDES; + } else { + if( axis == NPY_MAXDIMS ) { + int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); + for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } + if( !is_out_shape_good ) { + PyErr_SetString(PyExc_ValueError, + "output array does not match result of np.argmax."); + return NULL; + } + ((PyArrayObject_fields*)out)->nd = 0; + PyArray_ArgMax(op, axis, out); + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); + } else { + if( PyArray_CheckAxis(op, &axis, 0) == NULL ) { + return NULL; + } + int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); + for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { + if( i == axis ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(op)[i]; + } + } + if( !is_out_shape_good ) { + PyErr_SetString(PyExc_ValueError, + "output array does not match result of np.argmax."); + return NULL; + } + for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { + if( i != axis ) { + PyArray_DIMS(out)[k] = PyArray_DIMS(out)[i]; + PyArray_STRIDES(out)[k] = PyArray_STRIDES(out)[i]; + k++; + } + } + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op) - 1; + PyArray_ArgMax(op, axis, out); + for( int i = PyArray_NDIM(out) - 1; i >= axis; i-- ) { + PyArray_DIMS(out)[i + 1] = PyArray_DIMS(out)[i]; + PyArray_STRIDES(out)[i + 1] = PyArray_STRIDES(out)[i]; + } + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); + PyArray_DIMS(out)[axis] = 1; + if( axis < PyArray_NDIM(op) - 1 ) { + PyArray_STRIDES(out)[axis] = PyArray_STRIDES(out)[axis + 1]*PyArray_DIMS(out)[axis + 1]; + } else { + PyArray_STRIDES(out)[axis] = PyArray_DESCR(out)->elsize; + } } - PyArray_CopyInto(out, ret); - Py_DECREF(ret); ret = out; - Py_INCREF(ret); } return (PyObject*)ret; } diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 1b95f36cfc4a..bb1924e56cd1 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4337,7 +4337,9 @@ def test_object_argmax_with_NULLs(self): def test_np_argmax_keepdims(self): sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2)] + (3, 3), (2, 3, 4), (4, 3, 2), + (1, 2, 3, 4), (2, 3, 4, 1), + (3, 4, 1, 2), (4, 1, 2, 3)] for size in sizes: arr = np.random.normal(size=size) for axis in range(-len(size), len(size)): @@ -4345,8 +4347,9 @@ def test_np_argmax_keepdims(self): assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) outarray = np.empty(res.shape, dtype=res.dtype) - np.argmax(arr, axis=axis, out=outarray, - keepdims=True) + res1 = np.argmax(arr, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) assert_equal(res, outarray) # Testing for axis=None, keepdims=True @@ -4354,7 +4357,8 @@ def test_np_argmax_keepdims(self): assert_(res.ndim == arr.ndim) assert_(res.shape == (1,)*arr.ndim) outarray = np.empty(res.shape, dtype=res.dtype) - np.argmax(arr, axis=None, out=outarray, keepdims=True) + res1 = np.argmax(arr, axis=None, out=outarray, keepdims=True) + assert_(res1 is outarray) assert_equal(res, outarray) class TestArgmin: From 91e75301bfd5289e84394c1a5cd42f6c9a7d5df9 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 18 Jun 2021 11:53:14 +0530 Subject: [PATCH 16/37] Comments added to PyArray_ArgMaxKeepdims --- numpy/core/src/multiarray/calculation.c | 41 +++++++++++++++++++++++++ numpy/core/tests/test_multiarray.py | 8 +++++ 2 files changed, 49 insertions(+) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 8353fc5e8e93..11306fb9d109 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -156,7 +156,11 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) NPY_NO_EXPORT PyObject* PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { PyArrayObject* ret = NULL; + if( out == NULL ) { + /* + * Case 1: When out is not provided in the input. + */ ret = (PyArrayObject*) PyArray_ArgMax(op, axis, NULL); if( ret == NULL ) { return NULL; @@ -165,11 +169,19 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { npy_intp* ret_DIMS = npy_alloc_cache_dim(NPY_MAXDIMS); npy_intp* ret_STRIDES = npy_alloc_cache_dim(NPY_MAXDIMS); if( axis == NPY_MAXDIMS ) { + /* + * Change dimensions and strides so that resulting + * array has shape, (1, 1, ..., 1) + */ for( int i = 0; i < PyArray_NDIM(op); i++ ) { ret_DIMS[i] = 1; ret_STRIDES[i] = PyArray_DESCR(ret)->elsize; } } else { + /* + * Change dimensions and strides so that considered + * axis has size 1 in the resulting array. + */ check_and_adjust_axis(&axis, PyArray_NDIM(op)); for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { ret_DIMS[i] = PyArray_DIMS(op)[i]; @@ -188,7 +200,14 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { ((PyArrayObject_fields*)ret)->dimensions = ret_DIMS; ((PyArrayObject_fields*)ret)->strides = ret_STRIDES; } else { + /* + * Case 2: When out is provided in the input. + */ if( axis == NPY_MAXDIMS ) { + /* + * `out` array should have same number of dimensions as + * input array and all the axes have size 1. + */ int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { is_out_shape_good = PyArray_DIMS(out)[i] == 1; @@ -198,10 +217,21 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { "output array does not match result of np.argmax."); return NULL; } + /* + * Set the number of dimensions to 0 so that `PyArray_ArgMax` + * perceives `out` as a scalar. + */ ((PyArrayObject_fields*)out)->nd = 0; PyArray_ArgMax(op, axis, out); + /* + * Set the number of dimensions to original so that `out` + * is perceived correctly by the user. + */ ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); } else { + /* + * Check and fix the axis for negative values. + */ if( PyArray_CheckAxis(op, &axis, 0) == NULL ) { return NULL; } @@ -218,6 +248,12 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { "output array does not match result of np.argmax."); return NULL; } + /* + * Converts the shape and strides such that out.shape + * is transformed from, (d1, d2, ..., 1, ..., dn) to + * (d1, d2, ..., dn). That is axis of size 1 is removed. + * This is done so that `PyArray_ArgMax` perceives it correctly. + */ for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { if( i != axis ) { PyArray_DIMS(out)[k] = PyArray_DIMS(out)[i]; @@ -227,6 +263,10 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { } ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op) - 1; PyArray_ArgMax(op, axis, out); + /* + * Once the results are stored in `out`, the shape and + * strides of `out` are restored. + */ for( int i = PyArray_NDIM(out) - 1; i >= axis; i-- ) { PyArray_DIMS(out)[i + 1] = PyArray_DIMS(out)[i]; PyArray_STRIDES(out)[i + 1] = PyArray_STRIDES(out)[i]; @@ -239,6 +279,7 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { PyArray_STRIDES(out)[axis] = PyArray_DESCR(out)->elsize; } } + // `ret` and `out` refer to the same object. ret = out; } return (PyObject*)ret; diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index bb1924e56cd1..efa3a42cb5ea 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4343,7 +4343,12 @@ def test_np_argmax_keepdims(self): for size in sizes: arr = np.random.normal(size=size) for axis in range(-len(size), len(size)): + res_orig = np.argmax(arr, axis=axis) + new_shape = list(size) + new_shape[axis] = 1 + res_orig = res_orig.reshape(new_shape) res = np.argmax(arr, axis=axis, keepdims=True) + assert_equal(res, res_orig) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) outarray = np.empty(res.shape, dtype=res.dtype) @@ -4353,7 +4358,10 @@ def test_np_argmax_keepdims(self): assert_equal(res, outarray) # Testing for axis=None, keepdims=True + res_orig = np.argmax(arr, axis=None) + res_orig = res_orig.reshape((1,)*arr.ndim) res = np.argmax(arr, axis=None, keepdims=True) + assert_equal(res, res_orig) assert_(res.ndim == arr.ndim) assert_(res.shape == (1,)*arr.ndim) outarray = np.empty(res.shape, dtype=res.dtype) From a1c0faa1a9ffc18e4bf81c857ac0b84df9c17e64 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 18 Jun 2021 11:57:16 +0530 Subject: [PATCH 17/37] Updated implementation of PyArray_ArgMinKeepdims to match with PyArray_ArgMaxKeepdims --- numpy/core/src/multiarray/calculation.c | 307 +++++++++--------------- numpy/core/tests/test_multiarray.py | 22 +- 2 files changed, 129 insertions(+), 200 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 11306fb9d109..da75ff247b52 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -403,217 +403,134 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) NPY_NO_EXPORT PyObject* PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { - PyArrayObject *ap = NULL, *rp = NULL; - PyArray_ArgFunc* arg_func; - char *ip; - npy_intp *rptr; - npy_intp i, n, m, axis_copy; - npy_intp op_NDIM = PyArray_NDIM(op); - int elsize; - NPY_BEGIN_THREADS_DEF; - - /* - * `axis_copy` retains a copy of original axis value because, `axis` - * itself is changed by the call in the following line. This is done - * for two reasons, because `axis_copy` helps in checking whether axis - * was `None` or not as received from Python interface. - */ - axis_copy = axis; - if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { - return NULL; - } - - /* - * We need to permute the array so that axis is placed at the end. - * And all other dimensions are shifted left. - */ - PyArray_Dims newaxes; - if (axis != PyArray_NDIM(ap) - 1) { - npy_intp dims[NPY_MAXDIMS]; - int i; - - newaxes.ptr = dims; - newaxes.len = PyArray_NDIM(ap); - for (i = 0; i < axis; i++) { - dims[i] = i; - } - for (i = axis; i < PyArray_NDIM(ap) - 1; i++) { - dims[i] = i + 1; - } - dims[PyArray_NDIM(ap) - 1] = axis; - op = (PyArrayObject *)PyArray_Transpose(ap, &newaxes); - Py_DECREF(ap); - if (op == NULL) { - return NULL; - } - } - else { - op = ap; - } - - /* - * We need to permute the out array so that its dimensions, - * other than the axis, are in line with the input array. - */ - if( out && PyArray_NDIM(out) == PyArray_NDIM(ap) ) { - npy_intp dims[NPY_MAXDIMS]; - - int i; - newaxes.len = PyArray_NDIM(out); - newaxes.ptr = dims; - for (i = 0; i < axis; i++) { - dims[i] = i; - } - for (i = axis; i < PyArray_NDIM(out) - 1; i++) { - dims[i] = i + 1; - } - dims[PyArray_NDIM(out) - 1] = axis; - out = (PyArrayObject *)PyArray_Transpose(out, &newaxes); - } - - /* Will get native-byte order contiguous copy. */ - ap = (PyArrayObject *)PyArray_ContiguousFromAny((PyObject *)op, - PyArray_DESCR(op)->type_num, 1, 0); - - Py_DECREF(op); - if (ap == NULL) { - return NULL; - } - arg_func = PyArray_DESCR(ap)->f->argmin; - if (arg_func == NULL) { - PyErr_SetString(PyExc_TypeError, - "data type not ordered"); - goto fail; - } - elsize = PyArray_DESCR(ap)->elsize; - m = PyArray_DIMS(ap)[PyArray_NDIM(ap) - 1]; - if (m == 0) { - PyErr_SetString(PyExc_ValueError, - "attempt to get argmin of an empty sequence"); - goto fail; - } + PyArrayObject* ret = NULL; - if (!out) { + if( out == NULL ) { /* - * If out array is NULL then we create a new array - * such that when `axis` is not `None` then all the - * dimensions are same as the input array except the - * `axis` which is of size 1. When `axis` is `None`, - * then all the dimensions are of size 1. Note that, - * `axis_copy` is used to check whether `axis` was `None` - * in the input because `axis` itself has changed due to previous - * code. + * Case 1: When out is not provided in the input. */ - int newdims_len; - if( axis_copy == NPY_MAXDIMS ) { - newdims_len = op_NDIM; - } else { - newdims_len = PyArray_NDIM(ap); + ret = (PyArrayObject*) PyArray_ArgMin(op, axis, NULL); + if( ret == NULL ) { + return NULL; } - npy_intp newdims[NPY_MAXDIMS]; - for( int dim = 0; dim < newdims_len - 1; dim++ ) { - if( axis_copy == NPY_MAXDIMS ) { - newdims[dim] = 1; + ((PyArrayObject_fields*)ret)->nd = PyArray_NDIM(op); + npy_intp* ret_DIMS = npy_alloc_cache_dim(NPY_MAXDIMS); + npy_intp* ret_STRIDES = npy_alloc_cache_dim(NPY_MAXDIMS); + if( axis == NPY_MAXDIMS ) { + /* + * Change dimensions and strides so that resulting + * array has shape, (1, 1, ..., 1) + */ + for( int i = 0; i < PyArray_NDIM(op); i++ ) { + ret_DIMS[i] = 1; + ret_STRIDES[i] = PyArray_DESCR(ret)->elsize; + } + } else { + /* + * Change dimensions and strides so that considered + * axis has size 1 in the resulting array. + */ + check_and_adjust_axis(&axis, PyArray_NDIM(op)); + for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { + ret_DIMS[i] = PyArray_DIMS(op)[i]; + if( i != axis ) { + ret_STRIDES[i] = PyArray_STRIDES(ret)[k]; + k++; + } + } + ret_DIMS[axis] = 1; + if( axis < PyArray_NDIM(op) - 1 ) { + ret_STRIDES[axis] = ret_STRIDES[axis + 1]*ret_DIMS[axis + 1]; } else { - newdims[dim] = PyArray_DIMS(ap)[dim]; + ret_STRIDES[axis] = PyArray_DESCR(ret)->elsize; } } - newdims[newdims_len - 1] = 1; - rp = (PyArrayObject *)PyArray_NewFromDescr( - Py_TYPE(ap), PyArray_DescrFromType(NPY_INTP), - newdims_len, newdims, NULL, NULL, - 0, (PyObject *)ap); - if (rp == NULL) { - goto fail; - } + ((PyArrayObject_fields*)ret)->dimensions = ret_DIMS; + ((PyArrayObject_fields*)ret)->strides = ret_STRIDES; } else { - int shape_check; - if( axis_copy == NPY_MAXDIMS ) { + /* + * Case 2: When out is provided in the input. + */ + if( axis == NPY_MAXDIMS ) { /* - * When axis is `None` in the input then all dimensions - * of out array should be 1. Note that for comparing number - * of dimensions we have used `op_NDIM` because it is the actual - * number of dimensions in the input. `ap` looses one dimension - * due to previous code. + * `out` array should have same number of dimensions as + * input array and all the axes have size 1. */ - shape_check = PyArray_NDIM(out) == op_NDIM; - for( int dim = 0; shape_check && dim < PyArray_NDIM(out); dim++ ) { - shape_check = PyArray_DIMS(out)[dim] == 1; + int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); + for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } + if( !is_out_shape_good ) { + PyErr_SetString(PyExc_ValueError, + "output array does not match result of np.argmax."); + return NULL; } + /* + * Set the number of dimensions to 0 so that `PyArray_ArgMin` + * perceives `out` as a scalar. + */ + ((PyArrayObject_fields*)out)->nd = 0; + PyArray_ArgMin(op, axis, out); + /* + * Set the number of dimensions to original so that `out` + * is perceived correctly by the user. + */ + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); } else { /* - * Standard case, the out array dimensions should be same - * as input array except the last one which should be 1. + * Check and fix the axis for negative values. */ - shape_check = (PyArray_NDIM(out) == PyArray_NDIM(ap)) && - PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), - PyArray_NDIM(out) - 1) && - (PyArray_DIMS(out)[PyArray_NDIM(out) - 1] == 1); - } - if (!shape_check) { - PyErr_SetString(PyExc_ValueError, - "output array does not match result of np.argmin."); - goto fail; - } - rp = (PyArrayObject *)PyArray_FromArray(out, - PyArray_DescrFromType(NPY_INTP), - NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); - if (rp == NULL) { - goto fail; + if( PyArray_CheckAxis(op, &axis, 0) == NULL ) { + return NULL; + } + int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); + for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { + if( i == axis ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(op)[i]; + } + } + if( !is_out_shape_good ) { + PyErr_SetString(PyExc_ValueError, + "output array does not match result of np.argmax."); + return NULL; + } + /* + * Converts the shape and strides such that out.shape + * is transformed from, (d1, d2, ..., 1, ..., dn) to + * (d1, d2, ..., dn). That is axis of size 1 is removed. + * This is done so that `PyArray_ArgMin` perceives it correctly. + */ + for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { + if( i != axis ) { + PyArray_DIMS(out)[k] = PyArray_DIMS(out)[i]; + PyArray_STRIDES(out)[k] = PyArray_STRIDES(out)[i]; + k++; + } + } + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op) - 1; + PyArray_ArgMin(op, axis, out); + /* + * Once the results are stored in `out`, the shape and + * strides of `out` are restored. + */ + for( int i = PyArray_NDIM(out) - 1; i >= axis; i-- ) { + PyArray_DIMS(out)[i + 1] = PyArray_DIMS(out)[i]; + PyArray_STRIDES(out)[i + 1] = PyArray_STRIDES(out)[i]; + } + ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); + PyArray_DIMS(out)[axis] = 1; + if( axis < PyArray_NDIM(op) - 1 ) { + PyArray_STRIDES(out)[axis] = PyArray_STRIDES(out)[axis + 1]*PyArray_DIMS(out)[axis + 1]; + } else { + PyArray_STRIDES(out)[axis] = PyArray_DESCR(out)->elsize; + } } + // `ret` and `out` refer to the same object. + ret = out; } - - NPY_BEGIN_THREADS_DESCR(PyArray_DESCR(ap)); - n = PyArray_SIZE(ap)/m; - rptr = (npy_intp *)PyArray_DATA(rp); - for (ip = PyArray_DATA(ap), i = 0; i < n; i++, ip += elsize*m) { - arg_func(ip, m, rptr, ap); - rptr += 1; - } - NPY_END_THREADS_DESCR(PyArray_DESCR(ap)); - - Py_DECREF(ap); - /* Trigger the UPDATEIFCOPY/WRITEBACKIFCOPY if necessary */ - if (out != NULL && out != rp) { - PyArray_ResolveWritebackIfCopy(rp); - Py_DECREF(rp); - rp = out; - Py_INCREF(rp); - } - - /* - * The following code takes the axis back to its original - * position which was placed in the last dimension earlier. - * This can be interpreted as reversing the previously taken - * transpose of output, which was taken to travel through - * the desired axis by just increasing the pointer by 1. - */ - npy_intp dims[NPY_MAXDIMS]; - newaxes.ptr = dims; - newaxes.len = PyArray_NDIM(rp); - npy_intp k; - for (i = 0, k = 0; i < PyArray_NDIM(rp); i++ ) { - if( i == axis ) { - continue; - } - dims[i] = k; - k++; - } - dims[axis] = PyArray_NDIM(rp) - 1; - op = (PyArrayObject *)PyArray_Transpose(rp, &newaxes); - if (op == NULL) { - Py_DECREF(rp); - return NULL; - } - Py_DECREF(rp); - rp = op; - Py_INCREF(rp); - return (PyObject *)rp; - - fail: - Py_DECREF(ap); - Py_XDECREF(rp); - return NULL; + return (PyObject*)ret; } /*NUMPY_API diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index efa3a42cb5ea..a7ff10e4c4f6 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4527,24 +4527,36 @@ def test_object_argmin_with_NULLs(self): def test_np_argmin_keepdims(self): sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2)] + (3, 3), (2, 3, 4), (4, 3, 2), + (1, 2, 3, 4), (2, 3, 4, 1), + (3, 4, 1, 2), (4, 1, 2, 3)] for size in sizes: arr = np.random.normal(size=size) for axis in range(-len(size), len(size)): + res_orig = np.argmin(arr, axis=axis) + new_shape = list(size) + new_shape[axis] = 1 + res_orig = res_orig.reshape(new_shape) res = np.argmin(arr, axis=axis, keepdims=True) + assert_equal(res, res_orig) assert_(res.ndim == arr.ndim) assert_(res.shape[axis] == 1) outarray = np.empty(res.shape, dtype=res.dtype) - np.argmin(arr, axis=axis, out=outarray, - keepdims=True) + res1 = np.argmin(arr, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) assert_equal(res, outarray) - + # Testing for axis=None, keepdims=True + res_orig = np.argmin(arr, axis=None) + res_orig = res_orig.reshape((1,)*arr.ndim) res = np.argmin(arr, axis=None, keepdims=True) + assert_equal(res, res_orig) assert_(res.ndim == arr.ndim) assert_(res.shape == (1,)*arr.ndim) outarray = np.empty(res.shape, dtype=res.dtype) - np.argmin(arr, axis=None, out=outarray, keepdims=True) + res1 = np.argmin(arr, axis=None, out=outarray, keepdims=True) + assert_(res1 is outarray) assert_equal(res, outarray) class TestMinMax: From fa5839b275ef2370b09a89c4083d8a08c6efc991 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 18 Jun 2021 12:02:44 +0530 Subject: [PATCH 18/37] Testing complete for PyArray_ArgMinKeepdims and PyArray_ArgMaxKeepdims --- numpy/core/tests/test_multiarray.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index a7ff10e4c4f6..e5f48ca61d9b 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4527,9 +4527,9 @@ def test_object_argmin_with_NULLs(self): def test_np_argmin_keepdims(self): sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2), - (1, 2, 3, 4), (2, 3, 4, 1), - (3, 4, 1, 2), (4, 1, 2, 3)] + (3, 3), (2, 3, 4), (4, 3, 2), + (1, 2, 3, 4), (2, 3, 4, 1), + (3, 4, 1, 2), (4, 1, 2, 3)] for size in sizes: arr = np.random.normal(size=size) for axis in range(-len(size), len(size)): From 2cd3ff1886c4d2804cd979271031b9d27a78209c Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 19 Jun 2021 12:08:42 +0530 Subject: [PATCH 19/37] PyArray_ArgMinWithKeepdims both keepdims=True and keepdims=False --- doc/source/reference/c-api/array.rst | 6 +- numpy/__init__.cython-30.pxd | 2 +- numpy/__init__.pxd | 2 +- numpy/core/code_generators/numpy_api.py | 1 + numpy/core/src/multiarray/calculation.c | 205 ++++++++---------------- numpy/core/src/multiarray/calculation.h | 2 +- numpy/core/src/multiarray/methods.c | 7 +- 7 files changed, 78 insertions(+), 147 deletions(-) diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index b591b3fe037d..d96b52d32ce2 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2032,12 +2032,12 @@ Calculation Equivalent to :meth:`ndarray.argmin` (*self*, *axis*). Return the index of the smallest element of *self* along *axis*. -.. c:function:: PyObject* PyArray_ArgMinKeepdims( \ - PyArrayObject* self, int axis, PyArrayObject* out) +.. c:function:: PyObject* PyArray_ArgMinWithKeepdims( \ + PyArrayObject* self, int axis, PyArrayObject* out, int keepdims) Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the smallest element of *self* along *axis*. The number of dimensions of the result matches with the - that of the input object. + that of the input object if keepdims evaluates to true. .. c:function:: PyObject* PyArray_Max( \ PyArrayObject* self, int axis, PyArrayObject* out) diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index 70aaf9f0a744..e11df52b876a 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -646,7 +646,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgMax (ndarray, int, ndarray) object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) - object PyArray_ArgMinKeepdims (ndarray, int, ndarray) + object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index edd8cc156001..1726bbc7cc07 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -604,7 +604,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgMax (ndarray, int, ndarray) object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) object PyArray_ArgMin (ndarray, int, ndarray) - object PyArray_ArgMinKeepdims (ndarray, int, ndarray) + object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/core/code_generators/numpy_api.py b/numpy/core/code_generators/numpy_api.py index fbd3233680fa..f3479b91fac0 100644 --- a/numpy/core/code_generators/numpy_api.py +++ b/numpy/core/code_generators/numpy_api.py @@ -350,6 +350,7 @@ 'PyArray_ResolveWritebackIfCopy': (302,), 'PyArray_SetWritebackIfCopyBase': (303,), # End 1.14 API + 'PyArray_ArgMinWithKeepdims': (304,), } ufunc_types_api = { diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index da75ff247b52..7c7339a369f0 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -289,7 +289,7 @@ PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { * ArgMin */ NPY_NO_EXPORT PyObject * -PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) +PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; @@ -297,11 +297,16 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) npy_intp *rptr; npy_intp i, n, m; int elsize; + int axis_copy = axis; + npy_intp _shape_buf[NPY_MAXDIMS]; + npy_intp *out_shape; + int out_ndim = PyArray_NDIM(op); NPY_BEGIN_THREADS_DEF; if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { return NULL; } + /* * We need to permute the array so that axis is placed at the end. * And all other dimensions are shifted left. @@ -337,6 +342,23 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) if (ap == NULL) { return NULL; } + + if (!keepdims) { + out_ndim = PyArray_NDIM(ap) - 1; + out_shape = PyArray_DIMS(ap); + } else { + out_shape = _shape_buf; + if (axis_copy == NPY_MAXDIMS) { + for( int i = 0; i < out_ndim; i++ ) { + out_shape[i] = 1; + } + } else { + out_ndim = PyArray_NDIM(ap); + memcpy(out_shape, PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); + out_shape[out_ndim - 1] = 1; + } + } + arg_func = PyArray_DESCR(ap)->f->argmin; if (arg_func == NULL) { PyErr_SetString(PyExc_TypeError, @@ -354,20 +376,40 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) if (!out) { rp = (PyArrayObject *)PyArray_NewFromDescr( Py_TYPE(ap), PyArray_DescrFromType(NPY_INTP), - PyArray_NDIM(ap) - 1, PyArray_DIMS(ap), NULL, NULL, + out_ndim, out_shape, NULL, NULL, 0, (PyObject *)ap); if (rp == NULL) { goto fail; } } else { - if ((PyArray_NDIM(out) != PyArray_NDIM(ap) - 1) || - !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), - PyArray_NDIM(out))) { + int is_out_shape_good = PyArray_NDIM(out) == out_ndim; + for( int i = 0, j = 0; is_out_shape_good && i < out_ndim; i++ ) { + if( keepdims ) { + if( i == axis || axis_copy == NPY_MAXDIMS ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; + j++; + } + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; + j++; + } + } + if ( !is_out_shape_good ) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); goto fail; } + if( keepdims && axis_copy != NPY_MAXDIMS ) { + memcpy(PyArray_DIMS(out), PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); + PyArray_DIMS(out)[out_ndim - 1] = 1; + memcpy(PyArray_STRIDES(out), PyArray_STRIDES(ap), out_ndim * sizeof(npy_intp)); + for( int i = 0; i < PyArray_NDIM(out); i++ ) { + PyArray_STRIDES(out)[i] /= PyArray_DIMS(ap)[out_ndim - 1]; + } + } rp = (PyArrayObject *)PyArray_FromArray(out, PyArray_DescrFromType(NPY_INTP), NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); @@ -393,6 +435,22 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) rp = out; Py_INCREF(rp); } + if( axis_copy == NPY_MAXDIMS ) { + return (PyObject *)rp; + } + if( keepdims ) { + for( int i = 0, k = 0; i < out_ndim; i++ ) { + if( i != axis ) { + PyArray_DIMS(rp)[i] = PyArray_DIMS(ap)[k]; + k++; + } + } + PyArray_DIMS(rp)[axis] = 1; + PyArray_STRIDES(rp)[out_ndim - 1] = PyArray_DESCR(rp)->elsize; + for( int i = out_ndim - 2; i >= 0; i-- ) { + PyArray_STRIDES(rp)[i] = PyArray_STRIDES(rp)[i + 1] * PyArray_DIMS(rp)[i + 1]; + } + } return (PyObject *)rp; fail: @@ -401,136 +459,13 @@ PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) return NULL; } -NPY_NO_EXPORT PyObject* -PyArray_ArgMinKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { - PyArrayObject* ret = NULL; - - if( out == NULL ) { - /* - * Case 1: When out is not provided in the input. - */ - ret = (PyArrayObject*) PyArray_ArgMin(op, axis, NULL); - if( ret == NULL ) { - return NULL; - } - ((PyArrayObject_fields*)ret)->nd = PyArray_NDIM(op); - npy_intp* ret_DIMS = npy_alloc_cache_dim(NPY_MAXDIMS); - npy_intp* ret_STRIDES = npy_alloc_cache_dim(NPY_MAXDIMS); - if( axis == NPY_MAXDIMS ) { - /* - * Change dimensions and strides so that resulting - * array has shape, (1, 1, ..., 1) - */ - for( int i = 0; i < PyArray_NDIM(op); i++ ) { - ret_DIMS[i] = 1; - ret_STRIDES[i] = PyArray_DESCR(ret)->elsize; - } - } else { - /* - * Change dimensions and strides so that considered - * axis has size 1 in the resulting array. - */ - check_and_adjust_axis(&axis, PyArray_NDIM(op)); - for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { - ret_DIMS[i] = PyArray_DIMS(op)[i]; - if( i != axis ) { - ret_STRIDES[i] = PyArray_STRIDES(ret)[k]; - k++; - } - } - ret_DIMS[axis] = 1; - if( axis < PyArray_NDIM(op) - 1 ) { - ret_STRIDES[axis] = ret_STRIDES[axis + 1]*ret_DIMS[axis + 1]; - } else { - ret_STRIDES[axis] = PyArray_DESCR(ret)->elsize; - } - } - ((PyArrayObject_fields*)ret)->dimensions = ret_DIMS; - ((PyArrayObject_fields*)ret)->strides = ret_STRIDES; - } else { - /* - * Case 2: When out is provided in the input. - */ - if( axis == NPY_MAXDIMS ) { - /* - * `out` array should have same number of dimensions as - * input array and all the axes have size 1. - */ - int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); - for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } - if( !is_out_shape_good ) { - PyErr_SetString(PyExc_ValueError, - "output array does not match result of np.argmax."); - return NULL; - } - /* - * Set the number of dimensions to 0 so that `PyArray_ArgMin` - * perceives `out` as a scalar. - */ - ((PyArrayObject_fields*)out)->nd = 0; - PyArray_ArgMin(op, axis, out); - /* - * Set the number of dimensions to original so that `out` - * is perceived correctly by the user. - */ - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); - } else { - /* - * Check and fix the axis for negative values. - */ - if( PyArray_CheckAxis(op, &axis, 0) == NULL ) { - return NULL; - } - int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); - for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { - if( i == axis ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(op)[i]; - } - } - if( !is_out_shape_good ) { - PyErr_SetString(PyExc_ValueError, - "output array does not match result of np.argmax."); - return NULL; - } - /* - * Converts the shape and strides such that out.shape - * is transformed from, (d1, d2, ..., 1, ..., dn) to - * (d1, d2, ..., dn). That is axis of size 1 is removed. - * This is done so that `PyArray_ArgMin` perceives it correctly. - */ - for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { - if( i != axis ) { - PyArray_DIMS(out)[k] = PyArray_DIMS(out)[i]; - PyArray_STRIDES(out)[k] = PyArray_STRIDES(out)[i]; - k++; - } - } - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op) - 1; - PyArray_ArgMin(op, axis, out); - /* - * Once the results are stored in `out`, the shape and - * strides of `out` are restored. - */ - for( int i = PyArray_NDIM(out) - 1; i >= axis; i-- ) { - PyArray_DIMS(out)[i + 1] = PyArray_DIMS(out)[i]; - PyArray_STRIDES(out)[i + 1] = PyArray_STRIDES(out)[i]; - } - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); - PyArray_DIMS(out)[axis] = 1; - if( axis < PyArray_NDIM(op) - 1 ) { - PyArray_STRIDES(out)[axis] = PyArray_STRIDES(out)[axis + 1]*PyArray_DIMS(out)[axis + 1]; - } else { - PyArray_STRIDES(out)[axis] = PyArray_DESCR(out)->elsize; - } - } - // `ret` and `out` refer to the same object. - ret = out; - } - return (PyObject*)ret; +/*NUMPY_API + * ArgMin + */ +NPY_NO_EXPORT PyObject * +PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) +{ + return PyArray_ArgMinWithKeepdims(op, axis, out, 0); } /*NUMPY_API diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index e3762c8a91df..f801f014f22e 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -11,7 +11,7 @@ NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* -PyArray_ArgMinKeepdims(PyArrayObject* self, int axis, PyArrayObject *out); +PyArray_ArgMinWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); NPY_NO_EXPORT PyObject* PyArray_Max(PyArrayObject* self, int axis, PyArrayObject* out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index ece301a0d318..a8348eb4ec4e 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -328,12 +328,7 @@ array_argmin(PyArrayObject *self, return NULL; } - PyObject *ret; - if( keepdims ) { - ret = PyArray_ArgMinKeepdims(self, axis, out); - } else { - ret = PyArray_ArgMin(self, axis, out); - } + PyObject *ret = PyArray_ArgMinWithKeepdims(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { From 124abe3a928eb03a148fe70018b9e7989b90d652 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sat, 19 Jun 2021 12:20:20 +0530 Subject: [PATCH 20/37] matched implementation of PyArray_ArgMaxKeepdims and PyArray_ArgMinKeepdims --- doc/source/reference/c-api/array.rst | 6 +- numpy/__init__.cython-30.pxd | 2 +- numpy/__init__.pxd | 2 +- numpy/core/code_generators/numpy_api.py | 1 + numpy/core/src/multiarray/calculation.c | 245 ++++++++++-------------- numpy/core/src/multiarray/calculation.h | 2 +- numpy/core/src/multiarray/methods.c | 7 +- 7 files changed, 112 insertions(+), 153 deletions(-) diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index d96b52d32ce2..a86ced91234d 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2019,12 +2019,12 @@ Calculation Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the largest element of *self* along *axis*. -.. c:function:: PyObject* PyArray_ArgMaxKeepdims( \ - PyArrayObject* self, int axis, PyArrayObject* out) +.. c:function:: PyObject* PyArray_ArgMaxWithKeepdims( \ + PyArrayObject* self, int axis, PyArrayObject* out, int keepdims) Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the largest element of *self* along *axis*. The number of dimensions of the result matches with the - that of the input object. + that of the input object if keepdims evaluates to true. .. c:function:: PyObject* PyArray_ArgMin( \ PyArrayObject* self, int axis, PyArrayObject* out) diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index e11df52b876a..3803ae87fb0d 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -644,7 +644,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) object PyArray_ArgMax (ndarray, int, ndarray) - object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) + object PyArray_ArgMaxWithKeepdims (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index 1726bbc7cc07..75b8b151b190 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -602,7 +602,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) object PyArray_ArgMax (ndarray, int, ndarray) - object PyArray_ArgMaxKeepdims (ndarray, int, ndarray) + object PyArray_ArgMaxWithKeepdims (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) diff --git a/numpy/core/code_generators/numpy_api.py b/numpy/core/code_generators/numpy_api.py index f3479b91fac0..f5c54f01db14 100644 --- a/numpy/core/code_generators/numpy_api.py +++ b/numpy/core/code_generators/numpy_api.py @@ -351,6 +351,7 @@ 'PyArray_SetWritebackIfCopyBase': (303,), # End 1.14 API 'PyArray_ArgMinWithKeepdims': (304,), + 'PyArray_ArgMaxWithKeepdims': (305,), } ufunc_types_api = { diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 7c7339a369f0..2e9b7afbf601 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -41,7 +41,7 @@ power_of_ten(int n) * ArgMax */ NPY_NO_EXPORT PyObject * -PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) +PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; @@ -49,11 +49,19 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) npy_intp *rptr; npy_intp i, n, m; int elsize; + // Keep a copy because axis changes via call to PyArray_CheckAxis + int axis_copy = axis; + npy_intp _shape_buf[NPY_MAXDIMS]; + npy_intp *out_shape; + // Keep the number of dimensions in original array + // Helps when axis is None. + int out_ndim = PyArray_NDIM(op); NPY_BEGIN_THREADS_DEF; if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { return NULL; } + /* * We need to permute the array so that axis is placed at the end. * And all other dimensions are shifted left. @@ -61,15 +69,15 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) if (axis != PyArray_NDIM(ap)-1) { PyArray_Dims newaxes; npy_intp dims[NPY_MAXDIMS]; - int j; + int i; newaxes.ptr = dims; newaxes.len = PyArray_NDIM(ap); - for (j = 0; j < axis; j++) { - dims[j] = j; + for (i = 0; i < axis; i++) { + dims[i] = i; } - for (j = axis; j < PyArray_NDIM(ap) - 1; j++) { - dims[j] = j + 1; + for (i = axis; i < PyArray_NDIM(ap) - 1; i++) { + dims[i] = i + 1; } dims[PyArray_NDIM(ap) - 1] = axis; op = (PyArrayObject *)PyArray_Transpose(ap, &newaxes); @@ -89,6 +97,24 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) if (ap == NULL) { return NULL; } + + // Decides the shape of the output array. + if (!keepdims) { + out_ndim = PyArray_NDIM(ap) - 1; + out_shape = PyArray_DIMS(ap); + } else { + out_shape = _shape_buf; + if (axis_copy == NPY_MAXDIMS) { + for( int i = 0; i < out_ndim; i++ ) { + out_shape[i] = 1; + } + } else { + out_ndim = PyArray_NDIM(ap); + memcpy(out_shape, PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); + out_shape[out_ndim - 1] = 1; + } + } + arg_func = PyArray_DESCR(ap)->f->argmax; if (arg_func == NULL) { PyErr_SetString(PyExc_TypeError, @@ -106,20 +132,46 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) if (!out) { rp = (PyArrayObject *)PyArray_NewFromDescr( Py_TYPE(ap), PyArray_DescrFromType(NPY_INTP), - PyArray_NDIM(ap) - 1, PyArray_DIMS(ap), NULL, NULL, + out_ndim, out_shape, NULL, NULL, 0, (PyObject *)ap); if (rp == NULL) { goto fail; } } else { - if ((PyArray_NDIM(out) != PyArray_NDIM(ap) - 1) || - !PyArray_CompareLists(PyArray_DIMS(out), PyArray_DIMS(ap), - PyArray_NDIM(out))) { + int is_out_shape_good = PyArray_NDIM(out) == out_ndim; + for( int i = 0, j = 0; is_out_shape_good && i < out_ndim; i++ ) { + if( keepdims ) { + if( i == axis || axis_copy == NPY_MAXDIMS ) { + is_out_shape_good = PyArray_DIMS(out)[i] == 1; + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; + j++; + } + } else { + is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; + j++; + } + } + if ( !is_out_shape_good ) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmax."); goto fail; } + /* + * Match the shape of the output array with that of `ap`. + * The reason is because the code below after this else + * block doesn't know whether `rp` is obtained from user + * input or is created manually in the associated if block. + */ + if( keepdims && axis_copy != NPY_MAXDIMS ) { + memcpy(PyArray_DIMS(out), PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); + PyArray_DIMS(out)[out_ndim - 1] = 1; + memcpy(PyArray_STRIDES(out), PyArray_STRIDES(ap), out_ndim * sizeof(npy_intp)); + for( int i = 0; i < PyArray_NDIM(out); i++ ) { + PyArray_STRIDES(out)[i] /= PyArray_DIMS(ap)[out_ndim - 1]; + } + } rp = (PyArrayObject *)PyArray_FromArray(out, PyArray_DescrFromType(NPY_INTP), NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); @@ -138,13 +190,33 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) NPY_END_THREADS_DESCR(PyArray_DESCR(ap)); Py_DECREF(ap); - /* Trigger the UPDATEIFCOPY/WRTIEBACKIFCOPY if necessary */ + /* Trigger the UPDATEIFCOPY/WRITEBACKIFCOPY if necessary */ if (out != NULL && out != rp) { PyArray_ResolveWritebackIfCopy(rp); Py_DECREF(rp); rp = out; Py_INCREF(rp); } + /* + * If axis is None then no need to fix the dimensions + * and strides of the output array. + */ + if( axis_copy == NPY_MAXDIMS ) { + return (PyObject *)rp; + } + if( keepdims ) { + for( int i = 0, k = 0; i < out_ndim; i++ ) { + if( i != axis ) { + PyArray_DIMS(rp)[i] = PyArray_DIMS(ap)[k]; + k++; + } + } + PyArray_DIMS(rp)[axis] = 1; + PyArray_STRIDES(rp)[out_ndim - 1] = PyArray_DESCR(rp)->elsize; + for( int i = out_ndim - 2; i >= 0; i-- ) { + PyArray_STRIDES(rp)[i] = PyArray_STRIDES(rp)[i + 1] * PyArray_DIMS(rp)[i + 1]; + } + } return (PyObject *)rp; fail: @@ -153,136 +225,13 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) return NULL; } -NPY_NO_EXPORT PyObject* -PyArray_ArgMaxKeepdims(PyArrayObject *op, int axis, PyArrayObject* out) { - PyArrayObject* ret = NULL; - - if( out == NULL ) { - /* - * Case 1: When out is not provided in the input. - */ - ret = (PyArrayObject*) PyArray_ArgMax(op, axis, NULL); - if( ret == NULL ) { - return NULL; - } - ((PyArrayObject_fields*)ret)->nd = PyArray_NDIM(op); - npy_intp* ret_DIMS = npy_alloc_cache_dim(NPY_MAXDIMS); - npy_intp* ret_STRIDES = npy_alloc_cache_dim(NPY_MAXDIMS); - if( axis == NPY_MAXDIMS ) { - /* - * Change dimensions and strides so that resulting - * array has shape, (1, 1, ..., 1) - */ - for( int i = 0; i < PyArray_NDIM(op); i++ ) { - ret_DIMS[i] = 1; - ret_STRIDES[i] = PyArray_DESCR(ret)->elsize; - } - } else { - /* - * Change dimensions and strides so that considered - * axis has size 1 in the resulting array. - */ - check_and_adjust_axis(&axis, PyArray_NDIM(op)); - for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { - ret_DIMS[i] = PyArray_DIMS(op)[i]; - if( i != axis ) { - ret_STRIDES[i] = PyArray_STRIDES(ret)[k]; - k++; - } - } - ret_DIMS[axis] = 1; - if( axis < PyArray_NDIM(op) - 1 ) { - ret_STRIDES[axis] = ret_STRIDES[axis + 1]*ret_DIMS[axis + 1]; - } else { - ret_STRIDES[axis] = PyArray_DESCR(ret)->elsize; - } - } - ((PyArrayObject_fields*)ret)->dimensions = ret_DIMS; - ((PyArrayObject_fields*)ret)->strides = ret_STRIDES; - } else { - /* - * Case 2: When out is provided in the input. - */ - if( axis == NPY_MAXDIMS ) { - /* - * `out` array should have same number of dimensions as - * input array and all the axes have size 1. - */ - int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); - for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } - if( !is_out_shape_good ) { - PyErr_SetString(PyExc_ValueError, - "output array does not match result of np.argmax."); - return NULL; - } - /* - * Set the number of dimensions to 0 so that `PyArray_ArgMax` - * perceives `out` as a scalar. - */ - ((PyArrayObject_fields*)out)->nd = 0; - PyArray_ArgMax(op, axis, out); - /* - * Set the number of dimensions to original so that `out` - * is perceived correctly by the user. - */ - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); - } else { - /* - * Check and fix the axis for negative values. - */ - if( PyArray_CheckAxis(op, &axis, 0) == NULL ) { - return NULL; - } - int is_out_shape_good = PyArray_NDIM(out) == PyArray_NDIM(op); - for( int i = 0; is_out_shape_good && i < PyArray_NDIM(op); i++ ) { - if( i == axis ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(op)[i]; - } - } - if( !is_out_shape_good ) { - PyErr_SetString(PyExc_ValueError, - "output array does not match result of np.argmax."); - return NULL; - } - /* - * Converts the shape and strides such that out.shape - * is transformed from, (d1, d2, ..., 1, ..., dn) to - * (d1, d2, ..., dn). That is axis of size 1 is removed. - * This is done so that `PyArray_ArgMax` perceives it correctly. - */ - for( int i = 0, k = 0; i < PyArray_NDIM(op); i++ ) { - if( i != axis ) { - PyArray_DIMS(out)[k] = PyArray_DIMS(out)[i]; - PyArray_STRIDES(out)[k] = PyArray_STRIDES(out)[i]; - k++; - } - } - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op) - 1; - PyArray_ArgMax(op, axis, out); - /* - * Once the results are stored in `out`, the shape and - * strides of `out` are restored. - */ - for( int i = PyArray_NDIM(out) - 1; i >= axis; i-- ) { - PyArray_DIMS(out)[i + 1] = PyArray_DIMS(out)[i]; - PyArray_STRIDES(out)[i + 1] = PyArray_STRIDES(out)[i]; - } - ((PyArrayObject_fields*)out)->nd = PyArray_NDIM(op); - PyArray_DIMS(out)[axis] = 1; - if( axis < PyArray_NDIM(op) - 1 ) { - PyArray_STRIDES(out)[axis] = PyArray_STRIDES(out)[axis + 1]*PyArray_DIMS(out)[axis + 1]; - } else { - PyArray_STRIDES(out)[axis] = PyArray_DESCR(out)->elsize; - } - } - // `ret` and `out` refer to the same object. - ret = out; - } - return (PyObject*)ret; +/*NUMPY_API + * ArgMax + */ +NPY_NO_EXPORT PyObject * +PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) +{ + return PyArray_ArgMaxWithKeepdims(op, axis, out, 0); } /*NUMPY_API @@ -297,9 +246,12 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int npy_intp *rptr; npy_intp i, n, m; int elsize; + // Keep a copy because axis changes via call to PyArray_CheckAxis int axis_copy = axis; npy_intp _shape_buf[NPY_MAXDIMS]; npy_intp *out_shape; + // Keep the number of dimensions in original array + // Helps when axis is None. int out_ndim = PyArray_NDIM(op); NPY_BEGIN_THREADS_DEF; @@ -343,6 +295,7 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int return NULL; } + // Decides the shape of the output array. if (!keepdims) { out_ndim = PyArray_NDIM(ap) - 1; out_shape = PyArray_DIMS(ap); @@ -402,6 +355,12 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int "output array does not match result of np.argmin."); goto fail; } + /* + * Match the shape of the output array with that of `ap`. + * The reason is because the code below after this else + * block doesn't know whether `rp` is obtained from user + * input or is created manually in the associated if block. + */ if( keepdims && axis_copy != NPY_MAXDIMS ) { memcpy(PyArray_DIMS(out), PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); PyArray_DIMS(out)[out_ndim - 1] = 1; @@ -435,6 +394,10 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int rp = out; Py_INCREF(rp); } + /* + * If axis is None then no need to fix the dimensions + * and strides of the output array. + */ if( axis_copy == NPY_MAXDIMS ) { return (PyObject *)rp; } diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index f801f014f22e..c3498e878aae 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -5,7 +5,7 @@ NPY_NO_EXPORT PyObject* PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* -PyArray_ArgMaxKeepdims(PyArrayObject* self, int axis, PyArrayObject *out); +PyArray_ArgMaxWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index a8348eb4ec4e..d1e21fd53a4e 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -295,12 +295,7 @@ array_argmax(PyArrayObject *self, return NULL; } - PyObject *ret; - if( keepdims ) { - ret = PyArray_ArgMaxKeepdims(self, axis, out); - } else { - ret = PyArray_ArgMax(self, axis, out); - } + PyObject *ret = PyArray_ArgMaxWithKeepdims(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { From 6bd9f4c3db182b87c7adbbc90405c6f7eb23783e Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sun, 20 Jun 2021 01:46:28 +0530 Subject: [PATCH 21/37] simplified implementation --- numpy/core/src/multiarray/calculation.c | 92 ++++--------------------- 1 file changed, 14 insertions(+), 78 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 2e9b7afbf601..7c494bdfe143 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -53,8 +53,9 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int int axis_copy = axis; npy_intp _shape_buf[NPY_MAXDIMS]; npy_intp *out_shape; - // Keep the number of dimensions in original array - // Helps when axis is None. + // Keep the number of dimensions and shape of + // original array. Helps when `keepdims` is True. + npy_intp* original_op_shape = PyArray_DIMS(op); int out_ndim = PyArray_NDIM(op); NPY_BEGIN_THREADS_DEF; @@ -109,9 +110,8 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int out_shape[i] = 1; } } else { - out_ndim = PyArray_NDIM(ap); - memcpy(out_shape, PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); - out_shape[out_ndim - 1] = 1; + memcpy(out_shape, original_op_shape, out_ndim * sizeof(npy_intp)); + out_shape[axis] = 1; } } @@ -158,20 +158,6 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int "output array does not match result of np.argmax."); goto fail; } - /* - * Match the shape of the output array with that of `ap`. - * The reason is because the code below after this else - * block doesn't know whether `rp` is obtained from user - * input or is created manually in the associated if block. - */ - if( keepdims && axis_copy != NPY_MAXDIMS ) { - memcpy(PyArray_DIMS(out), PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); - PyArray_DIMS(out)[out_ndim - 1] = 1; - memcpy(PyArray_STRIDES(out), PyArray_STRIDES(ap), out_ndim * sizeof(npy_intp)); - for( int i = 0; i < PyArray_NDIM(out); i++ ) { - PyArray_STRIDES(out)[i] /= PyArray_DIMS(ap)[out_ndim - 1]; - } - } rp = (PyArrayObject *)PyArray_FromArray(out, PyArray_DescrFromType(NPY_INTP), NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); @@ -197,26 +183,6 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int rp = out; Py_INCREF(rp); } - /* - * If axis is None then no need to fix the dimensions - * and strides of the output array. - */ - if( axis_copy == NPY_MAXDIMS ) { - return (PyObject *)rp; - } - if( keepdims ) { - for( int i = 0, k = 0; i < out_ndim; i++ ) { - if( i != axis ) { - PyArray_DIMS(rp)[i] = PyArray_DIMS(ap)[k]; - k++; - } - } - PyArray_DIMS(rp)[axis] = 1; - PyArray_STRIDES(rp)[out_ndim - 1] = PyArray_DESCR(rp)->elsize; - for( int i = out_ndim - 2; i >= 0; i-- ) { - PyArray_STRIDES(rp)[i] = PyArray_STRIDES(rp)[i + 1] * PyArray_DIMS(rp)[i + 1]; - } - } return (PyObject *)rp; fail: @@ -250,8 +216,9 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int int axis_copy = axis; npy_intp _shape_buf[NPY_MAXDIMS]; npy_intp *out_shape; - // Keep the number of dimensions in original array - // Helps when axis is None. + // Keep the number of dimensions and shape of + // original array. Helps when `keepdims` is True. + npy_intp* original_op_shape = PyArray_DIMS(op); int out_ndim = PyArray_NDIM(op); NPY_BEGIN_THREADS_DEF; @@ -306,9 +273,12 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int out_shape[i] = 1; } } else { - out_ndim = PyArray_NDIM(ap); - memcpy(out_shape, PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); - out_shape[out_ndim - 1] = 1; + /* + * While `ap` may be transposed, we can ignore this for `out` because the + * transpose only reorders the size 1 `axis` (not changing memory layout). + */ + memcpy(out_shape, original_op_shape, out_ndim * sizeof(npy_intp)); + out_shape[axis] = 1; } } @@ -355,20 +325,6 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int "output array does not match result of np.argmin."); goto fail; } - /* - * Match the shape of the output array with that of `ap`. - * The reason is because the code below after this else - * block doesn't know whether `rp` is obtained from user - * input or is created manually in the associated if block. - */ - if( keepdims && axis_copy != NPY_MAXDIMS ) { - memcpy(PyArray_DIMS(out), PyArray_DIMS(ap), out_ndim * sizeof(npy_intp)); - PyArray_DIMS(out)[out_ndim - 1] = 1; - memcpy(PyArray_STRIDES(out), PyArray_STRIDES(ap), out_ndim * sizeof(npy_intp)); - for( int i = 0; i < PyArray_NDIM(out); i++ ) { - PyArray_STRIDES(out)[i] /= PyArray_DIMS(ap)[out_ndim - 1]; - } - } rp = (PyArrayObject *)PyArray_FromArray(out, PyArray_DescrFromType(NPY_INTP), NPY_ARRAY_CARRAY | NPY_ARRAY_WRITEBACKIFCOPY); @@ -394,26 +350,6 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int rp = out; Py_INCREF(rp); } - /* - * If axis is None then no need to fix the dimensions - * and strides of the output array. - */ - if( axis_copy == NPY_MAXDIMS ) { - return (PyObject *)rp; - } - if( keepdims ) { - for( int i = 0, k = 0; i < out_ndim; i++ ) { - if( i != axis ) { - PyArray_DIMS(rp)[i] = PyArray_DIMS(ap)[k]; - k++; - } - } - PyArray_DIMS(rp)[axis] = 1; - PyArray_STRIDES(rp)[out_ndim - 1] = PyArray_DESCR(rp)->elsize; - for( int i = out_ndim - 2; i >= 0; i-- ) { - PyArray_STRIDES(rp)[i] = PyArray_STRIDES(rp)[i + 1] * PyArray_DIMS(rp)[i + 1]; - } - } return (PyObject *)rp; fail: From 55a85d3eb771409c19c59e3285b6c3af4c8417e7 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sun, 20 Jun 2021 01:48:33 +0530 Subject: [PATCH 22/37] Added missing comment --- numpy/core/src/multiarray/calculation.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 7c494bdfe143..4b11a63c1bba 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -110,6 +110,10 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int out_shape[i] = 1; } } else { + /* + * While `ap` may be transposed, we can ignore this for `out` because the + * transpose only reorders the size 1 `axis` (not changing memory layout). + */ memcpy(out_shape, original_op_shape, out_ndim * sizeof(npy_intp)); out_shape[axis] = 1; } From 568251ecf94a6dfb13308f951402221111461041 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Sun, 20 Jun 2021 01:51:03 +0530 Subject: [PATCH 23/37] removed unwanted header --- numpy/core/src/multiarray/calculation.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 4b11a63c1bba..625b0ef4456c 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -17,9 +17,6 @@ #include "calculation.h" #include "array_assign.h" -#include "alloc.h" - - static double power_of_ten(int n) { From 9260d40ad8c2ef0b2cb7b35b2b65385ce38bcad1 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Mon, 21 Jun 2021 12:02:08 +0530 Subject: [PATCH 24/37] addressed all the reviews --- .../upcoming_changes/19211.new_feature.rst | 6 +- numpy/__init__.pyi | 6 - numpy/core/fromnumeric.py | 32 ++-- numpy/core/fromnumeric.pyi | 4 - numpy/core/src/multiarray/calculation.c | 58 +++---- numpy/core/src/multiarray/methods.c | 7 +- numpy/core/tests/test_multiarray.py | 149 ++++++++++-------- numpy/ma/core.py | 4 +- numpy/ma/core.pyi | 4 +- 9 files changed, 122 insertions(+), 148 deletions(-) diff --git a/doc/release/upcoming_changes/19211.new_feature.rst b/doc/release/upcoming_changes/19211.new_feature.rst index 1833d533e15f..dc1a3475df2e 100644 --- a/doc/release/upcoming_changes/19211.new_feature.rst +++ b/doc/release/upcoming_changes/19211.new_feature.rst @@ -1,9 +1,9 @@ -`keepdims` optional argument added to `numpy.argmin`, `numpy.argmax` +``keepdims`` optional argument added to `numpy.argmin`, `numpy.argmax` -------------------------------------------------------------------- -`keepdims` argument has been added to `numpy.argmin`, `numpy.argmax`. +``keepdims`` argument has been added to `numpy.argmin`, `numpy.argmax`. By default, it is `False` and hence no behaviour change should be expected -in existing codes using `numpy.argmin`, `numpy.argmax`. If set to `True`, +in existing codes using `numpy.argmin`, `numpy.argmax`. If set to ``True``, the axes which are reduced are left in the result as dimensions with size one. In simple words, the resulting array will be having same dimensions as the input array. \ No newline at end of file diff --git a/numpy/__init__.pyi b/numpy/__init__.pyi index 2442d1b4ebf5..4af5710d4524 100644 --- a/numpy/__init__.pyi +++ b/numpy/__init__.pyi @@ -1299,21 +1299,18 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., - keepdims: L[False] = ..., ) -> intp: ... @overload def argmax( self, axis: _ShapeLike = ..., out: None = ..., - keepdims: bool = ..., ) -> Any: ... @overload def argmax( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., - keepdims: bool = ..., ) -> _NdArraySubClass: ... @overload @@ -1321,21 +1318,18 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., - keepdims: L[False] = ..., ) -> intp: ... @overload def argmin( self, axis: _ShapeLike = ..., out: None = ..., - keepdims: bool = ..., ) -> Any: ... @overload def argmin( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., - keepdims: bool = ..., ) -> _NdArraySubClass: ... def argsort( diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index 26747fa78d1a..5d54597c25f8 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -49,12 +49,14 @@ def _wrapit(obj, method, *args, **kwds): def _wrapfunc(obj, method, *args, **kwds): + passkwargs = {k: v for k, v in kwds.items() + if v is not np._NoValue} bound = getattr(obj, method, None) if bound is None: - return _wrapit(obj, method, *args, **kwds) + return _wrapit(obj, method, *args, **passkwargs) try: - return bound(*args, **kwds) + return bound(*args, **passkwargs) except TypeError: # A TypeError occurs if the object does have such a method in its # class, but its signature is not identical to that of NumPy's. This @@ -63,7 +65,7 @@ def _wrapfunc(obj, method, *args, **kwds): # # Call _wrapit from within the except clause to ensure a potential # exception has a traceback chain. - return _wrapit(obj, method, *args, **kwds) + return _wrapit(obj, method, *args, **passkwargs) def _wrapreduction(obj, ufunc, method, axis, dtype, out, **kwargs): @@ -1114,12 +1116,12 @@ def argsort(a, axis=-1, kind=None, order=None): return _wrapfunc(a, 'argsort', axis=axis, kind=kind, order=order) -def _argmax_dispatcher(a, axis=None, out=None, keepdims=None): - return (a, out, keepdims) +def _argmax_dispatcher(a, axis=None, out=None, *, keepdims=np._NoValue): + return (a, out) @array_function_dispatch(_argmax_dispatcher) -def argmax(a, axis=None, out=None, keepdims=False): +def argmax(a, axis=None, out=None, *, keepdims=np._NoValue): """ Returns the indices of the maximum values along an axis. @@ -1134,9 +1136,9 @@ def argmax(a, axis=None, out=None, keepdims=False): If provided, the result will be inserted into this array. It should be of the appropriate shape and dtype. keepdims : bool, optional - If this is set to True, the axes which are reduced are left - in the result as dimensions with size one. With this option, - the result will broadcast correctly against the array. + If this is set to True, the axes which are reduced are left + in the result as dimensions with size one. With this option, + the result will broadcast correctly against the array. Returns ------- @@ -1204,18 +1206,16 @@ def argmax(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ - if isinstance(a, np.matrix): - return _wrapfunc(a, 'argmax', axis=axis, out=out) return _wrapfunc(a, 'argmax', axis=axis, out=out, keepdims=keepdims) -def _argmin_dispatcher(a, axis=None, out=None, keepdims=None): - return (a, out, keepdims) +def _argmin_dispatcher(a, axis=None, out=None, *, keepdims=np._NoValue): + return (a, out) @array_function_dispatch(_argmin_dispatcher) -def argmin(a, axis=None, out=None, keepdims=False): +def argmin(a, axis=None, out=None, *, keepdims=np._NoValue): """ Returns the indices of the minimum values along an axis. @@ -1300,9 +1300,7 @@ def argmin(a, axis=None, out=None, keepdims=False): >>> res.shape (2, 1, 4) """ - if isinstance(a, np.matrix): - return _wrapfunc(a, 'argmin', axis=axis, out=out) - return _wrapfunc(a, 'argmin', axis=axis, out=out, keepdims=keepdims) + return _wrapfunc(a, 'argmin', axis, out=out, keepdims=keepdims) def _searchsorted_dispatcher(a, v, side=None, sorter=None): diff --git a/numpy/core/fromnumeric.pyi b/numpy/core/fromnumeric.pyi index 775e44a68533..3342ec3ac47b 100644 --- a/numpy/core/fromnumeric.pyi +++ b/numpy/core/fromnumeric.pyi @@ -130,14 +130,12 @@ def argmax( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., - keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmax( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., - keepdims: bool = ..., ) -> Any: ... @overload @@ -145,14 +143,12 @@ def argmin( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., - keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmin( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., - keepdims: bool = ..., ) -> Any: ... @overload diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 625b0ef4456c..80f7fa5b4890 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -38,7 +38,8 @@ power_of_ten(int n) * ArgMax */ NPY_NO_EXPORT PyObject * -PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) +PyArray_ArgMaxWithKeepdims(PyArrayObject *op, + int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; @@ -59,7 +60,6 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { return NULL; } - /* * We need to permute the array so that axis is placed at the end. * And all other dimensions are shifted left. @@ -67,15 +67,15 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int if (axis != PyArray_NDIM(ap)-1) { PyArray_Dims newaxes; npy_intp dims[NPY_MAXDIMS]; - int i; + int j; newaxes.ptr = dims; newaxes.len = PyArray_NDIM(ap); - for (i = 0; i < axis; i++) { - dims[i] = i; + for (j = 0; j < axis; j++) { + dims[j] = j; } - for (i = axis; i < PyArray_NDIM(ap) - 1; i++) { - dims[i] = i + 1; + for (j = axis; j < PyArray_NDIM(ap) - 1; j++) { + dims[j] = j + 1; } dims[PyArray_NDIM(ap) - 1] = axis; op = (PyArrayObject *)PyArray_Transpose(ap, &newaxes); @@ -103,10 +103,11 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int } else { out_shape = _shape_buf; if (axis_copy == NPY_MAXDIMS) { - for( int i = 0; i < out_ndim; i++ ) { + for (int i = 0; i < out_ndim; i++) { out_shape[i] = 1; } - } else { + } + else { /* * While `ap` may be transposed, we can ignore this for `out` because the * transpose only reorders the size 1 `axis` (not changing memory layout). @@ -140,21 +141,9 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int } } else { - int is_out_shape_good = PyArray_NDIM(out) == out_ndim; - for( int i = 0, j = 0; is_out_shape_good && i < out_ndim; i++ ) { - if( keepdims ) { - if( i == axis || axis_copy == NPY_MAXDIMS ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; - j++; - } - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; - j++; - } - } - if ( !is_out_shape_good ) { + if ((PyArray_NDIM(out) != out_ndim) || + !PyArray_CompareLists(PyArray_DIMS(out), out_shape, + out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmax."); goto fail; @@ -205,7 +194,8 @@ PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) * ArgMin */ NPY_NO_EXPORT PyObject * -PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) +PyArray_ArgMinWithKeepdims(PyArrayObject *op, + int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; PyArray_ArgFunc* arg_func; @@ -307,21 +297,9 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int } } else { - int is_out_shape_good = PyArray_NDIM(out) == out_ndim; - for( int i = 0, j = 0; is_out_shape_good && i < out_ndim; i++ ) { - if( keepdims ) { - if( i == axis || axis_copy == NPY_MAXDIMS ) { - is_out_shape_good = PyArray_DIMS(out)[i] == 1; - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; - j++; - } - } else { - is_out_shape_good = PyArray_DIMS(out)[i] == PyArray_DIMS(ap)[j]; - j++; - } - } - if ( !is_out_shape_good ) { + if ((PyArray_NDIM(out) != out_ndim) || + !PyArray_CompareLists(PyArray_DIMS(out), out_shape, + out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); goto fail; diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index d1e21fd53a4e..ad7464785f0c 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -290,7 +290,7 @@ array_argmax(PyArrayObject *self, if (npy_parse_arguments("argmax", args, len_args, kwnames, "|axis", &PyArray_AxisConverter, &axis, "|out", &PyArray_OutputConverter, &out, - "|keepdims", &PyArray_BoolConverter, &keepdims, + "$keepdims", &PyArray_BoolConverter, &keepdims, NULL, NULL, NULL) < 0) { return NULL; } @@ -314,15 +314,14 @@ array_argmin(PyArrayObject *self, PyArrayObject *out = NULL; npy_bool keepdims = NPY_FALSE; NPY_PREPARE_ARGPARSER; - if (npy_parse_arguments("argmin", args, len_args, kwnames, "|axis", &PyArray_AxisConverter, &axis, "|out", &PyArray_OutputConverter, &out, - "|keepdims", &PyArray_BoolConverter, &keepdims, + "$keepdims", &PyArray_BoolConverter, &keepdims, NULL, NULL, NULL) < 0) { return NULL; } - + PyObject *ret = PyArray_ArgMinWithKeepdims(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index e5f48ca61d9b..33be36c00bb5 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4192,6 +4192,85 @@ def test_unicode(self): assert_array_equal(g1 < g2, [g1[i] < g2[i] for i in [0, 1, 2]]) assert_array_equal(g1 > g2, [g1[i] > g2[i] for i in [0, 1, 2]]) +class TestArgmaxArgminCommon: + + @pytest.mark.parametrize('size', [(), (3,), (3, 2), (2, 3), + (3, 3), (2, 3, 4), (4, 3, 2), + (1, 2, 3, 4), (2, 3, 4, 1), + (3, 4, 1, 2), (4, 1, 2, 3)]) + @pytest.mark.parametrize('method', [np.argmax, np.argmin]) + def test_np_argmin_argmax_keepdims(self, size, method): + + arr = np.random.normal(size=size) + for axis in list(range(-len(size), len(size))) + [None]: + + # contiguous arrays + if axis is None: + new_shape = [1 for _ in range(len(size))] + else: + new_shape = list(size) + new_shape[axis] = 1 + new_shape = tuple(new_shape) + + _res_orig = method(arr, axis=axis) + res_orig = _res_orig.reshape(new_shape) + res = method(arr, axis=axis, keepdims=True) + assert_equal(res, res_orig) + assert_(res.shape == new_shape) + outarray = np.empty(res.shape, dtype=res.dtype) + res1 = method(arr, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) + assert_equal(res, outarray) + + if len(size) > 0: + wrong_shape = list(new_shape) + if axis is not None: + wrong_shape[axis] = 2 + else: + wrong_shape[0] = 2 + wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) + assert_raises(ValueError, method, + arr.T, axis=axis, + out=wrong_outarray, keepdims=True) + + # non-contiguous arrays + if axis is None: + new_shape = [1 for _ in range(len(size))] + else: + new_shape = list(size)[::-1] + new_shape[axis] = 1 + new_shape = tuple(new_shape) + + _res_orig = method(arr.T, axis=axis) + res_orig = _res_orig.reshape(new_shape) + res = method(arr.T, axis=axis, keepdims=True) + assert_equal(res, res_orig) + assert_(res.shape == new_shape) + outarray = np.empty(new_shape[::-1], dtype=res.dtype) + outarray = outarray.T + res1 = method(arr.T, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) + assert_equal(res, outarray) + + if len(size) > 0: + # one dimension lesser for non-zero sized + # array should raise an error + assert_raises(ValueError, method, + arr[0], axis=axis, + out=outarray, keepdims=True) + + if len(size) > 0: + wrong_shape = list(new_shape) + if axis is not None: + wrong_shape[axis] = 2 + else: + wrong_shape[0] = 2 + wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) + assert_raises(ValueError, method, + arr.T, axis=axis, + out=wrong_outarray, keepdims=True) class TestArgmax: @@ -4333,41 +4412,6 @@ def test_object_argmax_with_NULLs(self): assert_equal(a.argmax(), 3) a[1] = 30 assert_equal(a.argmax(), 1) - - def test_np_argmax_keepdims(self): - - sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2), - (1, 2, 3, 4), (2, 3, 4, 1), - (3, 4, 1, 2), (4, 1, 2, 3)] - for size in sizes: - arr = np.random.normal(size=size) - for axis in range(-len(size), len(size)): - res_orig = np.argmax(arr, axis=axis) - new_shape = list(size) - new_shape[axis] = 1 - res_orig = res_orig.reshape(new_shape) - res = np.argmax(arr, axis=axis, keepdims=True) - assert_equal(res, res_orig) - assert_(res.ndim == arr.ndim) - assert_(res.shape[axis] == 1) - outarray = np.empty(res.shape, dtype=res.dtype) - res1 = np.argmax(arr, axis=axis, out=outarray, - keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) - - # Testing for axis=None, keepdims=True - res_orig = np.argmax(arr, axis=None) - res_orig = res_orig.reshape((1,)*arr.ndim) - res = np.argmax(arr, axis=None, keepdims=True) - assert_equal(res, res_orig) - assert_(res.ndim == arr.ndim) - assert_(res.shape == (1,)*arr.ndim) - outarray = np.empty(res.shape, dtype=res.dtype) - res1 = np.argmax(arr, axis=None, out=outarray, keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) class TestArgmin: @@ -4523,41 +4567,6 @@ def test_object_argmin_with_NULLs(self): assert_equal(a.argmin(), 3) a[1] = 10 assert_equal(a.argmin(), 1) - - def test_np_argmin_keepdims(self): - - sizes = [(3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2), - (1, 2, 3, 4), (2, 3, 4, 1), - (3, 4, 1, 2), (4, 1, 2, 3)] - for size in sizes: - arr = np.random.normal(size=size) - for axis in range(-len(size), len(size)): - res_orig = np.argmin(arr, axis=axis) - new_shape = list(size) - new_shape[axis] = 1 - res_orig = res_orig.reshape(new_shape) - res = np.argmin(arr, axis=axis, keepdims=True) - assert_equal(res, res_orig) - assert_(res.ndim == arr.ndim) - assert_(res.shape[axis] == 1) - outarray = np.empty(res.shape, dtype=res.dtype) - res1 = np.argmin(arr, axis=axis, out=outarray, - keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) - - # Testing for axis=None, keepdims=True - res_orig = np.argmin(arr, axis=None) - res_orig = res_orig.reshape((1,)*arr.ndim) - res = np.argmin(arr, axis=None, keepdims=True) - assert_equal(res, res_orig) - assert_(res.ndim == arr.ndim) - assert_(res.shape == (1,)*arr.ndim) - outarray = np.empty(res.shape, dtype=res.dtype) - res1 = np.argmin(arr, axis=None, out=outarray, keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) class TestMinMax: diff --git a/numpy/ma/core.py b/numpy/ma/core.py index 9cae4cf7ac83..a3d85d0a199e 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -5491,7 +5491,7 @@ def argsort(self, axis=np._NoValue, kind=None, order=None, filled = self.filled(fill_value) return filled.argsort(axis=axis, kind=kind, order=order) - def argmin(self, axis=None, fill_value=None, out=None, + def argmin(self, axis=None, fill_value=None, out=None, *, keepdims=np._NoValue): """ Return array of indices to the minimum values along the given axis. @@ -5538,7 +5538,7 @@ def argmin(self, axis=None, fill_value=None, out=None, keepdims = False if keepdims is np._NoValue else bool(keepdims) return d.argmin(axis, out=out, keepdims=keepdims) - def argmax(self, axis=None, fill_value=None, out=None, + def argmax(self, axis=None, fill_value=None, out=None, *, keepdims=np._NoValue): """ Returns array of indices of the maximum values along the given axis. diff --git a/numpy/ma/core.pyi b/numpy/ma/core.pyi index 139b583265c1..e7e3f1f36818 100644 --- a/numpy/ma/core.pyi +++ b/numpy/ma/core.pyi @@ -270,8 +270,8 @@ class MaskedArray(ndarray[_ShapeType, _DType_co]): def std(self, axis=..., dtype=..., out=..., ddof=..., keepdims=...): ... def round(self, decimals=..., out=...): ... def argsort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... - def argmin(self, axis=..., fill_value=..., out=..., keepdims=...): ... - def argmax(self, axis=..., fill_value=..., out=..., keepdims=...): ... + def argmin(self, axis=..., fill_value=..., out=...): ... + def argmax(self, axis=..., fill_value=..., out=...): ... def sort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... def min(self, axis=..., out=..., fill_value=..., keepdims=...): ... # NOTE: deprecated From 06d9610b6c85d758639bdec0d515c8c4f390761d Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Mon, 21 Jun 2021 12:17:01 +0530 Subject: [PATCH 25/37] Removing unwanted changes --- numpy/__init__.pyi | 2 +- numpy/core/fromnumeric.py | 2 +- numpy/core/src/multiarray/calculation.c | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/numpy/__init__.pyi b/numpy/__init__.pyi index 4af5710d4524..ec38d49439a0 100644 --- a/numpy/__init__.pyi +++ b/numpy/__init__.pyi @@ -1323,7 +1323,7 @@ class _ArrayOrScalarCommon: def argmin( self, axis: _ShapeLike = ..., - out: None = ..., + out: None = ..., ) -> Any: ... @overload def argmin( diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index 5d54597c25f8..e790eb0a7d01 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -1300,7 +1300,7 @@ def argmin(a, axis=None, out=None, *, keepdims=np._NoValue): >>> res.shape (2, 1, 4) """ - return _wrapfunc(a, 'argmin', axis, out=out, keepdims=keepdims) + return _wrapfunc(a, 'argmin', axis=axis, out=out, keepdims=keepdims) def _searchsorted_dispatcher(a, v, side=None, sorter=None): diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 80f7fa5b4890..b16ae189a66d 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -216,7 +216,6 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, if ((ap = (PyArrayObject *)PyArray_CheckAxis(op, &axis, 0)) == NULL) { return NULL; } - /* * We need to permute the array so that axis is placed at the end. * And all other dimensions are shifted left. @@ -298,7 +297,7 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, } else { if ((PyArray_NDIM(out) != out_ndim) || - !PyArray_CompareLists(PyArray_DIMS(out), out_shape, + !PyArray_CompareLists(PyArray_DIMS(out), out_shape, out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); From 21127093e9c6922213546d2bdb538fa387525f92 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Mon, 21 Jun 2021 13:07:52 +0530 Subject: [PATCH 26/37] fixed docs --- doc/release/upcoming_changes/19211.new_feature.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/release/upcoming_changes/19211.new_feature.rst b/doc/release/upcoming_changes/19211.new_feature.rst index dc1a3475df2e..fc7c5ca94034 100644 --- a/doc/release/upcoming_changes/19211.new_feature.rst +++ b/doc/release/upcoming_changes/19211.new_feature.rst @@ -1,5 +1,5 @@ ``keepdims`` optional argument added to `numpy.argmin`, `numpy.argmax` --------------------------------------------------------------------- +---------------------------------------------------------------------- ``keepdims`` argument has been added to `numpy.argmin`, `numpy.argmax`. By default, it is `False` and hence no behaviour change should be expected From e0dd74e26878614ccaade05e208eea04deebe08f Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Mon, 21 Jun 2021 13:10:26 +0530 Subject: [PATCH 27/37] Added new lines --- numpy/core/tests/test_multiarray.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 33be36c00bb5..2431e33576d6 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4413,6 +4413,7 @@ def test_object_argmax_with_NULLs(self): a[1] = 30 assert_equal(a.argmax(), 1) + class TestArgmin: nan_arr = [ @@ -4568,6 +4569,7 @@ def test_object_argmin_with_NULLs(self): a[1] = 10 assert_equal(a.argmin(), 1) + class TestMinMax: def test_scalar(self): From 11d7e3386576c97d7a24b00736bd7a65f7456a68 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Tue, 22 Jun 2021 09:01:43 +0530 Subject: [PATCH 28/37] restored annotations --- numpy/__init__.pyi | 14 +++++++++++++- numpy/core/fromnumeric.pyi | 8 ++++++++ numpy/ma/core.pyi | 4 ++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/numpy/__init__.pyi b/numpy/__init__.pyi index ec38d49439a0..22929da539ee 100644 --- a/numpy/__init__.pyi +++ b/numpy/__init__.pyi @@ -1299,18 +1299,24 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., + *, + keepdims: L[False] = ..., ) -> intp: ... @overload def argmax( self, axis: _ShapeLike = ..., out: None = ..., + *, + keepdims: bool = ..., ) -> Any: ... @overload def argmax( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., + *, + keepdims: bool = ..., ) -> _NdArraySubClass: ... @overload @@ -1318,18 +1324,24 @@ class _ArrayOrScalarCommon: self, axis: None = ..., out: None = ..., + *, + keepdims: L[False] = ..., ) -> intp: ... @overload def argmin( self, axis: _ShapeLike = ..., - out: None = ..., + out: None = ..., + *, + keepdims: bool = ..., ) -> Any: ... @overload def argmin( self, axis: Optional[_ShapeLike] = ..., out: _NdArraySubClass = ..., + *, + keepdims: bool = ..., ) -> _NdArraySubClass: ... def argsort( diff --git a/numpy/core/fromnumeric.pyi b/numpy/core/fromnumeric.pyi index 3342ec3ac47b..45057e4b1299 100644 --- a/numpy/core/fromnumeric.pyi +++ b/numpy/core/fromnumeric.pyi @@ -130,12 +130,16 @@ def argmax( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., + *, + keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmax( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., + *, + keepdims: bool = ..., ) -> Any: ... @overload @@ -143,12 +147,16 @@ def argmin( a: ArrayLike, axis: None = ..., out: Optional[ndarray] = ..., + *, + keepdims: Literal[False] = ..., ) -> intp: ... @overload def argmin( a: ArrayLike, axis: Optional[int] = ..., out: Optional[ndarray] = ..., + *, + keepdims: bool = ..., ) -> Any: ... @overload diff --git a/numpy/ma/core.pyi b/numpy/ma/core.pyi index e7e3f1f36818..bc1f45a8d5ad 100644 --- a/numpy/ma/core.pyi +++ b/numpy/ma/core.pyi @@ -270,8 +270,8 @@ class MaskedArray(ndarray[_ShapeType, _DType_co]): def std(self, axis=..., dtype=..., out=..., ddof=..., keepdims=...): ... def round(self, decimals=..., out=...): ... def argsort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... - def argmin(self, axis=..., fill_value=..., out=...): ... - def argmax(self, axis=..., fill_value=..., out=...): ... + def argmin(self, axis=..., fill_value=..., out=..., *, keepdims=...): ... + def argmax(self, axis=..., fill_value=..., out=..., *, keepdims=...): ... def sort(self, axis=..., kind=..., order=..., endwith=..., fill_value=...): ... def min(self, axis=..., out=..., fill_value=..., keepdims=...): ... # NOTE: deprecated From 4c9f113ce55f89d21da5b1f1aaeaca2c18c29b7b Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Thu, 24 Jun 2021 10:16:56 +0530 Subject: [PATCH 29/37] parametrized test --- numpy/core/tests/test_multiarray.py | 145 ++++++++++++++-------------- 1 file changed, 74 insertions(+), 71 deletions(-) diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 2431e33576d6..6adec5f10756 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4194,83 +4194,86 @@ def test_unicode(self): class TestArgmaxArgminCommon: - @pytest.mark.parametrize('size', [(), (3,), (3, 2), (2, 3), - (3, 3), (2, 3, 4), (4, 3, 2), - (1, 2, 3, 4), (2, 3, 4, 1), - (3, 4, 1, 2), (4, 1, 2, 3)]) + sizes = [(), (3,), (3, 2), (2, 3), + (3, 3), (2, 3, 4), (4, 3, 2), + (1, 2, 3, 4), (2, 3, 4, 1), + (3, 4, 1, 2), (4, 1, 2, 3)] + + @pytest.mark.parametrize("size, axis", itertools.chain(*[[(size, axis) + for axis in list(range(-len(size), len(size))) + [None]] + for size in sizes])) @pytest.mark.parametrize('method', [np.argmax, np.argmin]) - def test_np_argmin_argmax_keepdims(self, size, method): + def test_np_argmin_argmax_keepdims(self, size, axis, method): arr = np.random.normal(size=size) - for axis in list(range(-len(size), len(size))) + [None]: - # contiguous arrays - if axis is None: - new_shape = [1 for _ in range(len(size))] + # contiguous arrays + if axis is None: + new_shape = [1 for _ in range(len(size))] + else: + new_shape = list(size) + new_shape[axis] = 1 + new_shape = tuple(new_shape) + + _res_orig = method(arr, axis=axis) + res_orig = _res_orig.reshape(new_shape) + res = method(arr, axis=axis, keepdims=True) + assert_equal(res, res_orig) + assert_(res.shape == new_shape) + outarray = np.empty(res.shape, dtype=res.dtype) + res1 = method(arr, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) + assert_equal(res, outarray) + + if len(size) > 0: + wrong_shape = list(new_shape) + if axis is not None: + wrong_shape[axis] = 2 else: - new_shape = list(size) - new_shape[axis] = 1 - new_shape = tuple(new_shape) - - _res_orig = method(arr, axis=axis) - res_orig = _res_orig.reshape(new_shape) - res = method(arr, axis=axis, keepdims=True) - assert_equal(res, res_orig) - assert_(res.shape == new_shape) - outarray = np.empty(res.shape, dtype=res.dtype) - res1 = method(arr, axis=axis, out=outarray, - keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) - - if len(size) > 0: - wrong_shape = list(new_shape) - if axis is not None: - wrong_shape[axis] = 2 - else: - wrong_shape[0] = 2 - wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) - assert_raises(ValueError, method, - arr.T, axis=axis, - out=wrong_outarray, keepdims=True) - - # non-contiguous arrays - if axis is None: - new_shape = [1 for _ in range(len(size))] + wrong_shape[0] = 2 + wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) + assert_raises(ValueError, method, + arr.T, axis=axis, + out=wrong_outarray, keepdims=True) + + # non-contiguous arrays + if axis is None: + new_shape = [1 for _ in range(len(size))] + else: + new_shape = list(size)[::-1] + new_shape[axis] = 1 + new_shape = tuple(new_shape) + + _res_orig = method(arr.T, axis=axis) + res_orig = _res_orig.reshape(new_shape) + res = method(arr.T, axis=axis, keepdims=True) + assert_equal(res, res_orig) + assert_(res.shape == new_shape) + outarray = np.empty(new_shape[::-1], dtype=res.dtype) + outarray = outarray.T + res1 = method(arr.T, axis=axis, out=outarray, + keepdims=True) + assert_(res1 is outarray) + assert_equal(res, outarray) + + if len(size) > 0: + # one dimension lesser for non-zero sized + # array should raise an error + assert_raises(ValueError, method, + arr[0], axis=axis, + out=outarray, keepdims=True) + + if len(size) > 0: + wrong_shape = list(new_shape) + if axis is not None: + wrong_shape[axis] = 2 else: - new_shape = list(size)[::-1] - new_shape[axis] = 1 - new_shape = tuple(new_shape) - - _res_orig = method(arr.T, axis=axis) - res_orig = _res_orig.reshape(new_shape) - res = method(arr.T, axis=axis, keepdims=True) - assert_equal(res, res_orig) - assert_(res.shape == new_shape) - outarray = np.empty(new_shape[::-1], dtype=res.dtype) - outarray = outarray.T - res1 = method(arr.T, axis=axis, out=outarray, - keepdims=True) - assert_(res1 is outarray) - assert_equal(res, outarray) - - if len(size) > 0: - # one dimension lesser for non-zero sized - # array should raise an error - assert_raises(ValueError, method, - arr[0], axis=axis, - out=outarray, keepdims=True) - - if len(size) > 0: - wrong_shape = list(new_shape) - if axis is not None: - wrong_shape[axis] = 2 - else: - wrong_shape[0] = 2 - wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) - assert_raises(ValueError, method, - arr.T, axis=axis, - out=wrong_outarray, keepdims=True) + wrong_shape[0] = 2 + wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) + assert_raises(ValueError, method, + arr.T, axis=axis, + out=wrong_outarray, keepdims=True) class TestArgmax: From db9c70467886d8dfefa3b438a4db5f6b6122e6b2 Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Wed, 30 Jun 2021 08:34:15 +0530 Subject: [PATCH 30/37] Apply suggestions from code review Co-authored-by: Sebastian Berg --- doc/release/upcoming_changes/19211.new_feature.rst | 9 ++++----- numpy/core/src/multiarray/calculation.c | 9 +++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/release/upcoming_changes/19211.new_feature.rst b/doc/release/upcoming_changes/19211.new_feature.rst index fc7c5ca94034..33a2aaa22400 100644 --- a/doc/release/upcoming_changes/19211.new_feature.rst +++ b/doc/release/upcoming_changes/19211.new_feature.rst @@ -1,9 +1,8 @@ ``keepdims`` optional argument added to `numpy.argmin`, `numpy.argmax` ---------------------------------------------------------------------- -``keepdims`` argument has been added to `numpy.argmin`, `numpy.argmax`. -By default, it is `False` and hence no behaviour change should be expected -in existing codes using `numpy.argmin`, `numpy.argmax`. If set to ``True``, +``keepdims`` argument is added to `numpy.argmin`, `numpy.argmax`. +If set to ``True``, the axes which are reduced are left in the result as dimensions with size one. the axes which are reduced are left in the result as dimensions with size one. -In simple words, the resulting array will be having same dimensions -as the input array. \ No newline at end of file +The resulting array has the same number of dimensions and will broadcast with the +input array. diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index b16ae189a66d..4ae5295f9c0f 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -100,7 +100,8 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, if (!keepdims) { out_ndim = PyArray_NDIM(ap) - 1; out_shape = PyArray_DIMS(ap); - } else { + } + else { out_shape = _shape_buf; if (axis_copy == NPY_MAXDIMS) { for (int i = 0; i < out_ndim; i++) { @@ -109,9 +110,9 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, } else { /* - * While `ap` may be transposed, we can ignore this for `out` because the - * transpose only reorders the size 1 `axis` (not changing memory layout). - */ + * While `ap` may be transposed, we can ignore this for `out` because the + * transpose only reorders the size 1 `axis` (not changing memory layout). + */ memcpy(out_shape, original_op_shape, out_ndim * sizeof(npy_intp)); out_shape[axis] = 1; } From 11cd59782405f09a5f238393ac14fd6b6ff25d79 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Wed, 30 Jun 2021 08:55:23 +0530 Subject: [PATCH 31/37] keyword handling now done in np.argmin/np.argmax --- numpy/core/fromnumeric.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/numpy/core/fromnumeric.py b/numpy/core/fromnumeric.py index e790eb0a7d01..9c6af47673c2 100644 --- a/numpy/core/fromnumeric.py +++ b/numpy/core/fromnumeric.py @@ -49,14 +49,12 @@ def _wrapit(obj, method, *args, **kwds): def _wrapfunc(obj, method, *args, **kwds): - passkwargs = {k: v for k, v in kwds.items() - if v is not np._NoValue} bound = getattr(obj, method, None) if bound is None: - return _wrapit(obj, method, *args, **passkwargs) + return _wrapit(obj, method, *args, **kwds) try: - return bound(*args, **passkwargs) + return bound(*args, **kwds) except TypeError: # A TypeError occurs if the object does have such a method in its # class, but its signature is not identical to that of NumPy's. This @@ -65,7 +63,7 @@ def _wrapfunc(obj, method, *args, **kwds): # # Call _wrapit from within the except clause to ensure a potential # exception has a traceback chain. - return _wrapit(obj, method, *args, **passkwargs) + return _wrapit(obj, method, *args, **kwds) def _wrapreduction(obj, ufunc, method, axis, dtype, out, **kwargs): @@ -1206,8 +1204,8 @@ def argmax(a, axis=None, out=None, *, keepdims=np._NoValue): >>> res.shape (2, 1, 4) """ - return _wrapfunc(a, 'argmax', axis=axis, out=out, - keepdims=keepdims) + kwds = {'keepdims': keepdims} if keepdims is not np._NoValue else {} + return _wrapfunc(a, 'argmax', axis=axis, out=out, **kwds) def _argmin_dispatcher(a, axis=None, out=None, *, keepdims=np._NoValue): @@ -1300,7 +1298,8 @@ def argmin(a, axis=None, out=None, *, keepdims=np._NoValue): >>> res.shape (2, 1, 4) """ - return _wrapfunc(a, 'argmin', axis=axis, out=out, keepdims=keepdims) + kwds = {'keepdims': keepdims} if keepdims is not np._NoValue else {} + return _wrapfunc(a, 'argmin', axis=axis, out=out, **kwds) def _searchsorted_dispatcher(a, v, side=None, sorter=None): From bb906bfb65f2d3d2d7637a93e946c4891e14448f Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Wed, 30 Jun 2021 08:59:38 +0530 Subject: [PATCH 32/37] corrected indendation --- numpy/core/src/multiarray/calculation.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 4ae5295f9c0f..b03ddb3dca97 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -143,8 +143,8 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, } else { if ((PyArray_NDIM(out) != out_ndim) || - !PyArray_CompareLists(PyArray_DIMS(out), out_shape, - out_ndim)) { + !PyArray_CompareLists(PyArray_DIMS(out), out_shape, + out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmax."); goto fail; @@ -299,7 +299,7 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, else { if ((PyArray_NDIM(out) != out_ndim) || !PyArray_CompareLists(PyArray_DIMS(out), out_shape, - out_ndim)) { + out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); goto fail; From 3b72f5947ed74853f929d806a553231a5a96acc5 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Wed, 30 Jun 2021 09:06:20 +0530 Subject: [PATCH 33/37] used with pytest.riases(ValueError) --- numpy/core/tests/test_multiarray.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 6adec5f10756..8438c8bd5ddc 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -4233,8 +4233,8 @@ def test_np_argmin_argmax_keepdims(self, size, axis, method): else: wrong_shape[0] = 2 wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) - assert_raises(ValueError, method, - arr.T, axis=axis, + with pytest.raises(ValueError): + method(arr.T, axis=axis, out=wrong_outarray, keepdims=True) # non-contiguous arrays @@ -4260,9 +4260,9 @@ def test_np_argmin_argmax_keepdims(self, size, axis, method): if len(size) > 0: # one dimension lesser for non-zero sized # array should raise an error - assert_raises(ValueError, method, - arr[0], axis=axis, - out=outarray, keepdims=True) + with pytest.raises(ValueError): + method(arr[0], axis=axis, + out=outarray, keepdims=True) if len(size) > 0: wrong_shape = list(new_shape) @@ -4271,8 +4271,8 @@ def test_np_argmin_argmax_keepdims(self, size, axis, method): else: wrong_shape[0] = 2 wrong_outarray = np.empty(wrong_shape, dtype=res.dtype) - assert_raises(ValueError, method, - arr.T, axis=axis, + with pytest.raises(ValueError): + method(arr.T, axis=axis, out=wrong_outarray, keepdims=True) class TestArgmax: From 66adc84e0add1ca3af5b0b940246ca0c69fecaab Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Wed, 30 Jun 2021 09:27:39 +0530 Subject: [PATCH 34/37] fixed release notes --- doc/release/upcoming_changes/19211.c_api.rst | 10 ++++++++++ doc/release/upcoming_changes/19211.new_feature.rst | 1 - 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 doc/release/upcoming_changes/19211.c_api.rst diff --git a/doc/release/upcoming_changes/19211.c_api.rst b/doc/release/upcoming_changes/19211.c_api.rst new file mode 100644 index 000000000000..eef347633abf --- /dev/null +++ b/doc/release/upcoming_changes/19211.c_api.rst @@ -0,0 +1,10 @@ +Addition of ``PyArray_ArgMinWithKeepdims`` and ``PyArray_ArgMaxWithKeepdims`` +----------------------------------------------------------------------------- + +Two new functions ``PyArray_ArgMinWithKeepdims`` and ``PyArray_ArgMaxWithKeepdims`` +are added. The signatures of these functions are same as ``PyArray_ArgMin`` and +``PyArray_ArgMax`` but with one additional argument, ``int keepdims``. If ``keepdims`` +is equivalent to ``true`` then the axes which are reduced are left in the result as +dimensions with size one. The resulting array has the same number of dimensions and +will broadcast with the input array. Otherwise, the result will be same as the existing, +``PyArray_ArgMin`` and ``PyArray_ArgMax``, both of which have no change in their behaviour. diff --git a/doc/release/upcoming_changes/19211.new_feature.rst b/doc/release/upcoming_changes/19211.new_feature.rst index 33a2aaa22400..40e42387c153 100644 --- a/doc/release/upcoming_changes/19211.new_feature.rst +++ b/doc/release/upcoming_changes/19211.new_feature.rst @@ -3,6 +3,5 @@ ``keepdims`` argument is added to `numpy.argmin`, `numpy.argmax`. If set to ``True``, the axes which are reduced are left in the result as dimensions with size one. -the axes which are reduced are left in the result as dimensions with size one. The resulting array has the same number of dimensions and will broadcast with the input array. From 4be86dd0400d4f52fd72dbc312d69942fd5f7c73 Mon Sep 17 00:00:00 2001 From: czgdp1807 Date: Fri, 2 Jul 2021 09:02:01 +0530 Subject: [PATCH 35/37] removed PyArray_ArgMaxWithKeepdims and PyArray_ArgMinWithKeepdims from public C-API --- doc/release/upcoming_changes/19211.c_api.rst | 10 ---------- doc/source/reference/c-api/array.rst | 14 -------------- numpy/__init__.cython-30.pxd | 2 -- numpy/__init__.pxd | 2 -- numpy/core/code_generators/numpy_api.py | 2 -- numpy/core/src/multiarray/calculation.c | 14 ++++---------- numpy/core/src/multiarray/calculation.h | 4 ++-- numpy/core/src/multiarray/methods.c | 4 ++-- 8 files changed, 8 insertions(+), 44 deletions(-) delete mode 100644 doc/release/upcoming_changes/19211.c_api.rst diff --git a/doc/release/upcoming_changes/19211.c_api.rst b/doc/release/upcoming_changes/19211.c_api.rst deleted file mode 100644 index eef347633abf..000000000000 --- a/doc/release/upcoming_changes/19211.c_api.rst +++ /dev/null @@ -1,10 +0,0 @@ -Addition of ``PyArray_ArgMinWithKeepdims`` and ``PyArray_ArgMaxWithKeepdims`` ------------------------------------------------------------------------------ - -Two new functions ``PyArray_ArgMinWithKeepdims`` and ``PyArray_ArgMaxWithKeepdims`` -are added. The signatures of these functions are same as ``PyArray_ArgMin`` and -``PyArray_ArgMax`` but with one additional argument, ``int keepdims``. If ``keepdims`` -is equivalent to ``true`` then the axes which are reduced are left in the result as -dimensions with size one. The resulting array has the same number of dimensions and -will broadcast with the input array. Otherwise, the result will be same as the existing, -``PyArray_ArgMin`` and ``PyArray_ArgMax``, both of which have no change in their behaviour. diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index a86ced91234d..cb2f4b645af2 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -2019,26 +2019,12 @@ Calculation Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of the largest element of *self* along *axis*. -.. c:function:: PyObject* PyArray_ArgMaxWithKeepdims( \ - PyArrayObject* self, int axis, PyArrayObject* out, int keepdims) - - Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of - the largest element of *self* along *axis*. The number of dimensions of the result matches with the - that of the input object if keepdims evaluates to true. - .. c:function:: PyObject* PyArray_ArgMin( \ PyArrayObject* self, int axis, PyArrayObject* out) Equivalent to :meth:`ndarray.argmin` (*self*, *axis*). Return the index of the smallest element of *self* along *axis*. -.. c:function:: PyObject* PyArray_ArgMinWithKeepdims( \ - PyArrayObject* self, int axis, PyArrayObject* out, int keepdims) - - Equivalent to :meth:`ndarray.argmax` (*self*, *axis*). Return the index of - the smallest element of *self* along *axis*. The number of dimensions of the result matches with the - that of the input object if keepdims evaluates to true. - .. c:function:: PyObject* PyArray_Max( \ PyArrayObject* self, int axis, PyArrayObject* out) diff --git a/numpy/__init__.cython-30.pxd b/numpy/__init__.cython-30.pxd index 3803ae87fb0d..42a46d0b832b 100644 --- a/numpy/__init__.cython-30.pxd +++ b/numpy/__init__.cython-30.pxd @@ -644,9 +644,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) object PyArray_ArgMax (ndarray, int, ndarray) - object PyArray_ArgMaxWithKeepdims (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) - object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/__init__.pxd b/numpy/__init__.pxd index 75b8b151b190..97f3da2e5673 100644 --- a/numpy/__init__.pxd +++ b/numpy/__init__.pxd @@ -602,9 +602,7 @@ cdef extern from "numpy/arrayobject.h": object PyArray_ArgSort (ndarray, int, NPY_SORTKIND) object PyArray_SearchSorted (ndarray, object, NPY_SEARCHSIDE, PyObject *) object PyArray_ArgMax (ndarray, int, ndarray) - object PyArray_ArgMaxWithKeepdims (ndarray, int, ndarray, int) object PyArray_ArgMin (ndarray, int, ndarray) - object PyArray_ArgMinWithKeepdims (ndarray, int, ndarray, int) object PyArray_Reshape (ndarray, object) object PyArray_Newshape (ndarray, PyArray_Dims *, NPY_ORDER) object PyArray_Squeeze (ndarray) diff --git a/numpy/core/code_generators/numpy_api.py b/numpy/core/code_generators/numpy_api.py index f5c54f01db14..fbd3233680fa 100644 --- a/numpy/core/code_generators/numpy_api.py +++ b/numpy/core/code_generators/numpy_api.py @@ -350,8 +350,6 @@ 'PyArray_ResolveWritebackIfCopy': (302,), 'PyArray_SetWritebackIfCopyBase': (303,), # End 1.14 API - 'PyArray_ArgMinWithKeepdims': (304,), - 'PyArray_ArgMaxWithKeepdims': (305,), } ufunc_types_api = { diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index b03ddb3dca97..9559afd3f5c8 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -34,11 +34,8 @@ power_of_ten(int n) return ret; } -/*NUMPY_API - * ArgMax - */ NPY_NO_EXPORT PyObject * -PyArray_ArgMaxWithKeepdims(PyArrayObject *op, +_PyArray_ArgMaxWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; @@ -188,14 +185,11 @@ PyArray_ArgMaxWithKeepdims(PyArrayObject *op, NPY_NO_EXPORT PyObject * PyArray_ArgMax(PyArrayObject *op, int axis, PyArrayObject *out) { - return PyArray_ArgMaxWithKeepdims(op, axis, out, 0); + return _PyArray_ArgMaxWithKeepdims(op, axis, out, 0); } -/*NUMPY_API - * ArgMin - */ NPY_NO_EXPORT PyObject * -PyArray_ArgMinWithKeepdims(PyArrayObject *op, +_PyArray_ArgMinWithKeepdims(PyArrayObject *op, int axis, PyArrayObject *out, int keepdims) { PyArrayObject *ap = NULL, *rp = NULL; @@ -343,7 +337,7 @@ PyArray_ArgMinWithKeepdims(PyArrayObject *op, NPY_NO_EXPORT PyObject * PyArray_ArgMin(PyArrayObject *op, int axis, PyArrayObject *out) { - return PyArray_ArgMinWithKeepdims(op, axis, out, 0); + return _PyArray_ArgMinWithKeepdims(op, axis, out, 0); } /*NUMPY_API diff --git a/numpy/core/src/multiarray/calculation.h b/numpy/core/src/multiarray/calculation.h index c3498e878aae..49105a1385c0 100644 --- a/numpy/core/src/multiarray/calculation.h +++ b/numpy/core/src/multiarray/calculation.h @@ -5,13 +5,13 @@ NPY_NO_EXPORT PyObject* PyArray_ArgMax(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* -PyArray_ArgMaxWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); +_PyArray_ArgMaxWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); NPY_NO_EXPORT PyObject* PyArray_ArgMin(PyArrayObject* self, int axis, PyArrayObject *out); NPY_NO_EXPORT PyObject* -PyArray_ArgMinWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); +_PyArray_ArgMinWithKeepdims(PyArrayObject* self, int axis, PyArrayObject *out, int keepdims); NPY_NO_EXPORT PyObject* PyArray_Max(PyArrayObject* self, int axis, PyArrayObject* out); diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index ad7464785f0c..dc23b3471cb2 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -295,7 +295,7 @@ array_argmax(PyArrayObject *self, return NULL; } - PyObject *ret = PyArray_ArgMaxWithKeepdims(self, axis, out, keepdims); + PyObject *ret = _PyArray_ArgMaxWithKeepdims(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { @@ -322,7 +322,7 @@ array_argmin(PyArrayObject *self, return NULL; } - PyObject *ret = PyArray_ArgMinWithKeepdims(self, axis, out, keepdims); + PyObject *ret = _PyArray_ArgMinWithKeepdims(self, axis, out, keepdims); /* this matches the unpacking behavior of ufuncs */ if (out == NULL) { From 8f9f108cc66d1ca9768d345d8547ca0de242c151 Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Fri, 2 Jul 2021 16:20:21 +0530 Subject: [PATCH 36/37] Apply suggestions from code review Co-authored-by: Eric Wieser --- numpy/core/src/multiarray/calculation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index 9559afd3f5c8..bebfeaf89378 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -254,7 +254,7 @@ _PyArray_ArgMinWithKeepdims(PyArrayObject *op, } else { out_shape = _shape_buf; if (axis_copy == NPY_MAXDIMS) { - for( int i = 0; i < out_ndim; i++ ) { + for (int i = 0; i < out_ndim; i++) { out_shape[i] = 1; } } else { From c55aaeabcbad491d6280fd890fae2efb0ef76714 Mon Sep 17 00:00:00 2001 From: Gagandeep Singh Date: Fri, 2 Jul 2021 16:37:57 +0530 Subject: [PATCH 37/37] Apply suggestions from code review Co-authored-by: Eric Wieser --- numpy/core/src/multiarray/calculation.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c index bebfeaf89378..e89018889d10 100644 --- a/numpy/core/src/multiarray/calculation.c +++ b/numpy/core/src/multiarray/calculation.c @@ -292,8 +292,7 @@ _PyArray_ArgMinWithKeepdims(PyArrayObject *op, } else { if ((PyArray_NDIM(out) != out_ndim) || - !PyArray_CompareLists(PyArray_DIMS(out), out_shape, - out_ndim)) { + !PyArray_CompareLists(PyArray_DIMS(out), out_shape, out_ndim)) { PyErr_SetString(PyExc_ValueError, "output array does not match result of np.argmin."); goto fail;