You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/08/26 01:29:22 UTC

[GitHub] [incubator-mxnet] Zha0q1 opened a new pull request #19015: Numpy nonzero large tensor fix

Zha0q1 opened a new pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015


   This fixes nonzero against large tensors.
   Also cleaned up the loop a little bit.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-683186130


   Undefined action detected. 
   Permissible actions are : run ci [all], run ci [job1, job2] 
   Example : @mxnet-bot run ci [all] 
   Example : @mxnet-bot run ci [centos-cpu, clang]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478563458



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       This int64_t is actually the output data type. According to the original author this "must be int 64" https://github.com/apache/incubator-mxnet/blob/a2b77fb311204488bd72f99b681fc0ff334bca8d/src/operator/numpy/np_nonzero_op.cc#L33. 
   I think the reasoning is because the tensor is flattened, even with int64tensorsize=off we can have > int32_max many elements e.g.(2**20, 2**20)




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-683186026


   Undefined action detected. 
   Permissible actions are : run ci [all], run ci [job1, job2] 
   Example : @mxnet-bot run ci [all] 
   Example : @mxnet-bot run ci [centos-cpu, clang]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478477723



##########
File path: src/operator/numpy/np_nonzero_op.cc
##########
@@ -88,17 +88,16 @@ void NonzeroForwardCPU(const nnvm::NodeAttrs& attrs,
     const_cast<NDArray &>(out).Init(s);
     return;
   }
