[GitHub] szha commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

szha commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187525417
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
 
 Review comment:
   this is essentially about whether we allow the different copies of 
parameters to be out of sync.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] szha opened a new issue #10898: usage of thread_local causes compilation to fail on osx 10.10

[GitBox]

szha opened a new issue #10898: usage of thread_local causes compilation to 
fail on osx 10.10
URL: https://github.com/apache/incubator-mxnet/issues/10898
 
 
   ## Description
   (Brief description of the problem in no more than 2 sentences.)
   
   ## Environment info (Required)
   OSX 10.10
   
   ## Build info (Required if built from source)
   
   Compiler (gcc/clang/mingw/visual studio): clang
   
   MXNet commit hash: HEAD as of yesterday
   
   ## Error Message:
   ```bash
   ...
   g++ -std=c++11 -c -DMSHADOW_FORCE_STREAM -Wall -Wsign-compare -O3 -DNDEBUG=1 
-I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/mshadow/ 
-I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/dmlc-core/include 
-fPIC -I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/nnvm/include 
-I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/dlpack/include 
-I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/nnvm/tvm/include 
-Iinclude -funroll-loops -Wno-unused-parameter -Wno-unknown-pragmas 
-Wno-unused-local-typedefs -msse3 -mf16c -DMSHADOW_USE_CUDA=0 
-DMSHADOW_USE_CBLAS=1 -DMSHADOW_USE_MKL=0 
-I/System/Library/Frameworks/Accelerate.framework/Versions/Current/Frameworks/vecLib.framework/Versions/Current/Headers/
 -DMSHADOW_RABIT_PS=0 -DMSHADOW_DIST_PS=0 -DMSHADOW_USE_PASCAL=0 
-DMXNET_USE_SIGNAL_HANDLER=1 -DMXNET_USE_OPENCV=1 
-I/Users/travis/build/dmlc/mxnet-distro/deps/include/opencv -DMXNET_USE_LAPACK 
-I/Users/travis/build/dmlc/mxnet-distro/deps/include -ffunction-sections 
-fdata-sections -DMXNET_USE_DIST_KVSTORE 
-I/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/ps-lite/include 
-I/Users/travis/build/dmlc/mxnet-distro/deps/include -DMXNET_USE_NCCL=0 
-DMXNET_USE_LIBJPEG_TURBO=0 -MMD -c src/operator/nn/dropout.cc -o 
build/src/operator/nn/dropout.o
   
   warning: unknown warning option '-Wno-unused-local-typedefs' 
[-Wunknown-warning-option]
   
   In file included from src/operator/nn/dropout.cc:27:
   
   In file included from src/operator/nn/./dropout-inl.h:31:
   
   In file included from include/mxnet/operator.h:33:
   
   In file included from 
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/nnvm/include/nnvm/node.h:13:
   
   In file included from 
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/nnvm/include/nnvm/./base.h:12:
   
   In file included from 
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/dmlc-core/include/dmlc/memory.h:12:
   
   
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/dmlc-core/include/dmlc/./thread_local.h:26:9:
 warning: Warning: CXX11 thread_local is not formally 
supported [-W#pragma-messages]
   
   #pragma message("Warning: CXX11 thread_local is not formally supported")
   
   ^
   
   In file included from src/operator/nn/dropout.cc:27:
   
   src/operator/nn/./dropout-inl.h:378:12: error: 
thread-local storage is not supported for the current target
   
   static thread_local DropoutOp op;
   
      ^
   
   
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/mshadow/mshadow/base.h:922:8:
 note: expanded from macro 'MSHADOW_REAL_TYPE_SWITCH'
   
 {__VA_ARGS__} \
   
      ^
   
   In file included from src/operator/nn/dropout.cc:27:
   
   src/operator/nn/./dropout-inl.h:378:12: error: 
thread-local storage is not supported for the current target
   
   
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/mshadow/mshadow/base.h:928:8:
 note: expanded from macro 'MSHADOW_REAL_TYPE_SWITCH'
   
 {__VA_ARGS__} \
   
      ^
   
   In file included from src/operator/nn/dropout.cc:27:
   
   src/operator/nn/./dropout-inl.h:378:12: error: 
thread-local storage is not supported for the current target
   
   
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/mshadow/mshadow/base.h:934:8:
 note: expanded from macro 'MSHADOW_REAL_TYPE_SWITCH'
   
 {__VA_ARGS__} \
   
      ^
   
   In file included from src/operator/nn/dropout.cc:27:
   
   src/operator/nn/./dropout-inl.h:400:12: error: 
thread-local storage is not supported for the current target
   
   static thread_local DropoutOp op;
   
      ^
   
   
/Users/travis/build/dmlc/mxnet-distro/mxnet-build/3rdparty/mshadow/mshadow/base.h:922:8:
 note: expanded from macro 'MSHADOW_REAL_TYPE_SWITCH'
   
 {__VA_ARGS__} \
   
      ^
   
   In file included from src/operator/nn/dropout.cc:27:
   
   src/operator/nn/./dropout-inl.h:400:12: 

[GitHub] reminisce commented on a change in pull request #10882: move exec.reshape to backend

[GitBox]

reminisce commented on a change in pull request #10882: move exec.reshape to 
backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#discussion_r187518208
 
 

 ##
 File path: python/mxnet/executor.py
 ##
 @@ -399,62 +399,96 @@ def reshape(self, partial_shaping=False, 
allow_up_sizing=False, **kwargs):
 >>> texec.reshape(allow_up_sizing=True, **new_shape)
 """
 # pylint: disable=too-many-branches
-arg_shapes, _, aux_shapes = self._symbol.infer_shape(**kwargs)
-if arg_shapes is None:
-raise ValueError("Insufficient argument shapes provided.")
-
-new_arg_dict = {}
-new_grad_dict = {}
-for i, name in enumerate(self._symbol.list_arguments()):
-new_shape = arg_shapes[i]
-arr = self.arg_arrays[i]
-darr = None if self.grad_arrays is None else self.grad_arrays[i]
-if partial_shaping or name in kwargs or new_shape == arr.shape:
-if np.prod(new_shape) > np.prod(arr.shape):
-assert allow_up_sizing, "New shape of arg:%s larger than 
original. "%name + \
-"First making a big executor and then down sizing it " 
+ \
-"is more efficient than the reverse." + \
-"If you really want to up size, set 
allow_up_sizing=True " + \
-"to enable allocation of new arrays."
-new_arg_dict[name] = nd.empty(new_shape, ctx=arr.context, 
dtype=arr.dtype)
-if darr is not None:
-new_grad_dict[name] = nd.empty(new_shape, 
ctx=darr.context, dtype=arr.dtype)
+listed_arguments = self._symbol.list_arguments()
+
+provided_arg_shape_data = []  # shape data
+# argument shape index in sdata,
+# e.g. [sdata[indptr[0]], sdata[indptr[1]]) is the shape of the first 
arg
+provided_arg_shape_idx = [0]
+provided_arg_shape_names = []  # provided argument names
+for k, v in kwargs.items():
+# if k not in listed_arguments and k not in listed_aux_states:
+#   raise ValueError('arg name %s is not valid', k)
+if isinstance(v, tuple):
+provided_arg_shape_names.append(k)
+provided_arg_shape_data.extend(v)
+provided_arg_shape_idx.append(len(provided_arg_shape_data))
+
+args_handle, args = self._symbol._get_ndarray_inputs(
 
 Review comment:
   I think you can get all the arg/aux/grad arrays from the executor in 
backend? Is it necessary to prepare them here and pass to the backend again?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] chtlp opened a new issue #10897: performance and error issue with INT8 quantization example

[GitBox]

chtlp opened a new issue #10897: performance and error issue with INT8 
quantization example
URL: https://github.com/apache/incubator-mxnet/issues/10897
 
 
   Hi, I am trying the the new quantization example in the master branch of 
MxNet, however the original code seems to have much slower inference speed, 
while the revised code throws an error with cuDNN.
   https://github.com/apache/incubator-mxnet/blob/master/example/quantization/
   
   ## Description
   
   The example works fine when saving when saving the matrices in the `cpu()` 
context:
   
https://github.com/apache/incubator-mxnet/blob/master/example/quantization/imagenet_gen_qsym.py#L49
   
   However, the inference speed actually got slower on P100/GTX 1080ti:
   
   **Original model**
   
   ```
   CUDA_VISIBLE_DEVICES=0 python imagenet_inference.py 
--symbol-file=./model/imagenet1k-resnet-152-symbol.json 
--param-file=./model/imagenet1k-resnet-152-.params --rgb-mean=0,0,0 
--num-skipped-batches=50 --num-inference-batches=500 
--dataset=./data/val_256_q90.rec
   
   INFO:logger:Finished inference with 16000 images
   INFO:logger:Finished with 252.172165 images per second
   INFO:logger:('accuracy', 0.772375)
   INFO:logger:('top_k_accuracy_5', 0.93)
   ```
   **Quantized model**
   
   ```
   CUDA_VISIBLE_DEVICES=0 python imagenet_inference.py 
--symbol-file=./model/imagenet1k-resnet-152-quantized-5batches-naive-symbol.json
 --param-file=./model/imagenet1k-resnet-152-quantized-.params 
--rgb-mean=0,0,0 --num-skipped-batches=50 --num-inference-batches=500 
--dataset=./data/val_256_q90.rec
   
   INFO:logger:Finished inference with 16000 images
   INFO:logger:Finished with 105.886662 images per second
   INFO:logger:('accuracy', 0.7596875)
   INFO:logger:('top_k_accuracy_5', 0.9234375)
   ```
   
   When I tried to changed the saved context to `gpu()` on 
https://github.com/apache/incubator-mxnet/blob/master/example/quantization/imagenet_gen_qsym.py#L49
   cuDNN threw an exception during the inference:
   
   ```
File 
"/opt/home/linpengt/anaconda3/lib/python3.6/site-packages/mxnet/base.py", line 
149, in check_call
   raise MXNetError(py_str(_LIB.MXGetLastError()))
   mxnet.base.MXNetError: [15:40:54] 
src/operator/quantization/quantized_conv.cu:242: Check failed: e == 
CUDNN_STATUS_SUCCESS (6 vs. 0) cuDNN: CUDNN_STATUS_ARCH_MISMATCH
   ```
   
   Just wondering if the speed decrease and the error is as expected, or if 
there is a way to fix that. Thanks!
   ​
   ## Environment info (Required)
   
   ```
   --Python Info--
   Version  : 3.6.3
   Compiler : GCC 7.2.0
   Build: ('default', 'Oct 13 2017 12:02:49')
   Arch : ('64bit', '')
   Pip Info---
   Version  : 10.0.1
   Directory: /opt/home/linpengt/anaconda3/lib/python3.6/site-packages/pip
   --MXNet Info---
   Version  : 1.2.0
   Directory: /opt/home/linpengt/anaconda3/lib/python3.6/site-packages/mxnet
   Commit Hash   : f02caae5a06c84d2e764f3a4c4283cf838e3d69f
   --System Info--
   Platform : Linux-4.4.0-121-generic-x86_64-with-debian-stretch-sid
   system   : Linux
   node : dev0006.internal.moqi.ai
   release  : 4.4.0-121-generic
   version  : #145-Ubuntu SMP Fri Apr 13 13:47:23 UTC 2018
   --Hardware Info--
   machine  : x86_64
   processor: x86_64
   Architecture:  x86_64
   CPU op-mode(s):32-bit, 64-bit
   Byte Order:Little Endian
   CPU(s):56
   On-line CPU(s) list:   0-55
   Thread(s) per core:2
   Core(s) per socket:14
   Socket(s): 2
   NUMA node(s):  2
   Vendor ID: GenuineIntel
   CPU family:6
   Model: 79
   Model name:Intel(R) Xeon(R) CPU E5-2690 v4 @ 2.60GHz
   Stepping:  1
   CPU MHz:   1256.125
   CPU max MHz:   3500.
   CPU min MHz:   1200.
   BogoMIPS:  5201.68
   Virtualization:VT-x
   L1d cache: 32K
   L1i cache: 32K
   L2 cache:  256K
   L3 cache:  35840K
   NUMA node0 CPU(s): 0-13,28-41
   NUMA node1 CPU(s): 14-27,42-55
   Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge 
mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est 
tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch epb 
invpcid_single intel_pt retpoline kaiser tpr_shadow vnmi flexpriority ept vpid 
fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm cqm rdseed adx 
smap xsaveopt cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local dtherm ida arat 
pln pts
   --Network 

[GitHub] zheng-da commented on a change in pull request #10888: Fix thread local.

[GitBox]

