[GitHub] [incubator-mxnet] haojin2 commented on a change in pull request #15699: Numpy take operator implementation & bug fix in ndarray.take
haojin2 commented on a change in pull request #15699: Numpy take operator
implementation & bug fix in ndarray.take
URL: https://github.com/apache/incubator-mxnet/pull/15699#discussion_r31081
##
File path: src/operator/numpy/indexing_op.cc
##
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2019 by Contributors
+ * \file indexing_op.cc
+ * \brief CPU implementation of numpy indexing operator
+*/
+
+#include "./indexing_op.h"
+
+namespace mxnet {
+namespace op {
+
+template<>
+void NumpyTakeOpForward(const nnvm::NodeAttrs& attrs,
+const OpContext& ctx,
Review comment:
Align the arguments:
```c++
template<>
void NumpyTakeOpForward(const nnvm::NodeAttrs& attrs,
const OpContext& ctx,
const std::vector& inputs,
const std::vector& req,
const std::vector& outputs) {
```
Same for all functions.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] haojin2 commented on a change in pull request #15699: Numpy take operator implementation & bug fix in ndarray.take
haojin2 commented on a change in pull request #15699: Numpy take operator
implementation & bug fix in ndarray.take
URL: https://github.com/apache/incubator-mxnet/pull/15699#discussion_r310810460
##
File path: python/mxnet/numpy/multiarray.py
##
@@ -1879,6 +1879,85 @@ def concatenate(seq, axis=0, out=None):
return _mx_nd_np.concatenate(seq, axis=axis, out=out)
+@set_module('mxnet.numpy')
+def take(a, indices, axis=None, mode='clip', out=None):
+r"""
+Take elements from an array along an axis.
+
+When axis is not None, this function does the same thing as "fancy"
+indexing (indexing arrays using arrays); however, it can be easier to use
+if you need elements along a given axis. A call such as
+``np.take(arr, indices, axis=3)`` is equivalent to
+``arr[:,:,:,indices,...]``.
+
+Explained without fancy indexing, this is equivalent to the following use
+of `ndindex`, which sets each of ``ii``, ``jj``, and ``kk`` to a tuple of
+indices::
+
+Ni, Nk = a.shape[:axis], a.shape[axis+1:]
+Nj = indices.shape
+for ii in ndindex(Ni):
+for jj in ndindex(Nj):
+for kk in ndindex(Nk):
+out[ii + jj + kk] = a[ii + (indices[jj],) + kk]
+
+Parameters
+--
+a : ndarray
+The source array.
+indices : ndarray
+The indices of the values to extract. Also allow scalars for indices.
+axis : int, optional
+The axis over which to select values. By default, the flattened
+input array is used.
+out : ndarray, optional
+If provided, the result will be placed in this array. It should
+be of the appropriate shape and dtype.
+mode : {'clip', 'wrap'}, optional
+Specifies how out-of-bounds indices will behave.
+
+* 'clip' -- clip to the range (default)
+* 'wrap' -- wrap around
+
+'clip' mode means that all indices that are too large are replaced
+by the index that addresses the last element along that axis. Note
+that this disables indexing with negative numbers.
+
+Returns
+---
+out : ndarray
+The returned array has the same type as `a`.
+
+Notes
+-
+
+This function differs from the original `numpy.take
+`_ in
+the following way(s):
+
+- Only ndarray or scalar ndarray is accepted as valid input.
+- 'raise' mode is not supported.
+
+Examples
+
+>>> a = np.array([4, 3, 5, 7, 6, 8])
+>>> indices = np.array([0, 1, 4])
+>>> np.take(a, indices)
+array([4., 3., 6.])
+
+In this example for `a` is an ndarray, "fancy" indexing can be used.
+
+>>> a[indices]
+array([4., 3., 6.])
+
+If `indices` is not one dimensional, the output also has these dimensions.
+
+>>> np.take(a, np.array([[0, 1], [2, 3]]))
+array([[4., 3.],
+ [5., 7.]])
+"""
+return _mx_nd_np.take(a, indices, axis, mode, out)
+
Review comment:
Same here and for each and every new Python functions.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] haojin2 commented on a change in pull request #15699: Numpy take operator implementation & bug fix in ndarray.take
haojin2 commented on a change in pull request #15699: Numpy take operator
implementation & bug fix in ndarray.take
URL: https://github.com/apache/incubator-mxnet/pull/15699#discussion_r310810220
##
File path: python/mxnet/ndarray/numpy/_op.py
##
@@ -590,6 +590,88 @@ def concatenate(seq, axis=0, out=None):
return _npi.concatenate(*seq, dim=axis, out=out)
+@set_module('mxnet.ndarray.numpy')
+def take(a, indices, axis=None, mode='clip', out=None):
+r"""
+Take elements from an array along an axis.
+
+When axis is not None, this function does the same thing as "fancy"
+indexing (indexing arrays using arrays); however, it can be easier to use
+if you need elements along a given axis. A call such as
+``np.take(arr, indices, axis=3)`` is equivalent to
+``arr[:,:,:,indices,...]``.
+
+Explained without fancy indexing, this is equivalent to the following use
+of `ndindex`, which sets each of ``ii``, ``jj``, and ``kk`` to a tuple of
+indices::
+
+Ni, Nk = a.shape[:axis], a.shape[axis+1:]
+Nj = indices.shape
+for ii in ndindex(Ni):
+for jj in ndindex(Nj):
+for kk in ndindex(Nk):
+out[ii + jj + kk] = a[ii + (indices[jj],) + kk]
+
+Parameters
+--
+a : ndarray
+The source array.
+indices : ndarray
+The indices of the values to extract. Also allow scalars for indices.
+axis : int, optional
+The axis over which to select values. By default, the flattened
+input array is used.
+out : ndarray, optional
+If provided, the result will be placed in this array. It should
+be of the appropriate shape and dtype.
+mode : {'clip', 'wrap'}, optional
+Specifies how out-of-bounds indices will behave.
+
+* 'clip' -- clip to the range (default)
+* 'wrap' -- wrap around
+
+'clip' mode means that all indices that are too large are replaced
+by the index that addresses the last element along that axis. Note
+that this disables indexing with negative numbers.
+
+Returns
+---
+out : ndarray
+The returned array has the same type as `a`.
+
+Notes
+-
+
+This function differs from the original `numpy.take
+`_ in
+the following way(s):
+
+- Only ndarray or scalar ndarray is accepted as valid input.
+- 'raise' mode is not supported.
+
+Examples
+
+>>> a = np.array([4, 3, 5, 7, 6, 8])
+>>> indices = np.array([0, 1, 4])
+>>> np.take(a, indices)
+array([4., 3., 6.])
+
+In this example for `a` is an ndarray, "fancy" indexing can be used.
+
+>>> a[indices]
+array([4., 3., 6.])
+
+If `indices` is not one dimensional, the output also has these dimensions.
+
+>>> np.take(a, np.array([[0, 1], [2, 3]]))
+array([[4., 3.],
+ [5., 7.]])
+"""
+if mode not in ('wrap', 'clip'):
+raise NotImplementedError(
+"function take does not support mode '{}'".format(mode))
+return _npi.take(a, indices, axis, mode, out)
+
Review comment:
one more extra blank line below.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] haojin2 commented on a change in pull request #15699: Numpy take operator implementation & bug fix in ndarray.take
haojin2 commented on a change in pull request #15699: Numpy take operator
implementation & bug fix in ndarray.take
URL: https://github.com/apache/incubator-mxnet/pull/15699#discussion_r310809625
##
File path: src/operator/tensor/indexing_op.h
##
@@ -341,6 +341,53 @@ struct Take {
}
};
+template
Review comment:
Are you looking to share the kernels with the numpy version?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] haojin2 commented on a change in pull request #15699: Numpy take operator implementation & bug fix in ndarray.take
haojin2 commented on a change in pull request #15699: Numpy take operator
implementation & bug fix in ndarray.take
URL: https://github.com/apache/incubator-mxnet/pull/15699#discussion_r310808895
##
File path: src/operator/tensor/indexing_op.h
##
@@ -341,6 +341,53 @@ struct Take {
}
};
+template
Review comment:
Why are you moving device-specific kernel to `.h` ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