-  std::vector<int32_t> prefix_sum(in_size, 0);
+  std::vector<index_t> prefix_sum(in_size, 0);
   size_t valid_num = 0;
   // Calculate prefix sum
   MSHADOW_TYPE_SWITCH_WITH_BOOL(in.dtype(), DType, {
     DType* in_dptr = in.data().dptr<DType>();
     for (size_t i = 0; i < in_size; i++) {
-      prefix_sum[i] = (i == 0) ? 0 : prefix_sum[i - 1];
-      prefix_sum[i] += (in_dptr[i]) ? 1 : 0;
+      valid_num += (in_dptr[i] != 0);

Review comment:
       Nice work ! its actually correct : https://stackoverflow.com/questions/2725044/can-i-assume-booltrue-int1-for-any-c-compiler
   
   But for the sake of readability can you still keep it 
   ```
   (in_dptr[i]) ? 1 : 0;
   ```




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-682205111


   > Again pytest run results ?
   
   ?? rest LGTM!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-683186017


   @mxnet-bot add [pr-awaiting-merge]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478563458



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       This int64_t is actually the output data type. According to the original author this "must be int 64" https://github.com/apache/incubator-mxnet/blob/a2b77fb311204488bd72f99b681fc0ff334bca8d/src/operator/numpy/np_nonzero_op.cc#L33. 
   although I do agree that both these places can be changed to index_t




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-680406692


   Hey @Zha0q1 , Thanks for submitting the PR 
   All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands: 
   - To trigger all jobs: @mxnet-bot run ci [all] 
   - To trigger specific jobs: @mxnet-bot run ci [job1, job2] 
   *** 
   **CI supported jobs**: [centos-gpu, unix-gpu, clang, sanity, windows-cpu, unix-cpu, centos-cpu, windows-gpu, edge, miscellaneous, website]
   *** 
   _Note_: 
    Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin. 
   All CI tests must pass before the PR can be merged. 
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478710967



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,
+                                  const index_t* idx,
+                                  const mshadow::Shape<ndim> shape) {
+    index_t prev = (i == 0) ? 0 : idx[i - 1];
+    index_t curr = idx[i];
+    if (prev != curr) {
+      mshadow::Shape<ndim> coord = mxnet_op::unravel<ndim>(i, shape);
+      for (int j = 0; j < ndim; j++) {
+        out[prev * ndim + j] = coord[j];
+      }
+    }
+  }
+};
+
+struct NonzeroForwardKernelGPU {
+  // for gpu implementation because it does not support int 64 indexing

Review comment:
       aah ... makes sense two Fcompute each for CPU and GPU comes to same kernel eventually but for GPU's FCompute there is a prior call to cub that expects an int32_t pointer that it populates and returns. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 edited a comment on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 edited a comment on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-683186017


   @mxnet-bot add [awaiting-merge]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478563458



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       This int64_t is actually the output data type. According to the original author this "must be int 64" https://github.com/apache/incubator-mxnet/blob/a2b77fb311204488bd72f99b681fc0ff334bca8d/src/operator/numpy/np_nonzero_op.cc#L33. 
   I do also agree that both these places can be changed to index_t




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478467906



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       int64_t -> index_t 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-681998737


   Again pytest run results ?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478598552



##########
File path: src/operator/numpy/np_nonzero_op.cc
##########
@@ -88,17 +88,16 @@ void NonzeroForwardCPU(const nnvm::NodeAttrs& attrs,
     const_cast<NDArray &>(out).Init(s);
     return;
   }
-  std::vector<int32_t> prefix_sum(in_size, 0);
+  std::vector<index_t> prefix_sum(in_size, 0);
   size_t valid_num = 0;
   // Calculate prefix sum
   MSHADOW_TYPE_SWITCH_WITH_BOOL(in.dtype(), DType, {
     DType* in_dptr = in.data().dptr<DType>();
     for (size_t i = 0; i < in_size; i++) {
-      prefix_sum[i] = (i == 0) ? 0 : prefix_sum[i - 1];
-      prefix_sum[i] += (in_dptr[i]) ? 1 : 0;
+      valid_num += (in_dptr[i] != 0);

Review comment:
       I will add a comment there explaining the logic. This way of writing it is faster than branching




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-680680675


   @mxnet-bot run ci [unix-gpu ]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-682206330


   > > Again pytest run results ?
   > 
   > ?? rest LGTM!
   
   Sorry I was working on another task. Will make the changes and paste run results here before requesting for merge. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478478710



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,
+                                  const index_t* idx,
+                                  const mshadow::Shape<ndim> shape) {
+    index_t prev = (i == 0) ? 0 : idx[i - 1];
+    index_t curr = idx[i];
+    if (prev != curr) {
+      mshadow::Shape<ndim> coord = mxnet_op::unravel<ndim>(i, shape);
+      for (int j = 0; j < ndim; j++) {
+        out[prev * ndim + j] = coord[j];
+      }
+    }
+  }
+};
+
+struct NonzeroForwardKernelGPU {
+  // for gpu implementation because it does not support int 64 indexing

Review comment:
       Can you point me to where the cub library is getting called ?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-680680726


   Jenkins CI successfully triggered : [unix-gpu]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-682268611


   test result:
   ```
   ubuntu@ip-172-31-38-169:~/incubator-mxnet$ pytest tests/nightly/test_np_large_array.py::test_nonzero
   /home/ubuntu/anaconda3/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
     return f(*args, **kwds)
   /home/ubuntu/anaconda3/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
     return f(*args, **kwds)
   /home/ubuntu/anaconda3/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
     return f(*args, **kwds)
   ============================================ test session starts =============================================
   platform linux -- Python 3.7.7, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
   rootdir: /home/ubuntu/incubator-mxnet, inifile: pytest.ini
   plugins: remotedata-0.3.2, openfiles-0.4.0, astropy-header-0.1.2, hypothesis-5.8.3, arraydiff-0.3, doctestplus-0.5.0
   collected 1 item                                                                                             
   
   tests/nightly/test_np_large_array.py         .                                                                 [100%]
   
   ============================================== warnings summary ==============================================
   tests/nightly/test_np_large_array.py:88
     /home/ubuntu/incubator-mxnet/tests/nightly/test_np_large_array.py:88: DeprecationWarning: invalid escape sequence \ 
       '''
   
   tests/nightly/test_np_large_array.py:568
     /home/ubuntu/incubator-mxnet/tests/nightly/test_np_large_array.py:568: DeprecationWarning: invalid escape sequence \ 
       '''
   
   -- Docs: https://docs.pytest.org/en/latest/warnings.html
   ================================= 1 passed, 2 warnings in 166.16s (0:02:46) ==================================
   ```


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478556931



##########
File path: src/operator/numpy/np_nonzero_op.cc
##########
@@ -88,17 +88,16 @@ void NonzeroForwardCPU(const nnvm::NodeAttrs& attrs,
     const_cast<NDArray &>(out).Init(s);
     return;
   }
-  std::vector<int32_t> prefix_sum(in_size, 0);
+  std::vector<index_t> prefix_sum(in_size, 0);
   size_t valid_num = 0;
   // Calculate prefix sum
   MSHADOW_TYPE_SWITCH_WITH_BOOL(in.dtype(), DType, {
     DType* in_dptr = in.data().dptr<DType>();
     for (size_t i = 0; i < in_size; i++) {
-      prefix_sum[i] = (i == 0) ? 0 : prefix_sum[i - 1];
-      prefix_sum[i] += (in_dptr[i]) ? 1 : 0;
+      valid_num += (in_dptr[i] != 0);

Review comment:
       yes sir




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478563458



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       This int64_t is actually the output data type. According to the original author this "must be int 64" https://github.com/apache/incubator-mxnet/blob/a2b77fb311204488bd72f99b681fc0ff334bca8d/src/operator/numpy/np_nonzero_op.cc#L33. 
   




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478563458



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       This int64_t is actually the output data type. According to the original author this "must be int 64" https://github.com/apache/incubator-mxnet/blob/a2b77fb311204488bd72f99b681fc0ff334bca8d/src/operator/numpy/np_nonzero_op.cc#L33. 
   I do also think this both these places can be changed to index_t




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] sxjscience merged pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
sxjscience merged pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r478557832



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,
+                                  const index_t* idx,
+                                  const mshadow::Shape<ndim> shape) {
+    index_t prev = (i == 0) ? 0 : idx[i - 1];
+    index_t curr = idx[i];
+    if (prev != curr) {
+      mshadow::Shape<ndim> coord = mxnet_op::unravel<ndim>(i, shape);
+      for (int j = 0; j < ndim; j++) {
+        out[prev * ndim + j] = coord[j];
+      }
+    }
+  }
+};
+
+struct NonzeroForwardKernelGPU {
+  // for gpu implementation because it does not support int 64 indexing

Review comment:
       In our code the cub function is called here: https://github.com/apache/incubator-mxnet/blob/master/src/operator/numpy/np_nonzero_op.cu#L71
   You can also refer to https://github.com/apache/incubator-mxnet/issues/19020




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] Zha0q1 removed a comment on pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
Zha0q1 removed a comment on pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#issuecomment-683186017


   @mxnet-bot add [awaiting-merge]


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-mxnet] sxjscience commented on a change in pull request #19015: Numpy nonzero large tensor fix

Posted by GitBox <gi...@apache.org>.
sxjscience commented on a change in pull request #19015:
URL: https://github.com/apache/incubator-mxnet/pull/19015#discussion_r480277134



##########
File path: src/operator/numpy/np_nonzero_op-inl.h
##########
@@ -43,6 +43,25 @@ namespace mxnet {
 namespace op {
 
 struct NonzeroForwardKernel {
+  // this is for cpu
+  template<int ndim>
+  MSHADOW_XINLINE static void Map(index_t i,
+                                  int64_t* out,

Review comment:
       For me, I feel that it's better to use template because most of the codes are duplicated.




----------------------------------------------------------------
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:
users@infra.apache.org