zheng-da commented on a change in pull request #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#discussion_r187513606
 
 

 ##
 File path: src/operator/nn/mkldnn/mkldnn_act.cc
 ##
 @@ -134,7 +134,11 @@ class MKLDNNActForward {
 static MKLDNNActForward (const ActivationParam& param,
const OpContext , const NDArray 
_data,
const mkldnn::memory _mem) {
+#if DMLC_CXX11_THREAD_LOCAL
   static thread_local std::unordered_map fwds;
+#else
+  static MX_THREAD_LOCAL std::unordered_map fwds;
 
 Review comment:
   i think so.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on a change in pull request #10888: Fix thread local.

[GitBox]

zheng-da commented on a change in pull request #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#discussion_r187513562
 
 

 ##
 File path: src/operator/nn/deconvolution.cu
 ##
 @@ -40,9 +40,15 @@ static CuDNNDeconvolutionOp (const 
DeconvolutionParam& p
  const 
std::vector& in_shape,
  const 
std::vector& out_shape,
  const RunContext& rctx) {
+#if DMLC_CXX11_THREAD_LOCAL
 
 Review comment:
   not entirely sure. but the way we are using right now is to use 
MX_THREAD_LOCAL and thread_local depending on the macro.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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

[incubator-mxnet-site] branch asf-site updated: Bump the publish timestamp.

[zhasheng]

This is an automated email from the ASF dual-hosted git repository.

zhasheng pushed a commit to branch asf-site
in repository https://gitbox.apache.org/repos/asf/incubator-mxnet-site.git


The following commit(s) were added to refs/heads/asf-site by this push:
 new 6f59609  Bump the publish timestamp.
6f59609 is described below

commit 6f596098254b740ced99783b529478dde2e00061
Author: mxnet-ci 
AuthorDate: Fri May 11 01:56:42 2018 +

Bump the publish timestamp.
---
 date.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/date.txt b/date.txt
new file mode 100644
index 000..366e8c8
--- /dev/null
+++ b/date.txt
@@ -0,0 +1 @@
+Fri May 11 01:56:41 UTC 2018

-- 
To stop receiving notification emails like this one, please contact
zhash...@apache.org.

[GitHub] DickJC123 opened a new issue #10896: test_sparse_operator.py:test_sparse_mathematical_core fails under scipy v1.1

[GitBox]

DickJC123 opened a new issue #10896: 
test_sparse_operator.py:test_sparse_mathematical_core fails under scipy v1.1
URL: https://github.com/apache/incubator-mxnet/issues/10896
 
 
   
   ## Description
   Our CI systems pull in scipy via 'pip install scipy' and are now getting 
version 1.1 from the public server, rather than the previous 1.0.  The trouble 
with this is that the function scipy.special.psi() has a different behavior in 
the two versions when passed a 0 argument and this causes 
test_sparse_operator.py:test_sparse_mathematical_core to fail.
   
   ## Environment info (Required)
   
   ```
   --Python Info--
   ('Version  :', '2.7.12')
   ('Compiler :', 'GCC 5.4.0 20160609')
   ('Build:', ('default', 'Dec  4 2017 14:50:18'))
   ('Arch :', ('64bit', 'ELF'))
   Pip Info---
   ('Version  :', '10.0.1')
   ('Directory:', '/home/dcarter/.local/lib/python2.7/site-packages/pip')
   --MXNet Info---
   /home/dcarter/mxnet_dev/dgx/mxnet/python/mxnet/optimizer.py:136: 
UserWarning: WARNING: New optimizer mxnet.optimizer.NAG is overriding existing 
optimizer mxnet.optimizer.NAG
 Optimizer.opt_registry[name].__name__))
   ('Version  :', '1.1.0')
   ('Directory:', '/home/dcarter/mxnet_dev/dgx/mxnet/python/mxnet')
   Hashtag not found. Not installed from pre-built package.
   --System Info--
   ('Platform :', 'Linux-4.4.0-121-generic-x86_64-with-Ubuntu-16.04-xenial')
   ('system   :', 'Linux')
   ('node :', 'DCARTER-DT')
   ('release  :', '4.4.0-121-generic')
   ('version  :', '#145-Ubuntu SMP Fri Apr 13 13:47:23 UTC 2018')
   --Hardware Info--
   ('machine  :', 'x86_64')
   ('processor:', 'x86_64')
   Architecture:  x86_64
   CPU op-mode(s):32-bit, 64-bit
   Byte Order:Little Endian
   CPU(s):12
   On-line CPU(s) list:   0-11
   Thread(s) per core:2
   Core(s) per socket:6
   Socket(s): 1
   NUMA node(s):  1
   Vendor ID: GenuineIntel
   CPU family:6
   Model: 63
   Model name:Intel(R) Core(TM) i7-5930K CPU @ 3.50GHz
   Stepping:  2
   CPU MHz:   1292.539
   CPU max MHz:   3700.
   CPU min MHz:   1200.
   BogoMIPS:  6996.26
   Virtualization:VT-x
   L1d cache: 32K
   L1i cache: 32K
   L2 cache:  256K
   L3 cache:  15360K
   NUMA node0 CPU(s): 0-11
   Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge 
mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 
ssse3 sdbg fma cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm epb invpcid_single 
retpoline kaiser tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 
avx2 smep bmi2 erms invpcid cqm xsaveopt cqm_llc cqm_occup_llc dtherm ida arat 
pln pts
   
   ```
   
   ## Error Message:
   
   See the end of 'Steps to reproduce'
   
   ## Steps to reproduce:
   
   Normal/correct behavior:
   
   ```
   $ python -c "import numpy; print(numpy.__version__)"
   1.14.3
   $ python -c "import scipy; print(scipy.__version__)"
   1.0.0
   $ python -c "import scipy.special; print(scipy.special.psi(0))"
   inf
   $ nosetests 
tests/python/unittest/test_sparse_operator.py:test_sparse_mathematical_core
...
   OK
   ```
   Problematic behavior:
   ```
   $ sudo pip install scipy==1.1 --force-reinstall
   $ python -c "import numpy; print(numpy.__version__)"
   1.14.3
   $ python -c "import scipy; print(scipy.__version__)"
   1.1.0
   $ python -c "import scipy.special; print(scipy.special.psi(0))"
   -inf <  Note minus inf, different than with scipy 
v1.0
   $ nosetests 
tests/python/unittest/test_sparse_operator.py:test_sparse_mathematical_core
   /home/dcarter/mxnet_dev/dgx/mxnet/python/mxnet/optimizer.py:136: 
UserWarning: WARNING: New optimizer mxnet.optimizer.NAG is overriding existing 
optimizer mxnet.optimizer.NAG
 Optimizer.opt_registry[name].__name__))
   [INFO] Setting module np/mx/python random seeds, use 
MXNET_MODULE_SEED=900978872 to reproduce.
   [INFO] Setting test np/mx/python random seeds, use 
MXNET_TEST_SEED=2040547495 to reproduce.
   F
   ==
   FAIL: test_sparse_operator.test_sparse_mathematical_core
   --
   Traceback (most recent call last):
 File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
   self.test(*self.arg)
 File 

[GitHub] szha closed issue #9634: [Feature request] Warning message or exception on mxnet.metric.Loss.update interface for performance

[GitBox]

szha closed issue #9634: [Feature request] Warning message or exception on 
mxnet.metric.Loss.update interface for performance
URL: https://github.com/apache/incubator-mxnet/issues/9634
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] szha closed pull request #10892: [MXNET-412] Loss update performance

[GitBox]

szha closed pull request #10892: [MXNET-412] Loss update performance
URL: https://github.com/apache/incubator-mxnet/pull/10892
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/python/mxnet/metric.py b/python/mxnet/metric.py
index 76118ccfd2c..aa3ab44c48a 100644
--- a/python/mxnet/metric.py
+++ b/python/mxnet/metric.py
@@ -421,7 +421,7 @@ def update(self, labels, preds):
 label = label.flat
 pred_label = pred_label.flat
 
-labels, preds = check_label_shapes(label, pred_label)
+check_label_shapes(label, pred_label)
 
 self.sum_metric += (pred_label == label).sum()
 self.num_inst += len(pred_label)
@@ -1159,6 +1159,10 @@ def __init__(self, name='loss',
 name, output_names=output_names, label_names=label_names)
 
 def update(self, _, preds):
+
+if isinstance(preds, ndarray.ndarray.NDArray):
+preds = [preds]
+
 for pred in preds:
 self.sum_metric += ndarray.sum(pred).asscalar()
 self.num_inst += pred.size
diff --git a/tests/python/unittest/test_metric.py 
b/tests/python/unittest/test_metric.py
index 1571a0b2428..7bc9c10ce5b 100644
--- a/tests/python/unittest/test_metric.py
+++ b/tests/python/unittest/test_metric.py
@@ -32,6 +32,7 @@ def test_metrics():
 check_metric('perplexity', -1)
 check_metric('pearsonr')
 check_metric('nll_loss')
+check_metric('loss')
 composite = mx.metric.create(['acc', 'f1'])
 check_metric(composite)
 
@@ -41,7 +42,6 @@ def test_nll_loss():
 label = mx.nd.array([2, 1])
 metric.update([label], [pred])
 _, loss = metric.get()
-expected_loss = 0.0
 expected_loss = -(np.log(pred[0][2].asscalar()) + 
np.log(pred[1][1].asscalar())) / 2
 assert loss == expected_loss
 
@@ -65,6 +65,16 @@ def test_acc_2d_label():
float(label.asnumpy().ravel().size)
 assert acc == expected_acc
 
+def test_loss_update():
+pred = mx.nd.array([[0.3, 0.7], [0, 1.], [0.4, 0.6]])
+metric1 = mx.metric.create('loss')
+metric2 = mx.metric.create('loss')
+metric1.update(None, [pred])
+metric2.update(None, pred)
+_, acc1 = metric1.get()
+_, acc2 = metric2.get()
+assert acc1 == acc2
+
 def test_f1():
 microF1 = mx.metric.create("f1", average="micro")
 macroF1 = mx.metric.F1(average="macro")


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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

[incubator-mxnet] branch master updated: [MXNET-412] Loss update performance (#10892)

[zhasheng]

This is an automated email from the ASF dual-hosted git repository.

zhasheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-mxnet.git


The following commit(s) were added to refs/heads/master by this push:
 new 4967feb  [MXNET-412] Loss update performance (#10892)
4967feb is described below

commit 4967feb0860b768a5da88ce8c11ea8f41fa458b4
Author: Alexander Zai 
AuthorDate: Thu May 10 18:08:42 2018 -0700

[MXNET-412] Loss update performance (#10892)

* convert pred to list if ndarray is input

* remove unused line

* remove unused line

* test loss.update can accept list and ndarray

* do not store return values when checking shape in accuracy.update
---
 python/mxnet/metric.py   |  6 +-
 tests/python/unittest/test_metric.py | 12 +++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/python/mxnet/metric.py b/python/mxnet/metric.py
index 76118cc..aa3ab44 100644
--- a/python/mxnet/metric.py
+++ b/python/mxnet/metric.py
@@ -421,7 +421,7 @@ class Accuracy(EvalMetric):
 label = label.flat
 pred_label = pred_label.flat
 
-labels, preds = check_label_shapes(label, pred_label)
+check_label_shapes(label, pred_label)
 
 self.sum_metric += (pred_label == label).sum()
 self.num_inst += len(pred_label)
@@ -1159,6 +1159,10 @@ class Loss(EvalMetric):
 name, output_names=output_names, label_names=label_names)
 
 def update(self, _, preds):
+
+if isinstance(preds, ndarray.ndarray.NDArray):
+preds = [preds]
+
 for pred in preds:
 self.sum_metric += ndarray.sum(pred).asscalar()
 self.num_inst += pred.size
diff --git a/tests/python/unittest/test_metric.py 
b/tests/python/unittest/test_metric.py
index 1571a0b..7bc9c10 100644
--- a/tests/python/unittest/test_metric.py
+++ b/tests/python/unittest/test_metric.py
@@ -32,6 +32,7 @@ def test_metrics():
 check_metric('perplexity', -1)
 check_metric('pearsonr')
 check_metric('nll_loss')
+check_metric('loss')
 composite = mx.metric.create(['acc', 'f1'])
 check_metric(composite)
 
@@ -41,7 +42,6 @@ def test_nll_loss():
 label = mx.nd.array([2, 1])
 metric.update([label], [pred])
 _, loss = metric.get()
-expected_loss = 0.0
 expected_loss = -(np.log(pred[0][2].asscalar()) + 
np.log(pred[1][1].asscalar())) / 2
 assert loss == expected_loss
 
@@ -65,6 +65,16 @@ def test_acc_2d_label():
float(label.asnumpy().ravel().size)
 assert acc == expected_acc
 
+def test_loss_update():
+pred = mx.nd.array([[0.3, 0.7], [0, 1.], [0.4, 0.6]])
+metric1 = mx.metric.create('loss')
+metric2 = mx.metric.create('loss')
+metric1.update(None, [pred])
+metric2.update(None, pred)
+_, acc1 = metric1.get()
+_, acc2 = metric2.get()
+assert acc1 == acc2
+
 def test_f1():
 microF1 = mx.metric.create("f1", average="micro")
 macroF1 = mx.metric.F1(average="macro")

-- 
To stop receiving notification emails like this one, please contact
zhash...@apache.org.

[GitHub] threeleafzerg commented on a change in pull request #10696: [MXNET-366]Extend MXNet Distributed Training by MPI AllReduce

[GitBox]

threeleafzerg commented on a change in pull request #10696: [MXNET-366]Extend 
MXNet Distributed Training by MPI AllReduce
URL: https://github.com/apache/incubator-mxnet/pull/10696#discussion_r187496727
 
 

 ##
 File path: Makefile
 ##
 @@ -382,6 +402,10 @@ else
endif
 endif
 
+MPI_SRC = $(wildcard  src/mpi_collectives/src/*.cc)
 
 Review comment:
   @szha I know that. SRC_FILTER is put here for future extension. Because 
maybe in future, another source code which is put within src/ need different 
compilation rule same as MPI_SRC.  SRC_FILTER will looks like  SRC_FILTER += 
$(wildcard src/[other-component]/src/*.cc)


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] threeleafzerg commented on a change in pull request #10696: [MXNET-366]Extend MXNet Distributed Training by MPI AllReduce

[GitBox]

threeleafzerg commented on a change in pull request #10696: [MXNET-366]Extend 
MXNet Distributed Training by MPI AllReduce
URL: https://github.com/apache/incubator-mxnet/pull/10696#discussion_r187496727
 
 

 ##
 File path: Makefile
 ##
 @@ -382,6 +402,10 @@ else
endif
 endif
 
+MPI_SRC = $(wildcard  src/mpi_collectives/src/*.cc)
 
 Review comment:
   @szha I know that. SRC_FILTER is put here for future extension. Because 
maybe in future, another source code which is put within src/ need different 
compilation rule same as MPI_SRC.  SRC_FILTER will looks like  SRC_FILTER += 
$(wildcard src//src/*.cc)


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rahul003 commented on a change in pull request #10845: [MXNET-406] support init/pull dense weight, push row_sparse grad in kvstore

[GitBox]

rahul003 commented on a change in pull request #10845: [MXNET-406] support 
init/pull dense weight, push row_sparse grad in kvstore
URL: https://github.com/apache/incubator-mxnet/pull/10845#discussion_r187449223
 
 

 ##
 File path: src/kvstore/comm.h
 ##
 @@ -228,7 +244,11 @@ class CommCPU : public Comm {
   NDArray retained_cpu = (is_same_ctx && is_diff_var) ? *out :
   NDArray(kRowSparseStorage, src.shape(), src.ctx(), true,
   src.dtype(), src.aux_types());
-
+  if (!is_diff_var) {
+common::LogOnce("The output of row_sparse_pull on key " + 
std::to_string(key) +
+"refers to the same NDArray as the one stored in 
KVStore."
+"Incorrect result may be generated.");
 
 Review comment:
   What does this refer to? "Incorrect result may be generated" is unclear


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudh2290 commented on a change in pull request #10888: Fix thread local.

[GitBox]

anirudh2290 commented on a change in pull request #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#discussion_r187491601
 
 

 ##
 File path: src/operator/nn/mkldnn/mkldnn_act.cc
 ##
 @@ -134,7 +134,11 @@ class MKLDNNActForward {
 static MKLDNNActForward (const ActivationParam& param,
const OpContext , const NDArray 
_data,
const mkldnn::memory _mem) {
+#if DMLC_CXX11_THREAD_LOCAL
   static thread_local std::unordered_map fwds;
+#else
+  static MX_THREAD_LOCAL std::unordered_map fwds;
 
 Review comment:
   do we need to make this change here too: 
https://github.com/apache/incubator-mxnet/blob/35fb9ea1093bd0006a396b5e493e5949b66ca33e/src/c_api/c_api_profile.cc#L115
 ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rahul003 commented on a change in pull request #8918: Added in Large-Batch SGD with a warmup, and a LARS startegy. Also add…

[GitBox]

rahul003 commented on a change in pull request #8918: Added in Large-Batch SGD 
with a warmup, and a LARS startegy. Also add…
URL: https://github.com/apache/incubator-mxnet/pull/8918#discussion_r187485127
 
 

 ##
 File path: example/image-classification/common/fit.py
 ##
 @@ -180,40 +205,81 @@ def fit(args, network, data_loader, **kwargs):
 if args.optimizer in has_momentum:
 optimizer_params['momentum'] = args.mom
 
-monitor = mx.mon.Monitor(args.monitor, pattern=".*") if args.monitor > 0 
else None
+monitor = mx.mon.Monitor(
+args.monitor, pattern=".*") if args.monitor > 0 else None
 
-if args.network == 'alexnet':
-# AlexNet will not converge using Xavier
-initializer = mx.init.Normal()
-else:
-initializer = mx.init.Xavier(
-rnd_type='gaussian', factor_type="in", magnitude=2)
+# A limited number of optimizers have a warmup period
+has_warmup = {'lbsgd', 'lbnag'}
 
 Review comment:
   @chaseadams509 How large batch sizes did you experiment with? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] kpmurali opened a new pull request #10895: [MXNET-413] Fixing the broken links - Week of 5/7

[GitBox]

kpmurali opened a new pull request #10895: [MXNET-413] Fixing the broken links 
- Week of 5/7
URL: https://github.com/apache/incubator-mxnet/pull/10895
 
 
   ## Description ##
   Fixing the second set of broken links in the MXNet site.
   
   ### Changes ###
   - [ ] Fixing the broken link in the install download page
   - [ ] Fixing the scala-packages links


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] chaseadams509 commented on a change in pull request #8918: Added in Large-Batch SGD with a warmup, and a LARS startegy. Also add…

[GitBox]

chaseadams509 commented on a change in pull request #8918: Added in Large-Batch 
SGD with a warmup, and a LARS startegy. Also add…
URL: https://github.com/apache/incubator-mxnet/pull/8918#discussion_r187483224
 
 

 ##
 File path: example/image-classification/common/fit.py
 ##
 @@ -180,40 +205,81 @@ def fit(args, network, data_loader, **kwargs):
 if args.optimizer in has_momentum:
 optimizer_params['momentum'] = args.mom
 
-monitor = mx.mon.Monitor(args.monitor, pattern=".*") if args.monitor > 0 
else None
+monitor = mx.mon.Monitor(
+args.monitor, pattern=".*") if args.monitor > 0 else None
 
-if args.network == 'alexnet':
-# AlexNet will not converge using Xavier
-initializer = mx.init.Normal()
-else:
-initializer = mx.init.Xavier(
-rnd_type='gaussian', factor_type="in", magnitude=2)
+# A limited number of optimizers have a warmup period
+has_warmup = {'lbsgd', 'lbnag'}
 
 Review comment:
   Correct, lbnag was an optimizer we were experimenting with with 
incorporating the large-batch algorithm with Nesterov accelerated gradient. 
LBNAG was not showing the desired improvements, so we hadn't pushed it yet. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] chaseadams509 commented on a change in pull request #8918: Added in Large-Batch SGD with a warmup, and a LARS startegy. Also add…

[GitBox]

chaseadams509 commented on a change in pull request #8918: Added in Large-Batch 
SGD with a warmup, and a LARS startegy. Also add…
URL: https://github.com/apache/incubator-mxnet/pull/8918#discussion_r187483224
 
 

 ##
 File path: example/image-classification/common/fit.py
 ##
 @@ -180,40 +205,81 @@ def fit(args, network, data_loader, **kwargs):
 if args.optimizer in has_momentum:
 optimizer_params['momentum'] = args.mom
 
-monitor = mx.mon.Monitor(args.monitor, pattern=".*") if args.monitor > 0 
else None
+monitor = mx.mon.Monitor(
+args.monitor, pattern=".*") if args.monitor > 0 else None
 
-if args.network == 'alexnet':
-# AlexNet will not converge using Xavier
-initializer = mx.init.Normal()
-else:
-initializer = mx.init.Xavier(
-rnd_type='gaussian', factor_type="in", magnitude=2)
+# A limited number of optimizers have a warmup period
+has_warmup = {'lbsgd', 'lbnag'}
 
 Review comment:
   Correct, lbnag was an optimizer we were experimenting with with 
incorporating the large-batch algorithm with Nesterov accelerated gradient. 
LBNAG was not showing the desired improvements, so we hadn't pushed it yet. 
Because of that, lbnag should probably be removed from this list.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187482860
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
 
 Review comment:
   It's already available.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187482906
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cu
 ##
 @@ -77,6 +77,9 @@ NNVM_REGISTER_OP(_identity_with_attr_like_rhs)
 NNVM_REGISTER_OP(reshape_like)
 .set_attr("FCompute", UnaryOp::IdentityCompute);
 
+NNVM_REGISTER_OP(shape)
 
 Review comment:
   Please make sure the doc page can be rendered correctly at least.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ashokei commented on a change in pull request #8918: Added in Large-Batch SGD with a warmup, and a LARS startegy. Also add…

[GitBox]

ashokei commented on a change in pull request #8918: Added in Large-Batch SGD 
with a warmup, and a LARS startegy. Also add…
URL: https://github.com/apache/incubator-mxnet/pull/8918#discussion_r187482640
 
 

 ##
 File path: example/image-classification/common/fit.py
 ##
 @@ -180,40 +205,81 @@ def fit(args, network, data_loader, **kwargs):
 if args.optimizer in has_momentum:
 optimizer_params['momentum'] = args.mom
 
-monitor = mx.mon.Monitor(args.monitor, pattern=".*") if args.monitor > 0 
else None
+monitor = mx.mon.Monitor(
+args.monitor, pattern=".*") if args.monitor > 0 else None
 
-if args.network == 'alexnet':
-# AlexNet will not converge using Xavier
-initializer = mx.init.Normal()
-else:
-initializer = mx.init.Xavier(
-rnd_type='gaussian', factor_type="in", magnitude=2)
+# A limited number of optimizers have a warmup period
+has_warmup = {'lbsgd', 'lbnag'}
 
 Review comment:
   @chaseadams509 can you please address above comments. thanks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] lanking520 commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

lanking520 commented on a change in pull request #10660: [MXNET-357] New Scala 
API Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187481009
 
 

 ##
 File path: scala-package/init/src/main/scala/org/apache/mxnet/init/Base.scala
 ##
 @@ -37,7 +37,11 @@ object Base {
 
   @throws(classOf[UnsatisfiedLinkError])
   private def tryLoadInitLibrary(): Unit = {
-val baseDir = System.getProperty("user.dir") + "/init-native"
+// val baseDir = System.getProperty("user.dir") + "/init-native"
+var baseDir = System.getProperty("user.dir") + "/init-native"
+if (System.getenv().containsKey("BASEDIR")) {
+  baseDir = sys.env("BASEDIR")
+}
 
 Review comment:
   Done


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] lanking520 commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

lanking520 commented on a change in pull request #10660: [MXNET-357] New Scala 
API Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187480296
 
 

 ##
 File path: 
scala-package/macros/src/main/scala/org/apache/mxnet/SymbolMacro.scala
 ##
 @@ -136,20 +182,80 @@ private[mxnet] object SymbolImplMacros {
 result
   }
 
+  // Convert C++ Types to Scala Types
+  def typeConversion(in : String, argType : String = "") : String = {
+in match {
+  case "Shape(tuple)" | "ShapeorNone" => "org.apache.mxnet.Shape"
+  case "Symbol" | "NDArray" | "NDArray-or-Symbol" => 
"org.apache.mxnet.Symbol"
+  case "Symbol[]" | "NDArray[]" | "NDArray-or-Symbol[]" | 
"SymbolorSymbol[]"
+  => "Array[org.apache.mxnet.Symbol]"
+  case "float" | "real_t" | "floatorNone" => 
"org.apache.mxnet.Base.MXFloat"
+  case "int" | "intorNone" | "int(non-negative)" => "Int"
+  case "long" | "long(non-negative)" => "Long"
+  case "double" | "doubleorNone" => "Double"
+  case "string" => "String"
+  case "boolean" => "Boolean"
+  case "tupleof" | "tupleof" | "ptr" | "" => "Any"
+  case default => throw new IllegalArgumentException(
+s"Invalid type for args: $default, $argType")
+}
+  }
+
+
+  /**
+* By default, the argType come from the C++ API is a description more than 
a single word
+* For Example:
+*   , , 

[GitHub] rahul003 commented on issue #10853: update mshadow

[GitBox]

rahul003 commented on issue #10853: update mshadow
URL: https://github.com/apache/incubator-mxnet/pull/10853#issuecomment-388207581
 
 
   Hmm so it looks like this is what happened. We had updated mshadow to this 
commit for both v1.2 release branch and master branch. But this PR 
https://github.com/apache/incubator-mxnet/pull/10629 from 20 days back was 
trying to update mshadow to a commit which became older by the time it was 
merged 3 days back. That ended up taking mshadow back to an older commit. 
   
   Thanks for updating it now. But I wonder if git can help us avoid taking 
back submodules like this. 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rahul003 commented on issue #10853: update mshadow

[GitBox]

rahul003 commented on issue #10853: update mshadow
URL: https://github.com/apache/incubator-mxnet/pull/10853#issuecomment-388207581
 
 
   Hmm so it looks like this is what happened. We had updated mshadow to this 
commit for both v1.2 release branch and master branch. But this PR 
https://github.com/apache/incubator-mxnet/pull/10629 from 20 days back was 
trying to update mshadow to a commit which became stale by the time it was 
merged 3 days back. That ended up taking mshadow back to an older commit. 
   
   Thanks for updating it now. But I wonder if git can help us avoid taking 
back submodules like this. 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187479770
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
 
 Review comment:
   maybe it should. I'll think about this


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187479673
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
 
 Review comment:
   That shouldn't be allowed


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187479639
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
 
 Review comment:
   looks like its outdated. we should update


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] haojin2 opened a new pull request #10894: [MXNET-399] [WIP] Elemwise_mul between dense and csr on CPU & GPU

[GitBox]

haojin2 opened a new pull request #10894: [MXNET-399] [WIP] Elemwise_mul 
between dense and csr on CPU & GPU
URL: https://github.com/apache/incubator-mxnet/pull/10894
 
 
   ## Description ##
   As title
   
   ## Checklist ##
   ### Essentials ###
   - [x] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to 
the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) 
created (except PRs with tiny changes)
   - [x] Changes are complete (i.e. I finished coding on this PR)
   - [x] All changes have test coverage:
   - Unit tests are added for small changes to verify correctness (e.g. adding 
a new operator)
   - Nightly tests are added for complicated/long-running ones (e.g. changing 
distributed kvstore)
   - Build tests will be added for build configuration changes (e.g. adding a 
new build option with NCCL)
   - [x] Code is well-documented: 
   - For user-facing API changes, API doc string has been updated. 
   - For new C++ functions in header files, their functionalities and arguments 
are documented. 
   - For new examples, README.md is added to explain the what the example does, 
the source of the dataset, expected performance on test set and reference to 
the original paper if applicable
   - Check the API doc at 
http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
   - [x] To the my best knowledge, examples are either not affected by this 
change, or have been fixed to be compatible with this change
   
   ### Changes ###
   - [x] Elemwise_mul between default and csr
   - [x] Corresponding unit tests
   
   ## Comments ##
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] eric-haibin-lin commented on a change in pull request #10888: Fix thread local.

[GitBox]

eric-haibin-lin commented on a change in pull request #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#discussion_r187477992
 
 

 ##
 File path: src/operator/nn/deconvolution.cu
 ##
 @@ -40,9 +40,15 @@ static CuDNNDeconvolutionOp (const 
DeconvolutionParam& p
  const 
std::vector& in_shape,
  const 
std::vector& out_shape,
  const RunContext& rctx) {
+#if DMLC_CXX11_THREAD_LOCAL
 
 Review comment:
   What about `dmlc::ThreadLocalStore`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187476303
 
 

 ##
 File path: scala-package/init/src/main/scala/org/apache/mxnet/init/Base.scala
 ##
 @@ -37,7 +37,11 @@ object Base {
 
   @throws(classOf[UnsatisfiedLinkError])
   private def tryLoadInitLibrary(): Unit = {
-val baseDir = System.getProperty("user.dir") + "/init-native"
+// val baseDir = System.getProperty("user.dir") + "/init-native"
+var baseDir = System.getProperty("user.dir") + "/init-native"
+if (System.getenv().containsKey("BASEDIR")) {
+  baseDir = sys.env("BASEDIR")
+}
 
 Review comment:
   for system environment var, better to specify it is mxnet-related, e.g., 
MXNET_SCALA_MACRO_BASEDIR or something like it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187474931
 
 

 ##
 File path: 
scala-package/macros/src/main/scala/org/apache/mxnet/SymbolMacro.scala
 ##
 @@ -136,20 +182,80 @@ private[mxnet] object SymbolImplMacros {
 result
   }
 
+  // Convert C++ Types to Scala Types
+  def typeConversion(in : String, argType : String = "") : String = {
+in match {
+  case "Shape(tuple)" | "ShapeorNone" => "org.apache.mxnet.Shape"
+  case "Symbol" | "NDArray" | "NDArray-or-Symbol" => 
"org.apache.mxnet.Symbol"
+  case "Symbol[]" | "NDArray[]" | "NDArray-or-Symbol[]" | 
"SymbolorSymbol[]"
+  => "Array[org.apache.mxnet.Symbol]"
+  case "float" | "real_t" | "floatorNone" => 
"org.apache.mxnet.Base.MXFloat"
+  case "int" | "intorNone" | "int(non-negative)" => "Int"
+  case "long" | "long(non-negative)" => "Long"
+  case "double" | "doubleorNone" => "Double"
+  case "string" => "String"
+  case "boolean" => "Boolean"
+  case "tupleof" | "tupleof" | "ptr" | "" => "Any"
+  case default => throw new IllegalArgumentException(
+s"Invalid type for args: $default, $argType")
+}
+  }
+
+
+  /**
+* By default, the argType come from the C++ API is a description more than 
a single word
+* For Example:
+*   , , 

[GitHub] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187465827
 
 

 ##
 File path: 
scala-package/macros/src/main/scala/org/apache/mxnet/SymbolMacro.scala
 ##
 @@ -21,109 +21,155 @@ import scala.annotation.StaticAnnotation
 import scala.collection.mutable.ListBuffer
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox
-
 import org.apache.mxnet.init.Base._
 import org.apache.mxnet.utils.OperatorBuildUtils
 
 private[mxnet] class AddSymbolFunctions(isContrib: Boolean) extends 
StaticAnnotation {
   private[mxnet] def macroTransform(annottees: Any*) = macro 
SymbolImplMacros.addDefs
 }
 
+private[mxnet] class AddSymbolAPIs(isContrib: Boolean) extends 
StaticAnnotation {
+  private[mxnet] def macroTransform(annottees: Any*) = macro 
SymbolImplMacros.addNewDefs
+}
+
 private[mxnet] object SymbolImplMacros {
-  case class SymbolFunction(handle: SymbolHandle, keyVarNumArgs: String)
+  case class SymbolArg(argName: String, argType: String, isOptional : Boolean)
+  case class SymbolFunction(name: String, listOfArgs: List[SymbolArg])
 
   // scalastyle:off havetype
   def addDefs(c: blackbox.Context)(annottees: c.Expr[Any]*) = {
-impl(c)(false, annottees: _*)
+impl(c)(annottees: _*)
   }
-  // scalastyle:off havetype
+  def addNewDefs(c: blackbox.Context)(annottees: c.Expr[Any]*) = {
+newAPIImpl(c)(annottees: _*)
 
 Review comment:
   can we have a more meaningful name, for example, `typeSafeAPI`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187470051
 
 

 ##
 File path: 
scala-package/macros/src/main/scala/org/apache/mxnet/SymbolMacro.scala
 ##
 @@ -136,20 +132,67 @@ private[mxnet] object SymbolImplMacros {
 result
   }
 
+  // Convert C++ Types to Scala Types
+  private def typeConversion(in : String, argType : String = "") : String = {
+in match {
+  case "Shape(tuple)" | "ShapeorNone" => "org.apache.mxnet.Shape"
+  case "Symbol" | "NDArray" | "NDArray-or-Symbol" => 
"org.apache.mxnet.Symbol"
+  case "Symbol[]" | "NDArray[]" | "NDArray-or-Symbol[]" | 
"SymbolorSymbol[]"
+  => "Array[org.apache.mxnet.Symbol]"
+  case "float" | "real_t" | "floatorNone" => 
"org.apache.mxnet.Base.MXFloat"
+  case "int" | "intorNone" | "int(non-negative)" => "Int"
+  case "long" | "long(non-negative)" => "Long"
+  case "double" => "Double"
+  case "string" => "String"
+  case "boolean" => "Boolean"
+  case "tupleof" => "Any"
+  case default => throw new IllegalArgumentException(
+s"Invalid type for args: $default, $argType")
+}
+  }
+
+
+
+  private def argumentCleaner(argType : String) : (String, Boolean) = {
+val spaceRemoved = argType.replaceAll("\\s+", "")
+var commaRemoved : Array[String] = new Array[String](0)
+// Deal with the case e.g: stype : {'csr', 'default', 'row_sparse'}
+if (spaceRemoved.charAt(0)== '{') {
+  val endIdx = spaceRemoved.indexOf('}')
+  commaRemoved = spaceRemoved.substring(endIdx + 1).split(",")
+  commaRemoved(0) = "string"
 
 Review comment:
   gotcha


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187475250
 
 

 ##
 File path: 
scala-package/macros/src/test/scala/org/apache/mxnet/MacrosSuite.scala
 ##
 @@ -0,0 +1,57 @@
+/*
+ * 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.
+ */
+
+package org.apache.mxnet
+
+import org.scalatest.{BeforeAndAfterAll, FunSuite}
+import org.slf4j.LoggerFactory
+
+class MacrosSuite extends FunSuite with BeforeAndAfterAll {
+
+  private val logger = LoggerFactory.getLogger(classOf[MacrosSuite])
+
+
+  override def beforeAll() {
+  }
+
+  override def afterAll(): Unit = {
+
+  }
 
 Review comment:
   can be removed?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187475396
 
 

 ##
 File path: scala-package/core/src/main/scala/org/apache/mxnet/SymbolAPI.scala
 ##
 @@ -0,0 +1,20 @@
+/*
+ * 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.
+ */
+package org.apache.mxnet
+@AddSymbolAPIs(false)
+object SymbolAPI {
 
 Review comment:
   comment a bit for this placeholder.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187469159
 
 

 ##
 File path: 
scala-package/macros/src/main/scala/org/apache/mxnet/SymbolMacro.scala
 ##
 @@ -21,109 +21,155 @@ import scala.annotation.StaticAnnotation
 import scala.collection.mutable.ListBuffer
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox
-
 import org.apache.mxnet.init.Base._
 import org.apache.mxnet.utils.OperatorBuildUtils
 
 private[mxnet] class AddSymbolFunctions(isContrib: Boolean) extends 
StaticAnnotation {
   private[mxnet] def macroTransform(annottees: Any*) = macro 
SymbolImplMacros.addDefs
 }
 
+private[mxnet] class AddSymbolAPIs(isContrib: Boolean) extends 
StaticAnnotation {
+  private[mxnet] def macroTransform(annottees: Any*) = macro 
SymbolImplMacros.addNewDefs
+}
+
 private[mxnet] object SymbolImplMacros {
-  case class SymbolFunction(handle: SymbolHandle, keyVarNumArgs: String)
+  case class SymbolArg(argName: String, argType: String, isOptional : Boolean)
+  case class SymbolFunction(name: String, listOfArgs: List[SymbolArg])
 
   // scalastyle:off havetype
   def addDefs(c: blackbox.Context)(annottees: c.Expr[Any]*) = {
-impl(c)(false, annottees: _*)
+impl(c)(annottees: _*)
   }
-  // scalastyle:off havetype
+  def addNewDefs(c: blackbox.Context)(annottees: c.Expr[Any]*) = {
+newAPIImpl(c)(annottees: _*)
+  }
+  // scalastyle:on havetype
 
-  private val symbolFunctions: Map[String, SymbolFunction] = initSymbolModule()
+  private val symbolFunctions: List[SymbolFunction] = initSymbolModule()
 
-  private def impl(c: blackbox.Context)(addSuper: Boolean, annottees: 
c.Expr[Any]*): c.Expr[Any] = {
+  /**
+* Implementation for fixed input API structure
+*/
+  private def impl(c: blackbox.Context)(annottees: c.Expr[Any]*): c.Expr[Any] 
= {
 import c.universe._
 
 val isContrib: Boolean = c.prefix.tree match {
   case q"new AddSymbolFunctions($b)" => c.eval[Boolean](c.Expr(b))
 }
 
 val newSymbolFunctions = {
-  if (isContrib) symbolFunctions.filter(_._1.startsWith("_contrib_"))
-  else symbolFunctions.filter(!_._1.startsWith("_contrib_"))
+  if (isContrib) symbolFunctions.filter(
+func => func.name.startsWith("_contrib_") || 
!func.name.startsWith("_"))
+  else symbolFunctions.filter(!_.name.startsWith("_"))
 }
 
-val AST_TYPE_MAP_STRING_ANY = AppliedTypeTree(Ident(TypeName("Map")),
-  List(Ident(TypeName("String")), Ident(TypeName("Any"
-val AST_TYPE_MAP_STRING_STRING = AppliedTypeTree(Ident(TypeName("Map")),
-  List(Ident(TypeName("String")), Ident(TypeName("String"
-val AST_TYPE_SYMBOL_VARARG = AppliedTypeTree(
-  Select(
-Select(Ident(termNames.ROOTPKG), TermName("scala")),
-TypeName("")
-  ),
-  List(Select(Select(Select(
-Ident(TermName("org")), TermName("apache")), TermName("mxnet")), 
TypeName("Symbol")))
-)
-
-val functionDefs = newSymbolFunctions map { case (funcName, funcProp) =>
-  val functionScope = {
-if (isContrib) Modifiers()
-else {
-  if (funcName.startsWith("_")) Modifiers(Flag.PRIVATE) else 
Modifiers()
-}
-  }
-  val newName = {
-if (isContrib) funcName.substring(funcName.indexOf("_contrib_") + 
"_contrib_".length())
-else funcName
+
+val functionDefs = newSymbolFunctions map { symbolfunction =>
+val funcName = symbolfunction.name
+val tName = TermName(funcName)
+q"""
+def $tName(name : String = null, attr : Map[String, String] = null)
+(args : org.apache.mxnet.Symbol*)(kwargs : Map[String, Any] = null)
+ : org.apache.mxnet.Symbol = {
+  createSymbolGeneral($funcName,name,attr,args,kwargs)
+  }
+ """.asInstanceOf[DefDef]
   }
 
-  // It will generate definition something like,
-  // def Concat(name: String = null, attr: Map[String, String] = null)
-  //   (args: Symbol*)(kwargs: Map[String, Any] = null)
-  DefDef(functionScope, TermName(newName), List(),
-List(
-  List(
-ValDef(Modifiers(Flag.PARAM | Flag.DEFAULTPARAM), TermName("name"),
-  Ident(TypeName("String")), Literal(Constant(null))),
-ValDef(Modifiers(Flag.PARAM | Flag.DEFAULTPARAM), TermName("attr"),
-  AST_TYPE_MAP_STRING_STRING, Literal(Constant(null)))
-  ),
-  List(
-ValDef(Modifiers(), TermName("args"), AST_TYPE_SYMBOL_VARARG, 
EmptyTree)
-  ),
-  List(
-ValDef(Modifiers(Flag.PARAM | Flag.DEFAULTPARAM), 
TermName("kwargs"),
-  AST_TYPE_MAP_STRING_ANY, Literal(Constant(null)))
-  )
-), TypeTree(),
-Apply(
-  Ident(TermName("createSymbolGeneral")),
-  List(
-

[GitHub] yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API Design (Symbol)

[GitBox]

yzhliu commented on a change in pull request #10660: [MXNET-357] New Scala API 
Design (Symbol)
URL: https://github.com/apache/incubator-mxnet/pull/10660#discussion_r187476043
 
 

 ##
 File path: scala-package/init/src/main/scala/org/apache/mxnet/init/Base.scala
 ##
 @@ -37,7 +37,11 @@ object Base {
 
   @throws(classOf[UnsatisfiedLinkError])
   private def tryLoadInitLibrary(): Unit = {
-val baseDir = System.getProperty("user.dir") + "/init-native"
+// val baseDir = System.getProperty("user.dir") + "/init-native"
 
 Review comment:
   remove this line.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rahul003 commented on a change in pull request #8918: Added in Large-Batch SGD with a warmup, and a LARS startegy. Also add…

[GitBox]

rahul003 commented on a change in pull request #8918: Added in Large-Batch SGD 
with a warmup, and a LARS startegy. Also add…
URL: https://github.com/apache/incubator-mxnet/pull/8918#discussion_r187475085
 
 

 ##
 File path: example/image-classification/common/fit.py
 ##
 @@ -180,40 +205,81 @@ def fit(args, network, data_loader, **kwargs):
 if args.optimizer in has_momentum:
 optimizer_params['momentum'] = args.mom
 
-monitor = mx.mon.Monitor(args.monitor, pattern=".*") if args.monitor > 0 
else None
+monitor = mx.mon.Monitor(
+args.monitor, pattern=".*") if args.monitor > 0 else None
 
-if args.network == 'alexnet':
-# AlexNet will not converge using Xavier
-initializer = mx.init.Normal()
-else:
-initializer = mx.init.Xavier(
-rnd_type='gaussian', factor_type="in", magnitude=2)
+# A limited number of optimizers have a warmup period
+has_warmup = {'lbsgd', 'lbnag'}
 
 Review comment:
   It looks like LBNAG doesn't exist?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] azai91 commented on a change in pull request #10892: [MXNET-412] Loss update performance

[GitBox]

azai91 commented on a change in pull request #10892: [MXNET-412] Loss update 
performance
URL: https://github.com/apache/incubator-mxnet/pull/10892#discussion_r187473856
 
 

 ##
 File path: python/mxnet/metric.py
 ##
 @@ -421,8 +421,6 @@ def update(self, labels, preds):
 label = label.flat
 pred_label = pred_label.flat
 
-labels, preds = check_label_shapes(label, pred_label)
 
 Review comment:
   IDE detected that we were not using the return values. realize now that we 
are using to check shapes. will just call the function and not store return 
value.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] szha commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

szha commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187474145
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
 
 Review comment:
   This comes from the API doc in step


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] jeremiedb commented on issue #10739: [MXNet-R] package crashes using mx.metric.logloss

[GitBox]

jeremiedb commented on issue #10739: [MXNet-R] package crashes using 
mx.metric.logloss
URL: 
https://github.com/apache/incubator-mxnet/issues/10739#issuecomment-388198275
 
 
   I think the issue was raised from changing the metric calculation to 
NDArrays instead than on the conversion to R arrays as it can be beneficial in 
certain circumstances to keep the eval metric calculation on the device. 
   You can look for instance at this logloss definition: 
   
https://github.com/apache/incubator-mxnet/blob/master/R-package/R/metric.R#L96
   
   If need to perform calculation on R arrays, then you would need to apply the 
conversion within the metric function, for example `label <- as.array(label)`


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da opened a new pull request #10893: benchmark with float16

[GitBox]

zheng-da opened a new pull request #10893: benchmark with float16
URL: https://github.com/apache/incubator-mxnet/pull/10893
 
 
   ## Description ##
   (Brief description on what this PR is about)
   
   ## Checklist ##
   ### Essentials ###
   Please feel free to remove inapplicable items for your PR.
   - [ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to 
the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) 
created (except PRs with tiny changes)
   - [ ] Changes are complete (i.e. I finished coding on this PR)
   - [ ] All changes have test coverage:
   - Unit tests are added for small changes to verify correctness (e.g. adding 
a new operator)
   - Nightly tests are added for complicated/long-running ones (e.g. changing 
distributed kvstore)
   - Build tests will be added for build configuration changes (e.g. adding a 
new build option with NCCL)
   - [ ] Code is well-documented: 
   - For user-facing API changes, API doc string has been updated. 
   - For new C++ functions in header files, their functionalities and arguments 
are documented. 
   - For new examples, README.md is added to explain the what the example does, 
the source of the dataset, expected performance on test set and reference to 
the original paper if applicable
   - Check the API doc at 
http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
   - [ ] To the my best knowledge, examples are either not affected by this 
change, or have been fixed to be compatible with this change
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] azai91 commented on issue #9634: [Feature request] Warning message or exception on mxnet.metric.Loss.update interface for performance

[GitBox]

azai91 commented on issue #9634: [Feature request] Warning message or exception 
on mxnet.metric.Loss.update interface for performance
URL: 
https://github.com/apache/incubator-mxnet/issues/9634#issuecomment-388191453
 
 
   This PR should address this issue #10892 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on issue #10888: Fix thread local.

[GitBox]

zheng-da commented on issue #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#issuecomment-388189951
 
 
   I think we need to change dmlc and only use MX_THREAD_LOCAL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on issue #10888: Fix thread local.

[GitBox]

zheng-da commented on issue #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#issuecomment-388189836
 
 
   about thread_local?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudh2290 commented on a change in pull request #10892: [MXNET-412] Loss update performance

[GitBox]

anirudh2290 commented on a change in pull request #10892: [MXNET-412] Loss 
update performance
URL: https://github.com/apache/incubator-mxnet/pull/10892#discussion_r187463611
 
 

 ##
 File path: python/mxnet/metric.py
 ##
 @@ -421,8 +421,6 @@ def update(self, labels, preds):
 label = label.flat
 pred_label = pred_label.flat
 
-labels, preds = check_label_shapes(label, pred_label)
 
 Review comment:
   Why removing check_label_shapes ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] azai91 opened a new pull request #10892: [MXNET-412] Loss update performance

[GitBox]

azai91 opened a new pull request #10892: [MXNET-412] Loss update performance
URL: https://github.com/apache/incubator-mxnet/pull/10892
 
 
   ## Description ##
   (Brief description on what this PR is about)
   
   ## Checklist ##
   ### Essentials ###
   Please feel free to remove inapplicable items for your PR.
   - [ X] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to 
the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) 
created (except PRs with tiny changes)
   - [ X] Changes are complete (i.e. I finished coding on this PR)
   - [ X] All changes have test coverage:
   - Unit tests are added for small changes to verify correctness (e.g. adding 
a new operator)
   - Nightly tests are added for complicated/long-running ones (e.g. changing 
distributed kvstore)
   - Build tests will be added for build configuration changes (e.g. adding a 
new build option with NCCL)
   - [ ] Code is well-documented: 
   - For user-facing API changes, API doc string has been updated. 
   - For new C++ functions in header files, their functionalities and arguments 
are documented. 
   - For new examples, README.md is added to explain the what the example does, 
the source of the dataset, expected performance on test set and reference to 
the original paper if applicable
   - Check the API doc at 
http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
   - [X ] To the my best knowledge, examples are either not affected by this 
change, or have been fixed to be compatible with this change
   
   ### Changes ###
   - [ ] Feature1, tests, (and when applicable, API doc)
   - [ ] Feature2, tests, (and when applicable, API doc)
   
   ## Comments ##
   - If this change is a backward incompatible change, why must this change be 
made.
   - Interesting edge cases to note here
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] spidyDev commented on issue #10886: Added nullptr check for LeakyRelu gamma parameter.

[GitBox]

spidyDev commented on issue #10886: Added nullptr check for LeakyRelu gamma 
parameter.
URL: https://github.com/apache/incubator-mxnet/pull/10886#issuecomment-388186554
 
 
   @szha  @marcoabreu  Please review this PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187460561
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
+   })
+.set_attr("FInferType",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector* in_attrs,
+  std::vector* out_attrs) {
+   CHECK_EQ(in_attrs->size(), 1U);
+   CHECK_EQ(out_attrs->size(), 1U);
+   TYPE_ASSIGN_CHECK(*out_attrs, 0, 0U);
+   return out_attrs->at(0) != -1;
+   })
+.set_attr("FGradient", MakeZeroGradNodes)
 
 Review comment:
   Right, I'm asking the same thing. Why did you define FGradient attribute for 
the op?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] szha commented on issue #10861: split trainer.step into allreduce and update

[GitBox]

szha commented on issue #10861: split trainer.step into allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#issuecomment-388185983
 
 
   OK


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] szha commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

szha commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187460140
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
 
 Review comment:
   it's possible that users can call trainer.update directly without calling 
allreduce.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] lanking520 commented on issue #10858: make docs on Mac - scala docs failure

[GitBox]

lanking520 commented on issue #10858: make docs on Mac - scala docs failure
URL: 
https://github.com/apache/incubator-mxnet/issues/10858#issuecomment-388181872
 
 
   Is adding platform check in mxdoc.py could be a solution to this for short 
term? I think we really need a version control for dependencies of docs, update 
them on a timely manner.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] DickJC123 opened a new pull request #10891: [MXNET-9845] Fix flakeyness of test_operator.py:test_reduce.

[GitBox]

DickJC123 opened a new pull request #10891: [MXNET-9845] Fix flakeyness of 
test_operator.py:test_reduce.
URL: https://github.com/apache/incubator-mxnet/pull/10891
 
 
   ## Description ##
   This PR fixes issue https://github.com/apache/incubator-mxnet/issues/9845 by 
casting the numpy inputs to float32 for the min and max usages within this test.
   
   The problem is a recurring one for our CI:  MXNet calculates an operator 
output in float32, but the golden copy is calculated using numpy in float64.  
Normally for functions with a continuous derivative, this is not a problem.  
However, take a function like 'max' with two float64 input values A and B.  The 
maximum might be B, but if the values are first rounded to float32, they might 
then become equal at which point the maximum might be arbitrarily picked as 
either A or B.  This creates a problem when predicting the back-prop function, 
where MXNet's float32 back-prop of the max function may direct the gradient to 
the 'wrong' input compared to the float64 max function of numpy.
   
   I successfully ran:
   
   MXNET_TEST_COUNT=1 nosetests --verbose -s 
tests/python/unittest/test_operator.py:test_reduce
   
   I also made the indentation of test_reduce consistently 4 spaces, where I 
found it to be 2.
   ## Checklist ##
   ### Essentials ###
   Please feel free to remove inapplicable items for your PR.
   - [X ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to 
the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) 
created (except PRs with tiny changes)
   - [X ] Changes are complete (i.e. I finished coding on this PR)
   - [X ] All changes have test coverage:
   - Unit tests are added for small changes to verify correctness (e.g. adding 
a new operator)
   - Nightly tests are added for complicated/long-running ones (e.g. changing 
distributed kvstore)
   - Build tests will be added for build configuration changes (e.g. adding a 
new build option with NCCL)
   - [X ] Code is well-documented: 
   - For user-facing API changes, API doc string has been updated. 
   - For new C++ functions in header files, their functionalities and arguments 
are documented. 
   - For new examples, README.md is added to explain the what the example does, 
the source of the dataset, expected performance on test set and reference to 
the original paper if applicable
   - Check the API doc at 
http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
   - [ X] To the my best knowledge, examples are either not affected by this 
change, or have been fixed to be compatible with this change
   
   ### Changes ###
   - [X ] Feature1, tests, (and when applicable, API doc)
   - [ ] Feature2, tests, (and when applicable, API doc)
   
   ## Comments ##
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] haojin2 commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

haojin2 commented on a change in pull request #10889: [MXNET-382] Shape Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187454313
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
 
 Review comment:
   Would it be clearer if you add an example here?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187454291
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
+   })
+.set_attr("FInferType",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector* in_attrs,
+  std::vector* out_attrs) {
+   CHECK_EQ(in_attrs->size(), 1U);
+   CHECK_EQ(out_attrs->size(), 1U);
+   TYPE_ASSIGN_CHECK(*out_attrs, 0, 0U);
+   return out_attrs->at(0) != -1;
+   })
+.set_attr("FGradient", MakeZeroGradNodes)
 
 Review comment:
   no, shape and size operator will not have gradients right?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] eric-haibin-lin commented on issue #10888: Fix thread local.

[GitBox]

eric-haibin-lin commented on issue #10888: Fix thread local.
URL: https://github.com/apache/incubator-mxnet/pull/10888#issuecomment-388178939
 
 
   Could you add some explanation/doc so others won't be making the same 
mistake?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] aaronmarkham commented on issue #10858: make docs on Mac - scala docs failure

[GitBox]

aaronmarkham commented on issue #10858: make docs on Mac - scala docs failure
URL: 
https://github.com/apache/incubator-mxnet/issues/10858#issuecomment-388176530
 
 
   I compared versions.
   
   mac 
   ==
   maven 3.5.3
   java 1.8.0_171
   scala 2.12.6
   
   ubuntu
   =
   maven 3.3.9
   java 1.8.0_162
   scala 2.11.6
   sbt 1.1.5
   
   I believe the fix for this - to get Mac and Ubuntu on the same version 
scala, and this will fix the Mac docs build, by getting everything to work with 
the latest version of scala:
   
   1. Remove index and package.html from mxdoc.py line 89
   2. For CI and general Ubuntu deps, run the following:
   ```
   sudo apt-get remove scala-library scala
   sudo wget http://scala-lang.org/files/archive/scala-2.12.6.deb
   sudo dpkg -i scala-2.12.6.deb
   ```
   
   You go from 2000 and 
[late](https://mxnet.incubator.apache.org/api/scala/docs/index.html#org.apache.mxnet.Accuracy
   ).
   To 2000 and 
[next](http://54.172.99.8/api/scala/docs/org/apache/mxnet/Accuracy.html
   ).


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] haojin2 commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

haojin2 commented on a change in pull request #10889: [MXNET-382] Shape Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187452104
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op.h
 ##
 @@ -388,6 +388,26 @@ void CastCompute(const nnvm::NodeAttrs& attrs,
   });
 }
 
+template
+void ShapeCompute(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector& inputs,
+ const std::vector& req,
+ const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  CHECK_EQ(req.size(), 1U);
+  const TBlob& in_data = inputs[0];
+  const TBlob& out_data = outputs[0];
+  mshadow::Stream *s = ctx.get_stream();
+  TShape in_shape = in_data.shape_;
+  MSHADOW_TYPE_SWITCH(out_data.type_flag_, DType, {
+mxnet_op::Kernel::Launch(
+ s, in_data.ndim(), out_data.dptr(), in_shape.data());
+  });
+}
+
+
 
 Review comment:
   Please get rid of one blank line here, c++ use only 1 blank line between 
functions


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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

[incubator-mxnet] branch master updated: fix docs for new broadcast operators (#10887)

[haibin]

This is an automated email from the ASF dual-hosted git repository.

haibin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-mxnet.git


The following commit(s) were added to refs/heads/master by this push:
 new 35fb9ea  fix docs for new broadcast operators (#10887)
35fb9ea is described below

commit 35fb9ea1093bd0006a396b5e493e5949b66ca33e
Author: Hao Jin 
AuthorDate: Thu May 10 13:35:45 2018 -0700

fix docs for new broadcast operators (#10887)
---
 docs/api/python/ndarray/sparse.md |  2 ++
 docs/api/python/symbol/sparse.md  |  4 
 src/operator/tensor/elemwise_binary_broadcast_op_basic.cc | 10 ++
 3 files changed, 16 insertions(+)

diff --git a/docs/api/python/ndarray/sparse.md 
b/docs/api/python/ndarray/sparse.md
index 1f67e82..28fb386 100644
--- a/docs/api/python/ndarray/sparse.md
+++ b/docs/api/python/ndarray/sparse.md
@@ -386,6 +386,8 @@ We summarize the interface for each class in the following 
sections.
 elemwise_add
 elemwise_sub
 elemwise_mul
+broadcast_add
+broadcast_sub
 broadcast_mul
 broadcast_div
 negative
diff --git a/docs/api/python/symbol/sparse.md b/docs/api/python/symbol/sparse.md
index a44ff15..c4c8e92 100644
--- a/docs/api/python/symbol/sparse.md
+++ b/docs/api/python/symbol/sparse.md
@@ -97,6 +97,10 @@ In the rest of this document, we list sparse related 
routines provided by the
 elemwise_add
 elemwise_sub
 elemwise_mul
+broadcast_add
+broadcast_sub
+broadcast_mul
+broadcast_div
 negative
 dot
 add_n
diff --git a/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc 
b/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
index 78b2d45..61bc94e 100644
--- a/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
+++ b/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
@@ -29,6 +29,8 @@
 namespace mxnet {
 namespace op {
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_add)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_add)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_plus)
 .add_alias("broadcast_plus")
 .describe(R"code(Returns element-wise sum of the input arrays with 
broadcasting.
 
@@ -49,6 +51,7 @@ Example::
[ 2.,  2.,  2.]]
 
 Supported sparse operations:
+
broadcast_add(csr, dense(1D)) = dense
broadcast_add(dense(1D), csr) = dense
 
@@ -74,6 +77,8 @@ NNVM_REGISTER_OP(_backward_broadcast_add)
 
mshadow_op::identity>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_sub)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_sub)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_minus)
 .add_alias("broadcast_minus")
 .describe(R"code(Returns element-wise difference of the input arrays with 
broadcasting.
 
@@ -94,6 +99,7 @@ Example::
 [ 0.,  0.,  0.]]
 
 Supported sparse operations:
+
broadcast_sub/minus(csr, dense(1D)) = dense
broadcast_sub/minus(dense(1D), csr) = dense
 
@@ -119,6 +125,7 @@ NNVM_REGISTER_OP(_backward_broadcast_sub)
 
mshadow_op::negation>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_mul)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_mul)
 .describe(R"code(Returns element-wise product of the input arrays with 
broadcasting.
 
 Example::
@@ -133,6 +140,7 @@ Example::
   [ 1.,  1.,  1.]]
 
 Supported sparse operations:
+
broadcast_mul(csr, dense(1D)) = csr (CPU only)
 
 )code" ADD_FILELINE)
@@ -158,6 +166,7 @@ NNVM_REGISTER_OP(_backward_broadcast_mul)
   
mshadow_op::left>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_div)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_div)
 .describe(R"code(Returns element-wise division of the input arrays with 
broadcasting.
 
 Example::
@@ -172,6 +181,7 @@ Example::
   [ 2.,  2.,  2.]]
 
 Supported sparse operations:
+
broadcast_div(csr, dense(1D)) = csr (CPU only)
 
 )code" ADD_FILELINE)

-- 
To stop receiving notification emails like this one, please contact
hai...@apache.org.

[GitHub] eric-haibin-lin closed pull request #10887: [MXNET-364] Fix docs for sparse broadcast operators

[GitBox]

eric-haibin-lin closed pull request #10887: [MXNET-364] Fix docs for sparse 
broadcast operators
URL: https://github.com/apache/incubator-mxnet/pull/10887
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/docs/api/python/ndarray/sparse.md 
b/docs/api/python/ndarray/sparse.md
index 1f67e82194b..28fb3869650 100644
--- a/docs/api/python/ndarray/sparse.md
+++ b/docs/api/python/ndarray/sparse.md
@@ -386,6 +386,8 @@ We summarize the interface for each class in the following 
sections.
 elemwise_add
 elemwise_sub
 elemwise_mul
+broadcast_add
+broadcast_sub
 broadcast_mul
 broadcast_div
 negative
diff --git a/docs/api/python/symbol/sparse.md b/docs/api/python/symbol/sparse.md
index a44ff150356..c4c8e92c1f6 100644
--- a/docs/api/python/symbol/sparse.md
+++ b/docs/api/python/symbol/sparse.md
@@ -97,6 +97,10 @@ In the rest of this document, we list sparse related 
routines provided by the
 elemwise_add
 elemwise_sub
 elemwise_mul
+broadcast_add
+broadcast_sub
+broadcast_mul
+broadcast_div
 negative
 dot
 add_n
diff --git a/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc 
b/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
index 78b2d45567d..61bc94e4df1 100644
--- a/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
+++ b/src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
@@ -29,6 +29,8 @@
 namespace mxnet {
 namespace op {
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_add)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_add)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_plus)
 .add_alias("broadcast_plus")
 .describe(R"code(Returns element-wise sum of the input arrays with 
broadcasting.
 
@@ -49,6 +51,7 @@ Example::
[ 2.,  2.,  2.]]
 
 Supported sparse operations:
+
broadcast_add(csr, dense(1D)) = dense
broadcast_add(dense(1D), csr) = dense
 
@@ -74,6 +77,8 @@ NNVM_REGISTER_OP(_backward_broadcast_add)
 
mshadow_op::identity>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_sub)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_sub)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_minus)
 .add_alias("broadcast_minus")
 .describe(R"code(Returns element-wise difference of the input arrays with 
broadcasting.
 
@@ -94,6 +99,7 @@ Example::
 [ 0.,  0.,  0.]]
 
 Supported sparse operations:
+
broadcast_sub/minus(csr, dense(1D)) = dense
broadcast_sub/minus(dense(1D), csr) = dense
 
@@ -119,6 +125,7 @@ NNVM_REGISTER_OP(_backward_broadcast_sub)
 
mshadow_op::negation>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_mul)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_mul)
 .describe(R"code(Returns element-wise product of the input arrays with 
broadcasting.
 
 Example::
@@ -133,6 +140,7 @@ Example::
   [ 1.,  1.,  1.]]
 
 Supported sparse operations:
+
broadcast_mul(csr, dense(1D)) = csr (CPU only)
 
 )code" ADD_FILELINE)
@@ -158,6 +166,7 @@ NNVM_REGISTER_OP(_backward_broadcast_mul)
   
mshadow_op::left>);
 
 MXNET_OPERATOR_REGISTER_BINARY_BROADCAST(broadcast_div)
+MXNET_ADD_SPARSE_OP_ALIAS(broadcast_div)
 .describe(R"code(Returns element-wise division of the input arrays with 
broadcasting.
 
 Example::
@@ -172,6 +181,7 @@ Example::
   [ 2.,  2.,  2.]]
 
 Supported sparse operations:
+
broadcast_div(csr, dense(1D)) = csr (CPU only)
 
 )code" ADD_FILELINE)


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] eric-haibin-lin commented on a change in pull request #10887: [MXNET-364] Fix docs for sparse broadcast operators

[GitBox]

eric-haibin-lin commented on a change in pull request #10887: [MXNET-364] Fix 
docs for sparse broadcast operators
URL: https://github.com/apache/incubator-mxnet/pull/10887#discussion_r187451896
 
 

 ##
 File path: src/operator/tensor/elemwise_binary_broadcast_op_basic.cc
 ##
 @@ -172,6 +181,7 @@ Example::
   [ 2.,  2.,  2.]]
 
 Supported sparse operations:
+
broadcast_div(csr, dense(1D)) = csr (CPU only)
 
 Review comment:
   Can this be enabled on gpu easily? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on issue #10889: [MXNET-382] Shape Operator

[GitBox]

zheng-da commented on issue #10889: [MXNET-382] Shape Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#issuecomment-388176799
 
 
   the shape operator doesn't have backward. my biggest concern is that any 
computation that uses this operator can't perform a backward computation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] aaronmarkham commented on issue #10858: make docs on Mac - scala docs failure

[GitBox]

aaronmarkham commented on issue #10858: make docs on Mac - scala docs failure
URL: 
https://github.com/apache/incubator-mxnet/issues/10858#issuecomment-388176530
 
 
   I compared versions.
   
   mac 
   ==
   maven 3.5.3
   java 1.8.0_171
   scala 2.12.6
   
   ubuntu
   =
   maven 3.3.9
   java 1.8.0_162
   scala 2.11.6
   sbt 1.1.5
   
   I believe the fix for this - to get Mac and Ubuntu on the same version 
scala, and this will fix the Mac docs build, by getting everything to work with 
the latest version of scala:
   
   1. Remove index and package.html from mxdoc.py line 89
   2. For CI and general Ubuntu deps, run the following:
   ```
   sudo apt-get remove scala-library scala
   sudo wget http://scala-lang.org/files/archive/scala-2.12.6.deb
   sudo dpkg -i scala-2.12.6.deb
   ```
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187448444
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op.h
 ##
 @@ -388,6 +388,26 @@ void CastCompute(const nnvm::NodeAttrs& attrs,
   });
 }
 
+template
+void ShapeCompute(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector& inputs,
+ const std::vector& req,
+ const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  CHECK_EQ(req.size(), 1U);
+  const TBlob& in_data = inputs[0];
+  const TBlob& out_data = outputs[0];
+  mshadow::Stream *s = ctx.get_stream();
+  TShape in_shape = in_data.shape_;
+  MSHADOW_TYPE_SWITCH(out_data.type_flag_, DType, {
+mxnet_op::Kernel::Launch(
+ s, in_data.ndim(), out_data.dptr(), in_shape.data());
 
 Review comment:
   yes, i will make this change.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187447176
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
+   })
+.set_attr("FInferType",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector* in_attrs,
+  std::vector* out_attrs) {
+   CHECK_EQ(in_attrs->size(), 1U);
+   CHECK_EQ(out_attrs->size(), 1U);
+   TYPE_ASSIGN_CHECK(*out_attrs, 0, 0U);
 
 Review comment:
   okay, i will use mshadow::kInt64


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on issue #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on issue #10861: split trainer.step into allreduce and 
update
URL: https://github.com/apache/incubator-mxnet/pull/10861#issuecomment-388171910
 
 
   I think you should `assert not self._update_on_kvstore` in allreduce and 
update. Then do the real work in _allreduce and _update


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187444762
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
+"""
+self._allreduce(batch_size, skip_pull=False)
+
+def _allreduce(self, batch_size, skip_pull):
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._optimizer.rescale_grad = self._scale / batch_size
+
+for i, param in enumerate(self._params):
+if param.grad_req == 'null':
+continue
+
+if self._kvstore:
+self._kvstore.push(i, param.list_grad(), priority=-i)
+if not skip_pull:
+if not self._update_on_kvstore:
 
 Review comment:
   do this out side of for loop


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187444773
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
+"""
+self._allreduce(batch_size, skip_pull=False)
+
+def _allreduce(self, batch_size, skip_pull):
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._optimizer.rescale_grad = self._scale / batch_size
+
+for i, param in enumerate(self._params):
+if param.grad_req == 'null':
+continue
+
+if self._kvstore:
+self._kvstore.push(i, param.list_grad(), priority=-i)
+if not skip_pull:
 
 Review comment:
   do this out side of for loop


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187445689
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -187,13 +240,9 @@ def step(self, batch_size, ignore_stale_grad=False):
 "warning and skip updating of Parameters with 
stale gradient" \
 %(param.name, str(data.context)))
 
-if self._kvstore:
-self._kvstore.push(i, param.list_grad(), priority=-i)
-if self._update_on_kvstore:
-self._kvstore.pull(i, param.list_data(), priority=-i)
-continue
-else:
-self._kvstore.pull(i, param.list_grad(), priority=-i)
+if self._kvstore and self._update_on_kvstore:
 
 Review comment:
   A previous push is not guaranteed. I don't think it makes sense to allow 
users call allreduce and update separately with update_on_kvstore


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187443105
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
 
 Review comment:
   already done in allreduece


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187444700
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
+"""
+self._allreduce(batch_size, skip_pull=False)
+
+def _allreduce(self, batch_size, skip_pull):
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._optimizer.rescale_grad = self._scale / batch_size
+
+for i, param in enumerate(self._params):
+if param.grad_req == 'null':
+continue
+
+if self._kvstore:
 
 Review comment:
   do this outside of for loop


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187442772
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -49,6 +51,9 @@ class Trainer(object):
 on the type of compression being used. For example, 2bit compression 
requires a threshold.
 Arguments would then be {'type':'2bit', 'threshold':0.5}
 See mxnet.KVStore.set_gradient_compression method for more details on 
gradient compression.
+force_local_update : bool, default False
 
 Review comment:
   update_on_kvstore=None


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on a change in pull request #10861: split trainer.step into allreduce and update

[GitBox]

piiswrong commented on a change in pull request #10861: split trainer.step into 
allreduce and update
URL: https://github.com/apache/incubator-mxnet/pull/10861#discussion_r187443221
 
 

 ##
 File path: python/mxnet/gluon/trainer.py
 ##
 @@ -153,11 +161,55 @@ def set_learning_rate(self, lr):
 else:
 self._optimizer.set_learning_rate(lr)
 
-
 def step(self, batch_size, ignore_stale_grad=False):
 """Makes one step of parameter update. Should be called after
 `autograd.compute_gradient` and outside of `record()` scope.
 
+Parameters
+--
+batch_size : int
+Batch size of data processed. Gradient will be normalized by 
`1/batch_size`.
+Set this to 1 if you normalized loss manually with `loss = 
mean(loss)`.
+ignore_stale_grad : bool, optional, default=False
+If true, ignores Parameters with stale gradient (gradient that has 
not
+been updated by `backward` after last step) and skip update.
+"""
+if not self._kv_initialized:
+self._init_kvstore()
+
+self._allreduce(batch_size, skip_pull=self._update_on_kvstore)
+self.update(batch_size, ignore_stale_grad)
+
+def allreduce(self, batch_size):
+"""For each parameter, reduce the gradients from different contexts. 
Should be called after
+`autograd.compute_gradient`, outside of `record()` scope, and before 
`trainer.update`.
 
 Review comment:
   what's compute_gradient?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r18796
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
 
 Review comment:
   you suggest that I define a shape_is_none function and use it here?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187444326
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
 
 Review comment:
   will remove it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong closed pull request #10857: Fixed divison by zero bug in DistanceWeightedSampling in gluon example

[GitBox]

piiswrong closed pull request #10857: Fixed divison by zero bug in 
DistanceWeightedSampling in gluon example
URL: https://github.com/apache/incubator-mxnet/pull/10857
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/example/gluon/embedding_learning/model.py 
b/example/gluon/embedding_learning/model.py
index 0f041bc1fc4..91f7735497d 100644
--- a/example/gluon/embedding_learning/model.py
+++ b/example/gluon/embedding_learning/model.py
@@ -110,7 +110,8 @@ def hybrid_forward(self, F, x):
 mask[i:i+k, i:i+k] = 0
 
 weights = weights * F.array(mask) * (distance < 
self.nonzero_loss_cutoff)
-weights = weights / F.sum(weights, axis=1, keepdims=True)
+weights_sum = F.sum(weights, axis=1, keepdims=True)
+weights = weights / weights_sum
 
 a_indices = []
 p_indices = []
@@ -120,9 +121,10 @@ def hybrid_forward(self, F, x):
 for i in range(n):
 block_idx = i // k
 
-try:
+if weights_sum[i] != 0:
 n_indices += np.random.choice(n, k-1, p=np_weights[i]).tolist()
-except:
+else:
+# all samples are above the cutoff so we sample uniformly
 n_indices += np.random.choice(n, k-1).tolist()
 for j in range(block_idx * k, (block_idx + 1) * k):
 if j != i:
@@ -217,8 +219,11 @@ def hybrid_forward(self, F, anchors, positives, negatives, 
beta_in, a_indices=No
 pos_loss = F.maximum(d_ap - beta + self._margin, 0.0)
 neg_loss = F.maximum(beta - d_an + self._margin, 0.0)
 
-pair_cnt = float(F.sum((pos_loss > 0.0) + (neg_loss > 0.0)).asscalar())
-
-# Normalize based on the number of pairs.
-loss = (F.sum(pos_loss + neg_loss) + beta_reg_loss) / pair_cnt
+pair_cnt = F.sum((pos_loss > 0.0) + (neg_loss > 0.0))
+if pair_cnt == 0.0:
+# When poss_loss and neg_loss is zero then total loss is zero as 
well
+loss = F.sum(pos_loss + neg_loss)
+else:
+# Normalize based on the number of pairs.
+loss = (F.sum(pos_loss + neg_loss) + beta_reg_loss) / pair_cnt
 return gluon.loss._apply_weighting(F, loss, self._weight, None)


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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

[incubator-mxnet] branch master updated: Fixed divison by zero bug in DistanceWeightedSampling in gluon example (#10857)

[jxie]

This is an automated email from the ASF dual-hosted git repository.

jxie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-mxnet.git


The following commit(s) were added to refs/heads/master by this push:
 new 26c30af  Fixed divison by zero bug in DistanceWeightedSampling in 
gluon example (#10857)
26c30af is described below

commit 26c30affd0ecf77b2628a1b8ccbaccea7ce5245e
Author: Istvan Fehervari 
AuthorDate: Thu May 10 13:01:14 2018 -0700

Fixed divison by zero bug in DistanceWeightedSampling in gluon example 
(#10857)

* Fixed divison by zero bug in DistanceWeightedSampling in gluon example

Sample selection for training is based on a vector of computed probablities 
that come from distance weights. In the current implementation these weights 
can become zero when distance to all other neightbors is zero.
Zero weights lead to nan bugs in the model, this commit is supposed to fix 
it by changing those zero weights not being divided by zero any longer.

* Reworked how np.choice is invoked in DistanceWeightedSampling to avoid a 
try-except closure

* Added an early return to MarginLoss if loss becomes zero.

* Rewrote MargingLoss so it is hybridizable
---
 example/gluon/embedding_learning/model.py | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/example/gluon/embedding_learning/model.py 
b/example/gluon/embedding_learning/model.py
index 0f041bc..91f7735 100644
--- a/example/gluon/embedding_learning/model.py
+++ b/example/gluon/embedding_learning/model.py
@@ -110,7 +110,8 @@ class DistanceWeightedSampling(HybridBlock):
 mask[i:i+k, i:i+k] = 0
 
 weights = weights * F.array(mask) * (distance < 
self.nonzero_loss_cutoff)
-weights = weights / F.sum(weights, axis=1, keepdims=True)
+weights_sum = F.sum(weights, axis=1, keepdims=True)
+weights = weights / weights_sum
 
 a_indices = []
 p_indices = []
@@ -120,9 +121,10 @@ class DistanceWeightedSampling(HybridBlock):
 for i in range(n):
 block_idx = i // k
 
-try:
+if weights_sum[i] != 0:
 n_indices += np.random.choice(n, k-1, p=np_weights[i]).tolist()
-except:
+else:
+# all samples are above the cutoff so we sample uniformly
 n_indices += np.random.choice(n, k-1).tolist()
 for j in range(block_idx * k, (block_idx + 1) * k):
 if j != i:
@@ -217,8 +219,11 @@ class MarginLoss(gluon.loss.Loss):
 pos_loss = F.maximum(d_ap - beta + self._margin, 0.0)
 neg_loss = F.maximum(beta - d_an + self._margin, 0.0)
 
-pair_cnt = float(F.sum((pos_loss > 0.0) + (neg_loss > 0.0)).asscalar())
-
-# Normalize based on the number of pairs.
-loss = (F.sum(pos_loss + neg_loss) + beta_reg_loss) / pair_cnt
+pair_cnt = F.sum((pos_loss > 0.0) + (neg_loss > 0.0))
+if pair_cnt == 0.0:
+# When poss_loss and neg_loss is zero then total loss is zero as 
well
+loss = F.sum(pos_loss + neg_loss)
+else:
+# Normalize based on the number of pairs.
+loss = (F.sum(pos_loss + neg_loss) + beta_reg_loss) / pair_cnt
 return gluon.loss._apply_weighting(F, loss, self._weight, None)

-- 
To stop receiving notification emails like this one, please contact
j...@apache.org.

[GitHub] piiswrong commented on issue #10864: Support for axis parameter in linalg.gemm

[GitBox]

piiswrong commented on issue #10864: Support for axis parameter in linalg.gemm
URL: https://github.com/apache/incubator-mxnet/pull/10864#issuecomment-388165652
 
 
   This this behavior for axis common for other frameworks?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] piiswrong commented on issue #10864: Support for axis parameter in linalg.gemm

[GitBox]

piiswrong commented on issue #10864: Support for axis parameter in linalg.gemm
URL: https://github.com/apache/incubator-mxnet/pull/10864#issuecomment-388165517
 
 
   please rebase and try again


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zhreshold commented on issue #10875: How to debug hybridize() failures?

[GitBox]

zhreshold commented on issue #10875: How to debug hybridize() failures?
URL: 
https://github.com/apache/incubator-mxnet/issues/10875#issuecomment-388160416
 
 
   @piiswrong Is this a bug or simply we cannot detect to use broadcast 
operators properly?
   @hetong007  had similar problems yesterday.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

anirudhacharya commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187434058
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cu
 ##
 @@ -77,6 +77,9 @@ NNVM_REGISTER_OP(_identity_with_attr_like_rhs)
 NNVM_REGISTER_OP(reshape_like)
 .set_attr("FCompute", UnaryOp::IdentityCompute);
 
+NNVM_REGISTER_OP(shape)
 
 Review comment:
   what do you suggest the name of the operator should be? This is more or less 
the name used in a couple of other frameworks too.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on issue #10789: Reshape of input array when the shape is available only at runtime is not possible

[GitBox]

zheng-da commented on issue #10789: Reshape of input array when the shape is 
available only at runtime is not possible 
URL: 
https://github.com/apache/incubator-mxnet/issues/10789#issuecomment-388158233
 
 
   can you show a small example that reshape like that is absolutely required 
and reshape_like can't work?
   my concern is that the shape operator doesn't have backward function and the 
code with the shape operator will fail to compute gradients.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] some-guy1 opened a new issue #10890: [R] Is it possible to train Mxnet symbols and a GLM pretrained model as loss?

[GitBox]

some-guy1 opened a new issue #10890: [R] Is it possible to train Mxnet symbols 
and a GLM pretrained model as loss?
URL: https://github.com/apache/incubator-mxnet/issues/10890
 
 
   The idea is very similar to a GAN, but instead of two neural networks I want 
to use a neural network generator and a prebuilt GLM as the discriminator.
   
   Is this possible using the Mxnet symbols?
   
   Example pseudo-code:
   ```
   data <- mx.symbol.Variable('data')
   label <- mx.symbol.Variable('label')
   dconv <- mx.symbol.Deconvolution(data= data , kernel = c(3,3), 
stride=c(3,3), num_filter = 5)
   norm = mx.symbol.BatchNorm(data= dconv, fix_gamma=FALSE)
   act1 <- mx.symbol.LeakyReLU(data= norm, act_type = "leaky", name="act_3")
   
   #GLM that was already built and will not change
   class = abs(predict.glm(act1) - label)
   NN_Model <- mx.symbol.MakeLoss(class )
   ```
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187423582
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
+   })
+.set_attr("FInferType",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector* in_attrs,
+  std::vector* out_attrs) {
+   CHECK_EQ(in_attrs->size(), 1U);
+   CHECK_EQ(out_attrs->size(), 1U);
+   TYPE_ASSIGN_CHECK(*out_attrs, 0, 0U);
+   return out_attrs->at(0) != -1;
+   })
+.set_attr("FGradient", MakeZeroGradNodes)
 
 Review comment:
   Shouldn't there be no appropriate gradient definition for this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187424772
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
 
 Review comment:
   Why add this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187425800
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
+   })
+.set_attr("FInferType",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector* in_attrs,
+  std::vector* out_attrs) {
+   CHECK_EQ(in_attrs->size(), 1U);
+   CHECK_EQ(out_attrs->size(), 1U);
+   TYPE_ASSIGN_CHECK(*out_attrs, 0, 0U);
 
 Review comment:
   Use type enum variable name.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187426551
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cu
 ##
 @@ -77,6 +77,9 @@ NNVM_REGISTER_OP(_identity_with_attr_like_rhs)
 NNVM_REGISTER_OP(reshape_like)
 .set_attr("FCompute", UnaryOp::IdentityCompute);
 
+NNVM_REGISTER_OP(shape)
 
 Review comment:
   Where does the name come from? This looks confusing and conflicts with the 
property name `shape` of `NDArray` in Python. Need to ensure the documentation 
can be rendered correctly.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187425447
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -399,6 +399,37 @@ NNVM_REGISTER_OP(reshape_like)
 .add_argument("lhs", "NDArray-or-Symbol", "First input.")
 .add_argument("rhs", "NDArray-or-Symbol", "Second input.");
 
+NNVM_REGISTER_OP(shape)
+.describe("Returns a 1D int64 array containing the shape of data.")
+.set_num_inputs(1)
+.set_num_outputs(1)
+.set_attr("FInplaceIdentity",
+[](const NodeAttrs& attrs){ return std::vector{true}; })
+.set_attr("FIgnoreInputs",
+[](const NodeAttrs& attrs) { return std::vector(1, 1); })
+.set_attr("FCompute", ShapeCompute)
+.set_attr("FInferShape",
+   [](const nnvm::NodeAttrs& attrs,
+  std::vector *in_attrs,
+  std::vector *out_attrs) {
+CHECK_EQ(in_attrs->size(), 1U);
+CHECK_EQ(out_attrs->size(), 1U);
+TShape target_shape(1);
+target_shape[0] = in_attrs->at(0).ndim();
+SHAPE_ASSIGN_CHECK(*out_attrs, 0, target_shape);
+return out_attrs->at(0).ndim() != 0U && out_attrs->at(0).Size() != 
0U;
 
 Review comment:
   use `shape_is_none(out_attrs->at(0)`


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187423999
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op.h
 ##
 @@ -388,6 +388,26 @@ void CastCompute(const nnvm::NodeAttrs& attrs,
   });
 }
 
+template
+void ShapeCompute(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector& inputs,
+ const std::vector& req,
+ const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  CHECK_EQ(req.size(), 1U);
+  const TBlob& in_data = inputs[0];
+  const TBlob& out_data = outputs[0];
+  mshadow::Stream *s = ctx.get_stream();
+  TShape in_shape = in_data.shape_;
+  MSHADOW_TYPE_SWITCH(out_data.type_flag_, DType, {
+mxnet_op::Kernel::Launch(
+ s, in_data.ndim(), out_data.dptr(), in_shape.data());
 
 Review comment:
   Why `DType` for `out_data`? Isn't that `int64` as in the description?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187423678
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op.h
 ##
 @@ -388,6 +388,26 @@ void CastCompute(const nnvm::NodeAttrs& attrs,
   });
 }
 
+template
+void ShapeCompute(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
 
 Review comment:
   alignment.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10889: [MXNET-382] Shape Operator

[GitBox]

reminisce commented on a change in pull request #10889: [MXNET-382] Shape 
Operator
URL: https://github.com/apache/incubator-mxnet/pull/10889#discussion_r187424176
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op.h
 ##
 @@ -388,6 +388,26 @@ void CastCompute(const nnvm::NodeAttrs& attrs,
   });
 }
 
+template
+void ShapeCompute(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector& inputs,
+ const std::vector& req,
+ const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  CHECK_EQ(req.size(), 1U);
+  const TBlob& in_data = inputs[0];
+  const TBlob& out_data = outputs[0];
+  mshadow::Stream *s = ctx.get_stream();
+  TShape in_shape = in_data.shape_;
 
 Review comment:
   const TShape&.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ZiyueHuang commented on issue #10882: move exec.reshape to backend

[GitBox]

ZiyueHuang commented on issue #10882: move exec.reshape to backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#issuecomment-388150995
 
 
   Thanks for your comments.
   For the unit tests part, I found the existing tests cover both up sizing and 
down sizing, 
https://github.com/apache/incubator-mxnet/blob/master/tests/python/unittest/test_module.py#L703,
 
https://github.com/apache/incubator-mxnet/blob/master/tests/python/unittest/test_module.py#L205,
 
https://github.com/apache/incubator-mxnet/blob/master/tests/python/unittest/test_executor.py#L144.
   Will address other parts tomorrow.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ashokei commented on issue #10591: [MXNET-365] handle inplace in mkldnn FallBackCompute

[GitBox]

ashokei commented on issue #10591: [MXNET-365] handle inplace in mkldnn 
FallBackCompute
URL: https://github.com/apache/incubator-mxnet/pull/10591#issuecomment-388150383
 
 
   @zheng-da updated PR to use testname, @marcoabreu can you merge now, thanks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zheng-da commented on a change in pull request #10876: Update performance page.

[GitBox]

zheng-da commented on a change in pull request #10876: Update performance page.
URL: https://github.com/apache/incubator-mxnet/pull/10876#discussion_r187424551
 
 

 ##
 File path: docs/faq/perf.md
 ##
 @@ -117,37 +117,46 @@ and  
[MXNet-1.2.0.rc1](https://github.com/apache/incubator-mxnet/releases/downlo
 
 - K80 (single GPU)
 
-| Batch | Alexnet | VGG| Inception-BN | Inception-v3 | Resnet 50 | Resnet 
152 |
+| Batch | Alexnet | VGG 16| Inception-BN | Inception-v3 | Resnet 50 | 
Resnet 152 |
 
|---|-||--|--|---||
 | 1 | 243.93  | 43.59  | 68.62| 35.52| 67.41 | 23.65   
   |
 | 2 | 338.16  | 49.14  | 113.41   | 56.29| 93.35 | 33.88   
   |
 | 4 | 478.92  | 53.44  | 159.61   | 74.43| 119.18| 45.23   
   |
 | 8 | 683.52  | 70.50  | 190.49   | 86.23| 131.32| 50.54   
   |
 | 16| 1004.66 | 109.01 | 254.20   | 105.70   | 155.40| 62.55   
   |
 | 32| 1238.55 | 114.98 | 285.49   | 116.79   | 159.42| 64.99   
   |
+| 64 | 1346.72 | 123.56 | 308.73 | 122.21 | 167.58 | 70.21 |
+| 128 | 1416.91 | OOM | 320.98 | 123.11 | 171.55 | 71.85 |
+| 256 | 1462.97 | OOM | 329.16 | 127.53 | 153.01 | 57.23 |
 
 - M60
 
-| Batch | Alexnet | VGG| Inception-BN | Inception-v3 | Resnet 50 | Resnet 
152 |
+| Batch | Alexnet | VGG 16| Inception-BN | Inception-v3 | Resnet 50 | 
Resnet 152 |
 
|---|-||--|--|---||
 | 1 | 243.49  | 59.95  | 101.97   | 48.30| 95.46 | 39.29   
   |
 | 2 | 491.04  | 69.14  | 170.35   | 80.27| 142.61| 60.17   
   |
 | 4 | 711.54  | 78.94  | 257.89   | 123.09   | 182.36| 76.51   
   |
 | 8 | 1077.73 | 109.34 | 343.42   | 152.82   | 208.74| 87.27   
   |
 | 16| 1447.21 | 144.93 | 390.25   | 166.32   | 220.73| 92.41   
   |
 | 32| 1797.66 | 151.86 | 416.69   | 176.56   | 230.19| 97.03   
   |
+| 64 | 1779.38 | 150.18 | 427.51 | 183.47 | 239.12 | 101.59 |
+| 128 | 1787.36 | OOM | 439.04 | 185.29 | 243.31 | 103.39 |
+| 256 | 1899.10 | OOM | 450.22 | 183.42 | 242.36 | 100.98 |
 
 
 - V100
 
-| Batch | Alexnet | VGG| Inception-BN | Inception-v3 | Resnet 50 | Resnet 
152 |
+| Batch | Alexnet | VGG 16| Inception-BN | Inception-v3 | Resnet 50 | 
Resnet 152 |
 
|---|-||--|--|---||
-| 1 | 659.51  | 205.16 | 136.91   | 76.54| 162.15| 61.38   
   |
-| 2 | 1248.21 | 265.40 | 261.85   | 144.23   | 293.74| 116.30  
   |
-| 4 | 2122.41 | 333.97 | 477.22   | 270.03   | 479.14| 195.17  
   |
-| 8 | 3894.30 | 420.26 | 831.09   | 450.68   | 699.39| 294.19  
   |
-| 16| 5815.58 | 654.16 | 1332.26  | 658.97   | 947.45| 398.79  
   |
-| 32| 7906.09 | 708.43 | 1784.23  | 817.33   | 1076.81   | 451.82  
   |
+| 1 | 659.51  | 205.16 | 157.37 | 87.71 | 162.15| 61.38  |
 
 Review comment:
   this is generated by typora. it should be fine.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10882: move exec.reshape to backend

[GitBox]

reminisce commented on a change in pull request #10882: move exec.reshape to 
backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#discussion_r187421887
 
 

 ##
 File path: src/c_api/c_api_executor.cc
 ##
 @@ -510,6 +513,135 @@ int MXExecutorSimpleBind(SymbolHandle symbol_handle,
   API_END();
 }
 
+int MXExecutorReshapeEx(SymbolHandle symbol_handle,
+int partial_shaping,
+int allow_up_sizing,
+int dev_type,
+int dev_id,
+mx_uint num_map_keys,
+const char** map_keys,
+const int* map_dev_types,
+const int* map_dev_ids,
+const mx_uint num_provided_arg_shapes,
+const char** provided_arg_shape_names,
+const mx_uint* provided_arg_shape_data,
+const mx_uint* provided_arg_shape_idx,
+mx_uint len,
+NDArrayHandle *in_args,
+NDArrayHandle *arg_grad_store,
+mx_uint *grad_req_type,
+mx_uint aux_states_len,
+NDArrayHandle *aux_states,
+ExecutorHandle shared_exec,
+ExecutorHandle *out) {
+  API_BEGIN();
+  // create shape map for in_args and aux_states
+  std::unordered_map kwargs(num_provided_arg_shapes);
+  for (mx_uint i = 0; i < num_provided_arg_shapes; ++i) {
+auto p = kwargs.emplace(provided_arg_shape_names[i],
+TShape(provided_arg_shape_data+provided_arg_shape_idx[i],
+  provided_arg_shape_data+provided_arg_shape_idx[i+1]));
+CHECK(p.second) << "Duplicate shapes are provided for argument "
+  << provided_arg_shape_names[i] << " in reshape of executor";
+  }
+  nnvm::Symbol *symb = static_cast(symbol_handle);
+  // symbol to graph
+  nnvm::Graph g;
+  g.outputs = symb->outputs;
+  g.attrs["mxnet_version"] = 
std::make_shared(static_cast(MXNET_VERSION));
+  const nnvm::IndexedGraph& idx = g.indexed_graph();
+  std::vector arg_shapes(idx.input_nodes().size(), TShape());
+  mxnet::MatchArguments(idx, kwargs, _shapes, "InferShape");
+  try {
+g = mxnet::exec::InferShape(std::move(g), std::move(arg_shapes), 
"__shape__");
+  } catch (const mxnet::op::InferShapeError ) {
+throw dmlc::Error(err.msg);
+  }
+  CHECK_EQ(g.GetAttr("shape_num_unknown_nodes"), 0U);
+  const std::vector& shape_vec = 
g.GetAttr("shape");
+  NDArrayHandle *arg = in_args;
+  NDArrayHandle *grad = arg_grad_store;
+  NDArrayHandle *aux = aux_states;
+
+  for (uint32_t nid : idx.input_nodes()) {
+std::string name = idx[nid].source->attrs.name;
+const TShape& new_shape = shape_vec[idx.entry_id(nid, 0)];
+if (idx.mutable_input_nodes().count(nid) == 0) {
+  NDArray* arr = static_cast(*arg);
+  NDArray* darr = static_cast(*grad);
+  if (new_shape == arr->shape()) {
 
 Review comment:
   Please simplify this. There is no need to have a `do nothing` block.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10882: move exec.reshape to backend

[GitBox]

reminisce commented on a change in pull request #10882: move exec.reshape to 
backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#discussion_r187421519
 
 

 ##
 File path: src/c_api/c_api_executor.cc
 ##
 @@ -510,6 +513,135 @@ int MXExecutorSimpleBind(SymbolHandle symbol_handle,
   API_END();
 }
 
+int MXExecutorReshapeEx(SymbolHandle symbol_handle,
+int partial_shaping,
+int allow_up_sizing,
+int dev_type,
+int dev_id,
+mx_uint num_map_keys,
+const char** map_keys,
+const int* map_dev_types,
+const int* map_dev_ids,
+const mx_uint num_provided_arg_shapes,
+const char** provided_arg_shape_names,
+const mx_uint* provided_arg_shape_data,
+const mx_uint* provided_arg_shape_idx,
+mx_uint len,
+NDArrayHandle *in_args,
+NDArrayHandle *arg_grad_store,
+mx_uint *grad_req_type,
+mx_uint aux_states_len,
+NDArrayHandle *aux_states,
+ExecutorHandle shared_exec,
+ExecutorHandle *out) {
+  API_BEGIN();
+  // create shape map for in_args and aux_states
+  std::unordered_map kwargs(num_provided_arg_shapes);
+  for (mx_uint i = 0; i < num_provided_arg_shapes; ++i) {
+auto p = kwargs.emplace(provided_arg_shape_names[i],
+TShape(provided_arg_shape_data+provided_arg_shape_idx[i],
+  provided_arg_shape_data+provided_arg_shape_idx[i+1]));
+CHECK(p.second) << "Duplicate shapes are provided for argument "
+  << provided_arg_shape_names[i] << " in reshape of executor";
+  }
+  nnvm::Symbol *symb = static_cast(symbol_handle);
+  // symbol to graph
+  nnvm::Graph g;
+  g.outputs = symb->outputs;
+  g.attrs["mxnet_version"] = 
std::make_shared(static_cast(MXNET_VERSION));
+  const nnvm::IndexedGraph& idx = g.indexed_graph();
+  std::vector arg_shapes(idx.input_nodes().size(), TShape());
+  mxnet::MatchArguments(idx, kwargs, _shapes, "InferShape");
+  try {
+g = mxnet::exec::InferShape(std::move(g), std::move(arg_shapes), 
"__shape__");
+  } catch (const mxnet::op::InferShapeError ) {
+throw dmlc::Error(err.msg);
+  }
+  CHECK_EQ(g.GetAttr("shape_num_unknown_nodes"), 0U);
+  const std::vector& shape_vec = 
g.GetAttr("shape");
+  NDArrayHandle *arg = in_args;
+  NDArrayHandle *grad = arg_grad_store;
+  NDArrayHandle *aux = aux_states;
+
+  for (uint32_t nid : idx.input_nodes()) {
+std::string name = idx[nid].source->attrs.name;
+const TShape& new_shape = shape_vec[idx.entry_id(nid, 0)];
+if (idx.mutable_input_nodes().count(nid) == 0) {
 
 Review comment:
   Can you consolidate the implementation for args and aux? There is a big 
chunk of duplicate code.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10882: move exec.reshape to backend

[GitBox]

reminisce commented on a change in pull request #10882: move exec.reshape to 
backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#discussion_r187422560
 
 

 ##
 File path: src/c_api/c_api_executor.cc
 ##
 @@ -510,6 +513,135 @@ int MXExecutorSimpleBind(SymbolHandle symbol_handle,
   API_END();
 }
 
+int MXExecutorReshapeEx(SymbolHandle symbol_handle,
+int partial_shaping,
+int allow_up_sizing,
+int dev_type,
+int dev_id,
+mx_uint num_map_keys,
+const char** map_keys,
+const int* map_dev_types,
+const int* map_dev_ids,
+const mx_uint num_provided_arg_shapes,
+const char** provided_arg_shape_names,
+const mx_uint* provided_arg_shape_data,
+const mx_uint* provided_arg_shape_idx,
+mx_uint len,
+NDArrayHandle *in_args,
+NDArrayHandle *arg_grad_store,
+mx_uint *grad_req_type,
+mx_uint aux_states_len,
+NDArrayHandle *aux_states,
+ExecutorHandle shared_exec,
+ExecutorHandle *out) {
+  API_BEGIN();
+  // create shape map for in_args and aux_states
+  std::unordered_map kwargs(num_provided_arg_shapes);
+  for (mx_uint i = 0; i < num_provided_arg_shapes; ++i) {
+auto p = kwargs.emplace(provided_arg_shape_names[i],
+TShape(provided_arg_shape_data+provided_arg_shape_idx[i],
+  provided_arg_shape_data+provided_arg_shape_idx[i+1]));
+CHECK(p.second) << "Duplicate shapes are provided for argument "
+  << provided_arg_shape_names[i] << " in reshape of executor";
+  }
+  nnvm::Symbol *symb = static_cast(symbol_handle);
+  // symbol to graph
+  nnvm::Graph g;
+  g.outputs = symb->outputs;
+  g.attrs["mxnet_version"] = 
std::make_shared(static_cast(MXNET_VERSION));
+  const nnvm::IndexedGraph& idx = g.indexed_graph();
+  std::vector arg_shapes(idx.input_nodes().size(), TShape());
+  mxnet::MatchArguments(idx, kwargs, _shapes, "InferShape");
+  try {
+g = mxnet::exec::InferShape(std::move(g), std::move(arg_shapes), 
"__shape__");
+  } catch (const mxnet::op::InferShapeError ) {
+throw dmlc::Error(err.msg);
+  }
+  CHECK_EQ(g.GetAttr("shape_num_unknown_nodes"), 0U);
+  const std::vector& shape_vec = 
g.GetAttr("shape");
+  NDArrayHandle *arg = in_args;
+  NDArrayHandle *grad = arg_grad_store;
+  NDArrayHandle *aux = aux_states;
+
+  for (uint32_t nid : idx.input_nodes()) {
+std::string name = idx[nid].source->attrs.name;
+const TShape& new_shape = shape_vec[idx.entry_id(nid, 0)];
+if (idx.mutable_input_nodes().count(nid) == 0) {
+  NDArray* arr = static_cast(*arg);
+  NDArray* darr = static_cast(*grad);
+  if (new_shape == arr->shape()) {
+// do nothing
+  } else if (partial_shaping || kwargs.count(name)) {
+if (new_shape.Size() > arr->shape().Size()) {
+  CHECK(allow_up_sizing) << "New shape of arg:" << name
+  << " larger than original. "
+  << "First making a big executor and then down sizing it "
+  << "is more efficient than the reverse."
+  << "If you really want to up size, set allow_up_sizing=True "
+  << "to enable allocation of new arrays.";
+  NDArray* empty_arr = new NDArray(new_shape, arr->ctx(), false, 
arr->dtype());
 
 Review comment:
   This looks like a memory leak. Please use `arr.ReshapeAndAlloc(new_shape)`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] reminisce commented on a change in pull request #10882: move exec.reshape to backend

[GitBox]

reminisce commented on a change in pull request #10882: move exec.reshape to 
backend
URL: https://github.com/apache/incubator-mxnet/pull/10882#discussion_r187422678
 
 

 ##
 File path: src/c_api/c_api_executor.cc
 ##
 @@ -510,6 +513,135 @@ int MXExecutorSimpleBind(SymbolHandle symbol_handle,
   API_END();
 }
 
+int MXExecutorReshapeEx(SymbolHandle symbol_handle,
+int partial_shaping,
+int allow_up_sizing,
+int dev_type,
+int dev_id,
+mx_uint num_map_keys,
+const char** map_keys,
+const int* map_dev_types,
+const int* map_dev_ids,
+const mx_uint num_provided_arg_shapes,
+const char** provided_arg_shape_names,
+const mx_uint* provided_arg_shape_data,
+const mx_uint* provided_arg_shape_idx,
+mx_uint len,
+NDArrayHandle *in_args,
+NDArrayHandle *arg_grad_store,
+mx_uint *grad_req_type,
+mx_uint aux_states_len,
+NDArrayHandle *aux_states,
+ExecutorHandle shared_exec,
+ExecutorHandle *out) {
+  API_BEGIN();
+  // create shape map for in_args and aux_states
+  std::unordered_map kwargs(num_provided_arg_shapes);
+  for (mx_uint i = 0; i < num_provided_arg_shapes; ++i) {
+auto p = kwargs.emplace(provided_arg_shape_names[i],
+TShape(provided_arg_shape_data+provided_arg_shape_idx[i],
+  provided_arg_shape_data+provided_arg_shape_idx[i+1]));
+CHECK(p.second) << "Duplicate shapes are provided for argument "
+  << provided_arg_shape_names[i] << " in reshape of executor";
+  }
+  nnvm::Symbol *symb = static_cast(symbol_handle);
+  // symbol to graph
+  nnvm::Graph g;
+  g.outputs = symb->outputs;
+  g.attrs["mxnet_version"] = 
std::make_shared(static_cast(MXNET_VERSION));
+  const nnvm::IndexedGraph& idx = g.indexed_graph();
+  std::vector arg_shapes(idx.input_nodes().size(), TShape());
+  mxnet::MatchArguments(idx, kwargs, _shapes, "InferShape");
+  try {
+g = mxnet::exec::InferShape(std::move(g), std::move(arg_shapes), 
"__shape__");
+  } catch (const mxnet::op::InferShapeError ) {
+throw dmlc::Error(err.msg);
+  }
+  CHECK_EQ(g.GetAttr("shape_num_unknown_nodes"), 0U);
+  const std::vector& shape_vec = 
g.GetAttr("shape");
+  NDArrayHandle *arg = in_args;
+  NDArrayHandle *grad = arg_grad_store;
+  NDArrayHandle *aux = aux_states;
+
+  for (uint32_t nid : idx.input_nodes()) {
+std::string name = idx[nid].source->attrs.name;
+const TShape& new_shape = shape_vec[idx.entry_id(nid, 0)];
+if (idx.mutable_input_nodes().count(nid) == 0) {
+  NDArray* arr = static_cast(*arg);
+  NDArray* darr = static_cast(*grad);
+  if (new_shape == arr->shape()) {
+// do nothing
+  } else if (partial_shaping || kwargs.count(name)) {
+if (new_shape.Size() > arr->shape().Size()) {
+  CHECK(allow_up_sizing) << "New shape of arg:" << name
+  << " larger than original. "
+  << "First making a big executor and then down sizing it "
+  << "is more efficient than the reverse."
+  << "If you really want to up size, set allow_up_sizing=True "
+  << "to enable allocation of new arrays.";
+  NDArray* empty_arr = new NDArray(new_shape, arr->ctx(), false, 
arr->dtype());
+  *arr = *empty_arr;
+  if (darr != nullptr) {
+NDArray* empty_darr = new NDArray(new_shape, darr->ctx(), false, 
darr->dtype());
+*darr = *empty_darr;
+  }
+} else {
+  *arr = arr->Reshape(new_shape);
+  if (darr != nullptr) *darr = darr->Reshape(new_shape);
+}
+  } else {
+LOG(FATAL) << "Shape of unspecified array arg:" << name << " changed. "
+   << "This can cause the new executor to not share parameters 
"
+   << "with the old one. Please check for error in network."
+   << "If this is intended, set partial_shaping=True to 
suppress this warning.";
+  }
+  arg++;
+  grad++;
+} else {
+  NDArray* arr = static_cast(*aux);
+  if (new_shape == arr->shape()) {
+// do nothing
+  } else if (partial_shaping) {
+if (new_shape.Size() > arr->shape().Size()) {
+  CHECK(allow_up_sizing) << "New shape of arg:" << name
+  << " larger than original. "
+  << "First making a big executor and then down sizing it "
+  << "is more efficient than the reverse."
+  << "If you really want to up size, set allow_up_sizing=True "
+  << "to