[GitHub] piiswrong commented on a change in pull request #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
piiswrong commented on a change in pull request #9981: [fix issue#9976] The 
assignment problem in NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#discussion_r172040979
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -1128,7 +1128,7 @@ void CopyFromToImpl(const NDArray& from, const NDArray& 
to,
 }
 
 void CopyFromTo(const NDArray& from, const NDArray& to, int priority) {
-  if (from.var() == to.var()) {
+  if (from.var() == to.var() && from.byte_offset() == to.byte_offset()) {
 
 Review comment:
   should we also check for size? @reminisce 


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 test_cast (#9951)

2018-03-04 Thread 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 94d3c06  Fix test_cast (#9951)
94d3c06 is described below

commit 94d3c06f8511782f405f5dbf4bccf61647a78cf3
Author: Anirudh Subramanian 
AuthorDate: Sun Mar 4 00:32:39 2018 -0800

Fix test_cast (#9951)
---
 tests/python/unittest/test_operator.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/python/unittest/test_operator.py 
b/tests/python/unittest/test_operator.py
index 1a04e8e..1ee14b6 100644
--- a/tests/python/unittest/test_operator.py
+++ b/tests/python/unittest/test_operator.py
@@ -3229,8 +3229,8 @@ def test_cast():
 exe.arg_arrays[0][:] = X
 exe.forward(is_train=True)
 exe.backward(mx.nd.array(X, dtype=dsttype, ctx=default_context()))
-assert_almost_equal(exe.outputs[0].asnumpy(), 
X.astype(srctype).astype(dsttype), rtol=1e-3)
-assert_almost_equal(exe.grad_arrays[0].asnumpy(), 
X.astype(dsttype).astype(srctype), rtol=1e-3)
+assert_almost_equal(exe.outputs[0].asnumpy(), 
X.astype(srctype).astype(dsttype), rtol=1e-3, atol=1e-5)
+assert_almost_equal(exe.grad_arrays[0].asnumpy(), 
X.astype(dsttype).astype(srctype), rtol=1e-3, atol=1e-5)
 
 
 @with_seed()

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


[GitHub] Ldpe2G opened a new pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
Ldpe2G opened a new pull request #9984: add reshape predicator function to 
c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984
 
 
   ## Description ##
   
   add a function to c_predict_api  which can change the input shape of an 
already created predicator .
   related issue: #5360
   
   ## Checklist ##
   ### Essentials ###
   - [ ] Passed code style checking (`make lint`)
   - [ ] 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
   - [ ] 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] pengzhao-intel commented on issue #9977: Cpu lstm inference

2018-03-04 Thread GitBox
pengzhao-intel commented on issue #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#issuecomment-370213650
 
 
   @Jerryzcn Thanks for the info. So, I think it's better to merge this PR 
as-is.
   
   Our LSTM/GRU will be ready in this month and we will submit the code 
separately for the review :)


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] pengzhao-intel commented on a change in pull request #9958: Parallelization for ROIpooling OP

2018-03-04 Thread GitBox
pengzhao-intel commented on a change in pull request #9958: Parallelization for 
ROIpooling OP
URL: https://github.com/apache/incubator-mxnet/pull/9958#discussion_r172045940
 
 

 ##
 File path: src/operator/roi_pooling.cc
 ##
 @@ -74,7 +79,13 @@ inline void ROIPoolForward(const Tensor ,
 
 const Dtype* batch_data = bottom_data + data_size * roi_batch_ind;
 
+#pragma omp parallel for firstprivate(batch_data, top_data, argmax_data)
 for (int c = 0; c < channels_; ++c) {
+  // Increment all data pointers
+  const Dtype* batch_data_c = batch_data + c * data_size_c;
+  Dtype* top_data_c = top_data + c * out_size_c;
+  Dtype* argmax_data_c = argmax_data + c * max_idx_size_c;
+
 
 Review comment:
   @cjolivier01 Thanks for the great comments to Xinyu.
   Regarding replacing multiplication (FMA) to incremental addition (+=), two 
points of my view:
   
   1)  The incremental addition is more concise and logical rather than 
computing the index from start point each time by multiplication. But there're 
strong compute dependency and we **CANT** start the loop N+1 before the loop N. 
   Thus, we have to change to multiple styles for the parallelization.
   
   2) The efficiency of Multiplication (FMA) and addition (ADD) is same in the 
latest HW (Intel skylake)
   Take SSE instruct as an example:
   ADD, latency 4, CPI 0.5;
   
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=add=SSE=127,3673,127
   FMA (MUL+ADD), latency 4, CPI 0.5
   
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#expand=127,3673,2395,3673,2395,2407=mul=FMA,Other
   
   


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] pengzhao-intel commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
pengzhao-intel commented on issue #9939: add multi proposal operator (cpu 
version) and fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370225375
 
 
   @wkcn Nice work for the parallelization by OMP. **2X** speedup is a good 
start point.  
   
   I see you have added several OMP in the code. 
   Could you help profile where is the performance bottleneck now? 
   
   PS: you may need to add `gettimeofdate` in your C code and identify the most 
time-consuming loop (or section).
   


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] xinyu-intel commented on a change in pull request #9958: Parallelization for ROIpooling OP

2018-03-04 Thread GitBox
xinyu-intel commented on a change in pull request #9958: Parallelization for 
ROIpooling OP
URL: https://github.com/apache/incubator-mxnet/pull/9958#discussion_r172048022
 
 

 ##
 File path: src/operator/roi_pooling.cc
 ##
 @@ -98,30 +109,26 @@ inline void ROIPoolForward(const Tensor 
,
 
   const int pool_index = ph * pooled_width_ + pw;
   if (is_empty) {
-top_data[pool_index] = 0;
-argmax_data[pool_index] = -1;
+top_data_c[pool_index] = 0;
+argmax_data_c[pool_index] = -1;
   }
 
   for (int h = hstart; h < hend; ++h) {
 for (int w = wstart; w < wend; ++w) {
   const int index = h * width_ + w;
-  if (batch_data[index] > top_data[pool_index]) {
-top_data[pool_index] = batch_data[index];
-argmax_data[pool_index] = index;
+  if (batch_data_c[index] > top_data_c[pool_index]) {
+top_data_c[pool_index] = batch_data_c[index];
+argmax_data_c[pool_index] = index;
   }
 }
   }
 }
   }
-  // Increment all data pointers by one channel
-  batch_data += data.size(2) * data.size(3);
-  top_data += out.size(2) * out.size(3);
-  argmax_data += max_idx.size(2) * max_idx.size(3);
 }
-// Increment ROI data pointer
-bottom_rois += bbox.size(1);
+// Increase data pointers by one outsize
+top_data += channels_ * out_size_c;
 
 Review comment:
   Thanks for great comment, I've redesigned this part:)


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] xinyu-intel commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
xinyu-intel commented on issue #9939: add multi proposal operator (cpu version) 
and fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370230842
 
 
   @wkcn @pengzhao-intel I think there are so much units in multi_proposal.cc. 
We can help find which unit is the bottleneck and then fix 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] yajiedesign opened a new pull request #9985: add OPT:REF,Reduce the size of the binary file.

2018-03-04 Thread GitBox
yajiedesign opened a new pull request #9985: add OPT:REF,Reduce the size of the 
binary file.
URL: https://github.com/apache/incubator-mxnet/pull/9985
 
 
   ## Description ##
   add Gy
   add OPT:REF
   add OPT:ICF
   in msvc
   it reduce the size of the binary file.
   
   ## Checklist ##
   ### Essentials ###
   - [x] Passed code style checking (`make lint`)
   - [x] Changes are complete (i.e. I finished coding on this PR)
   - [x] All changes have test coverage:
   


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] cjolivier01 commented on a change in pull request #9985: add OPT:REF,Reduce the size of the windows binary file.

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9985: add OPT:REF,Reduce the 
size of the windows binary file.
URL: https://github.com/apache/incubator-mxnet/pull/9985#discussion_r172057234
 
 

 ##
 File path: CMakeLists.txt
 ##
 @@ -583,8 +590,8 @@ endif()
 
 if(USE_CUDA)
   if(FIRST_CUDA AND MSVC)
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd>")
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MT>")
+target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd 
-Gy>")
 
 Review comment:
   only valid for FIRST_CUDA?


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] cjolivier01 commented on a change in pull request #9985: add OPT:REF,Reduce the size of the windows binary file.

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9985: add OPT:REF,Reduce the 
size of the windows binary file.
URL: https://github.com/apache/incubator-mxnet/pull/9985#discussion_r172057243
 
 

 ##
 File path: CMakeLists.txt
 ##
 @@ -583,8 +590,8 @@ endif()
 
 if(USE_CUDA)
   if(FIRST_CUDA AND MSVC)
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd>")
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MT>")
+target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd 
-Gy>")
 
 Review comment:
   LGTM otherwise 


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] cjolivier01 commented on a change in pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9984: add reshape predicator 
function to c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984#discussion_r172057455
 
 

 ##
 File path: src/c_api/c_predict_api.cc
 ##
 @@ -243,6 +249,95 @@ int MXPredCreatePartialOut(const char* symbol_json_str,
   API_END_HANDLE_ERROR(delete ret);
 }
 
+int MXPredReshape(mx_uint num_input_nodes,
+  const char** input_keys,
+  const mx_uint* input_shape_indptr,
+  const mx_uint* input_shape_data,
+  PredictorHandle handle,
+  PredictorHandle* out) {
+  MXAPIPredictor* p = static_cast(handle);
+  MXAPIPredictor* ret = new MXAPIPredictor();
+
+  API_BEGIN();
+  // shape inference
+  std::unordered_map new_shape;
+  for (mx_uint i = 0; i < num_input_nodes; ++i) {
+new_shape[std::string(input_keys[i])] =
+TShape(input_shape_data + input_shape_indptr[i],
+input_shape_data + input_shape_indptr[i + 1]);
+  }
+  ret->sym = p->sym;
+  std::vector arg_names = 
ret->sym.ListInputNames(Symbol::kReadOnlyArgs);
+  std::vector aux_names = 
ret->sym.ListInputNames(Symbol::kAuxiliaryStates);
+  std::vector out_shapes(ret->sym.ListOutputNames().size());
+  std::vector aux_shapes(aux_names.size());
+  std::vector arg_shapes;
+  ret->key2arg = p->key2arg;
+
+  try {
+std::vector in_shapes;
 
 Review comment:
   can you use reserve()?


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] cjolivier01 commented on a change in pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9984: add reshape predicator 
function to c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984#discussion_r172057545
 
 

 ##
 File path: src/c_api/c_predict_api.cc
 ##
 @@ -243,6 +249,95 @@ int MXPredCreatePartialOut(const char* symbol_json_str,
   API_END_HANDLE_ERROR(delete ret);
 }
 
+int MXPredReshape(mx_uint num_input_nodes,
+  const char** input_keys,
+  const mx_uint* input_shape_indptr,
+  const mx_uint* input_shape_data,
+  PredictorHandle handle,
+  PredictorHandle* out) {
+  MXAPIPredictor* p = static_cast(handle);
+  MXAPIPredictor* ret = new MXAPIPredictor();
+
+  API_BEGIN();
+  // shape inference
+  std::unordered_map new_shape;
+  for (mx_uint i = 0; i < num_input_nodes; ++i) {
+new_shape[std::string(input_keys[i])] =
+TShape(input_shape_data + input_shape_indptr[i],
+input_shape_data + input_shape_indptr[i + 1]);
+  }
+  ret->sym = p->sym;
+  std::vector arg_names = 
ret->sym.ListInputNames(Symbol::kReadOnlyArgs);
+  std::vector aux_names = 
ret->sym.ListInputNames(Symbol::kAuxiliaryStates);
+  std::vector out_shapes(ret->sym.ListOutputNames().size());
+  std::vector aux_shapes(aux_names.size());
+  std::vector arg_shapes;
+  ret->key2arg = p->key2arg;
+
+  try {
+std::vector in_shapes;
+for (std::string key : ret->sym.ListInputNames(Symbol::kAll)) {
+  if (new_shape.count(key) != 0) {
+in_shapes.push_back(new_shape[key]);
+  } else {
+in_shapes.push_back(TShape());
+  }
+}
+nnvm::Graph g; g.outputs = ret->sym.outputs;
+g = mxnet::exec::InferShape(std::move(g), std::move(in_shapes), 
"__shape__");
+bool infer_complete = (g.GetAttr("shape_num_unknown_nodes") == 0);
+CHECK(infer_complete)
+  << "The shape information of is not enough to get the shapes";
+CopyAttr(g.indexed_graph(),
+ g.GetAttr("shape"),
+ _shapes, _shapes, _shapes);
+  } catch (const mxnet::op::InferShapeError ) {
+throw dmlc::Error(err.msg);
+  }
+
+  ret->arg_arrays = p->arg_arrays;
+  ret->ctx = p->ctx;
+  for (size_t i=0; i < arg_names.size(); ++i) {
+TShape newShape = arg_shapes[i];
+NDArray  = p->arg_arrays[i];
+if (new_shape.count(arg_names[i]) != 0) {
+  ret->arg_arrays[i].ReshapeAndAlloc(newShape);
+} else {
+  CHECK_EQ(newShape.Size(), arr.shape().Size())
+<< "arg " << arg_names[i]
+<< " shape has been changed, only allow to change the shape of input 
data.";
+}
+  }
+  p->arg_arrays.clear();
+
+  for (size_t i=0; i < aux_names.size(); ++i) {
+TShape newShape = aux_shapes[i];
+NDArray  = p->aux_arrays[i];
+CHECK_EQ(newShape.Size(), arr.shape().Size())
+  << "aux " << aux_names[i]
+  << " shape has been changed, only allow to change the shape of input 
data.";
+  }
+  ret->aux_arrays = p->aux_arrays;
+  p->aux_arrays.clear();
+
+  // bind
+  {
+std::map ctx_map;
+std::vector grad_store(ret->arg_arrays.size());
 
 Review comment:
   this and other places, defining to a size causes all of the constructors to 
be called. if the item is simply overwritten shortly after (ie the init item is 
just a placeholder), then it is more efficient (sometimes by a lot) to create 
an empty vector, call reserve() for the number of desired items, and then use 
emplace_back() (or push_back() if emplace can?t be called)


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] cjolivier01 commented on a change in pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9984: add reshape predicator 
function to c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984#discussion_r172057426
 
 

 ##
 File path: src/c_api/c_predict_api.cc
 ##
 @@ -243,6 +249,95 @@ int MXPredCreatePartialOut(const char* symbol_json_str,
   API_END_HANDLE_ERROR(delete ret);
 }
 
+int MXPredReshape(mx_uint num_input_nodes,
 
 Review comment:
   when does this get called? during training or during setup? 


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] cjolivier01 commented on a change in pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
cjolivier01 commented on a change in pull request #9984: add reshape predicator 
function to c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984#discussion_r172057371
 
 

 ##
 File path: src/c_api/c_predict_api.cc
 ##
 @@ -243,6 +249,95 @@ int MXPredCreatePartialOut(const char* symbol_json_str,
   API_END_HANDLE_ERROR(delete ret);
 }
 
+int MXPredReshape(mx_uint num_input_nodes,
+  const char** input_keys,
+  const mx_uint* input_shape_indptr,
+  const mx_uint* input_shape_data,
+  PredictorHandle handle,
+  PredictorHandle* out) {
+  MXAPIPredictor* p = static_cast(handle);
+  MXAPIPredictor* ret = new MXAPIPredictor();
+
 
 Review comment:
   please wrap with unique_ptr and then if all is successful, assign with the 
result of .release()


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 #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
reminisce commented on a change in pull request #9981: [fix issue#9976] The 
assignment problem in NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#discussion_r172065630
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -1128,7 +1128,7 @@ void CopyFromToImpl(const NDArray& from, const NDArray& 
to,
 }
 
 void CopyFromTo(const NDArray& from, const NDArray& to, int priority) {
-  if (from.var() == to.var()) {
+  if (from.var() == to.var() && from.byte_offset() == to.byte_offset()) {
 
 Review comment:
   @piiswrong There is a shape equality check after this. For `CopyFromTo`, I 
think the added condition is sufficient. Agree we should also check sizes if it 
is for a function of checking if two ndarrays are the same one.


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172065968
 
 

 ##
 File path: tests/ci_build/Dockerfile.build_cuda8_cudnn7
 ##
 @@ -0,0 +1,26 @@
+FROM nvidia/cuda:8.0-cudnn7-devel
+# cuda8.0 has to be used because this is the first ubuntu16.04 container
+# which is required due to OpenBLAS being incompatible with ubuntu14.04
+# the reason we used a gpu base container because we are going to test MKLDNN
+# operator implementation against GPU implementation
+
+COPY install/ubuntu_install_core.sh /install/
+RUN /install/ubuntu_install_core.sh
+COPY install/ubuntu_install_python.sh /install/
+RUN /install/ubuntu_install_python.sh
+COPY install/ubuntu_install_scala.sh /install/
+RUN /install/ubuntu_install_scala.sh
+COPY install/ubuntu_install_r.sh /install/
+RUN /install/ubuntu_install_r.sh
+COPY install/ubuntu_install_perl.sh /install/
+RUN /install/ubuntu_install_perl.sh
+
+# Allows to run tasks on a CPU without nvidia-docker and GPU
+COPY install/ubuntu_install_nvidia.sh /install/
+RUN /install/ubuntu_install_nvidia.sh
+
+# Add MKLML libraries
+RUN wget --no-check-certificate -O /tmp/mklml.tgz 
https://github.com/01org/mkl-dnn/releases/download/v0.12/mklml_lnx_2018.0.1.20171227.tgz
+RUN tar -zxvf /tmp/mklml.tgz && cp -rf mklml_*/* /usr/local/ && rm -rf mklml_*
+
+ENV LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/usr/local/lib
 
 Review comment:
   For whatever reasons libcudnn is in a strange path it seems.  To fix the 
build error you were having try replacing this line with:
   
   ```Dockerfile
   ENV 
LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/usr/local/lib:/usr/lib/x86_64-linux-gnu/
   ```


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] yajiedesign commented on a change in pull request #9985: add OPT:REF,Reduce the size of the windows binary file.

2018-03-04 Thread GitBox
yajiedesign commented on a change in pull request #9985: add OPT:REF,Reduce the 
size of the windows binary file.
URL: https://github.com/apache/incubator-mxnet/pull/9985#discussion_r172065972
 
 

 ##
 File path: CMakeLists.txt
 ##
 @@ -583,8 +590,8 @@ endif()
 
 if(USE_CUDA)
   if(FIRST_CUDA AND MSVC)
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd>")
-target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MT>")
+target_compile_options(mxnet PUBLIC "$<$:-Xcompiler=-MTd 
-Gy>")
 
 Review comment:
   It should be all effective


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] b0noI opened a new issue #9987: Implement an ability to use NCCL with the distributed training

2018-03-04 Thread GitBox
b0noI opened a new issue #9987: Implement an ability to use NCCL with the 
distributed training
URL: https://github.com/apache/incubator-mxnet/issues/9987
 
 
   Currently one can either use dist kvstore or nccl kvstore, however there is 
no way to specify NCCL mode for the distributed training.


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 #9803: R Metrics

2018-03-04 Thread GitBox
jeremiedb commented on issue #9803: R Metrics
URL: https://github.com/apache/incubator-mxnet/pull/9803#issuecomment-370259029
 
 
   Added a metric_cpu option defaulting to TRUE to have metrics computed on 
CPU, but allowing computation on GPU when needed. There's now no performance 
tradeoff. 
   
   An option has been added to perdiodically calll gc() in model.rnn. There's 
clearly a memory leak (both with FeedforwardCreate and model.rnn) but I cannot 
see how to fix this at root. 


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] kenfehling commented on issue #9501: LSTM doesn't work with Symbols and HybridBlocks

2018-03-04 Thread GitBox
kenfehling commented on issue #9501: LSTM doesn't work with Symbols and 
HybridBlocks
URL: 
https://github.com/apache/incubator-mxnet/issues/9501#issuecomment-370260477
 
 
   OK, 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] szha commented on issue #9501: LSTM doesn't work with Symbols and HybridBlocks

2018-03-04 Thread GitBox
szha commented on issue #9501: LSTM doesn't work with Symbols and HybridBlocks
URL: 
https://github.com/apache/incubator-mxnet/issues/9501#issuecomment-370260366
 
 
   Our LSTM cell doesn't support full sequence hybridization yet. The 
hybridization happens only at cell level. Thus, it will still have performance 
penalty.


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063709
 
 

 ##
 File path: src/operator/quantization/quantize_graph_pass.cc
 ##
 @@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2016 by Contributors
+ * \file quantization.cc
+ * \brief
+ */
+#include 
+#include 
+#include 
+#include 
+
+namespace mxnet {
+namespace op {
+
+using nnvm::Symbol;
+using nnvm::Node;
+using nnvm::NodePtr;
+using nnvm::NodeEntry;
+using nnvm::Graph;
+
+NodePtr CreateNode(std::string op_name, std::string node_name) {
+  NodePtr node = Node::Create();
+  node->attrs.name = node_name;
+  if (op_name == "nullptr") {
+node->attrs.op = nullptr;
+// ugly workaround because VariableParam is not exposed
+node->attrs.parsed =
+  
nnvm::Symbol::CreateVariable(node->attrs.name).outputs[0].node->attrs.parsed;
+  } else {
+node->attrs.op = Op::Get(op_name);
+  }
+  return node;
+}
+
+/*!
+ * \brief Insert a node named with ndoe_name holding the op of op_name
+ * before the node current and after the node previous.
+ */
+NodePtr InsertNode(std::string op_name,
+std::string node_name, NodePtr current, NodeEntry previous) {
+  NodePtr node = CreateNode(op_name, node_name);
+  node->inputs.emplace_back(previous);
+  current->inputs.emplace_back(NodeEntry{node, 0, 0});
+  return node;
+}
+
+std::vector OfflineParams(std::vector&& outputs,
+ std::unordered_set&& 
offline_params) {
+  std::string node_suffixs[3] = {"", "_min", "_max"};
+  std::unordered_map mirror_map;
+  nnvm::NodeEntryMap entry_var;
+  auto need_offline = [&](NodePtr n) {
+return n->op() &&
+   (n->op()->name == "_contrib_quantize") &&
+   n->inputs[0].node->is_variable() &&
+   offline_params.count(n->inputs[0].node->attrs.name);
+  };
+  DFSVisit(outputs, [&](const NodePtr& node) {
+for (NodeEntry& e : node->inputs) {
+  if (need_offline(e.node)) {
+std::string node_name = e.node->attrs.name;
+if (!entry_var.count(e)) {
+  entry_var[e] = CreateNode("nullptr", node_name + 
node_suffixs[e.index]);
+}
+e.node = entry_var[e];
+e.index = 0;
+e.version = 0;
+  }
+}
+  });
+  return outputs;
+}
+
+inline bool NeedQuantize(NodePtr node, const std::unordered_set 
excluded_nodes) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  return quantized_op_map.count(node->op()) && !excluded_nodes.count(node);
+}
+
+Graph QuantizeGraph(Graph &) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  static auto& need_requantize_map = 
Op::GetAttr("FNeedRequantize");
+  auto offline_params = 
src.GetAttr("offline_params");
+  auto excluded_nodes = 
src.GetAttr("excluded_nodes");
+
+  // mirror_map stores the mapping from the currently visited graph
 
 Review comment:
   Nit: This might be more readable if you wrapped your comments only on lines 
that exceed 100 chars (which is our standard line length for C++)


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063725
 
 

 ##
 File path: src/operator/quantization/quantize_graph_pass.cc
 ##
 @@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2016 by Contributors
+ * \file quantization.cc
+ * \brief
+ */
+#include 
+#include 
+#include 
+#include 
+
+namespace mxnet {
+namespace op {
+
+using nnvm::Symbol;
+using nnvm::Node;
+using nnvm::NodePtr;
+using nnvm::NodeEntry;
+using nnvm::Graph;
+
+NodePtr CreateNode(std::string op_name, std::string node_name) {
+  NodePtr node = Node::Create();
+  node->attrs.name = node_name;
+  if (op_name == "nullptr") {
+node->attrs.op = nullptr;
+// ugly workaround because VariableParam is not exposed
+node->attrs.parsed =
+  
nnvm::Symbol::CreateVariable(node->attrs.name).outputs[0].node->attrs.parsed;
+  } else {
+node->attrs.op = Op::Get(op_name);
+  }
+  return node;
+}
+
+/*!
+ * \brief Insert a node named with ndoe_name holding the op of op_name
+ * before the node current and after the node previous.
+ */
+NodePtr InsertNode(std::string op_name,
+std::string node_name, NodePtr current, NodeEntry previous) {
+  NodePtr node = CreateNode(op_name, node_name);
+  node->inputs.emplace_back(previous);
+  current->inputs.emplace_back(NodeEntry{node, 0, 0});
+  return node;
+}
+
+std::vector OfflineParams(std::vector&& outputs,
+ std::unordered_set&& 
offline_params) {
+  std::string node_suffixs[3] = {"", "_min", "_max"};
+  std::unordered_map mirror_map;
+  nnvm::NodeEntryMap entry_var;
+  auto need_offline = [&](NodePtr n) {
+return n->op() &&
+   (n->op()->name == "_contrib_quantize") &&
+   n->inputs[0].node->is_variable() &&
+   offline_params.count(n->inputs[0].node->attrs.name);
+  };
+  DFSVisit(outputs, [&](const NodePtr& node) {
+for (NodeEntry& e : node->inputs) {
+  if (need_offline(e.node)) {
+std::string node_name = e.node->attrs.name;
+if (!entry_var.count(e)) {
+  entry_var[e] = CreateNode("nullptr", node_name + 
node_suffixs[e.index]);
+}
+e.node = entry_var[e];
+e.index = 0;
+e.version = 0;
+  }
+}
+  });
+  return outputs;
+}
+
+inline bool NeedQuantize(NodePtr node, const std::unordered_set 
excluded_nodes) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  return quantized_op_map.count(node->op()) && !excluded_nodes.count(node);
+}
+
+Graph QuantizeGraph(Graph &) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  static auto& need_requantize_map = 
Op::GetAttr("FNeedRequantize");
+  auto offline_params = 
src.GetAttr("offline_params");
+  auto excluded_nodes = 
src.GetAttr("excluded_nodes");
+
+  // mirror_map stores the mapping from the currently visited graph
+  // to the newly created quantized graph. Key is the currently
+  // visited graph's node pointer, and value is a copied node of
+  // the key node. The existing key's value may be updated with
+  // the newly created quantize/dequantize op.
+  std::unordered_map mirror_map;
+  DFSVisit(src.outputs, [&](const NodePtr& node) {
+NodePtr new_node = Node::Create();
+// If the currently visited node need quantization,
 
 Review comment:
   typo need -> needs


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063832
 
 

 ##
 File path: src/operator/quantization/quantize_graph_pass.cc
 ##
 @@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2016 by Contributors
+ * \file quantization.cc
+ * \brief
+ */
+#include 
+#include 
+#include 
+#include 
+
+namespace mxnet {
+namespace op {
+
+using nnvm::Symbol;
+using nnvm::Node;
+using nnvm::NodePtr;
+using nnvm::NodeEntry;
+using nnvm::Graph;
+
+NodePtr CreateNode(std::string op_name, std::string node_name) {
+  NodePtr node = Node::Create();
+  node->attrs.name = node_name;
+  if (op_name == "nullptr") {
+node->attrs.op = nullptr;
+// ugly workaround because VariableParam is not exposed
+node->attrs.parsed =
+  
nnvm::Symbol::CreateVariable(node->attrs.name).outputs[0].node->attrs.parsed;
+  } else {
+node->attrs.op = Op::Get(op_name);
+  }
+  return node;
+}
+
+/*!
+ * \brief Insert a node named with ndoe_name holding the op of op_name
+ * before the node current and after the node previous.
+ */
+NodePtr InsertNode(std::string op_name,
+std::string node_name, NodePtr current, NodeEntry previous) {
+  NodePtr node = CreateNode(op_name, node_name);
+  node->inputs.emplace_back(previous);
+  current->inputs.emplace_back(NodeEntry{node, 0, 0});
+  return node;
+}
+
+std::vector OfflineParams(std::vector&& outputs,
+ std::unordered_set&& 
offline_params) {
+  std::string node_suffixs[3] = {"", "_min", "_max"};
+  std::unordered_map mirror_map;
+  nnvm::NodeEntryMap entry_var;
+  auto need_offline = [&](NodePtr n) {
+return n->op() &&
+   (n->op()->name == "_contrib_quantize") &&
+   n->inputs[0].node->is_variable() &&
+   offline_params.count(n->inputs[0].node->attrs.name);
+  };
+  DFSVisit(outputs, [&](const NodePtr& node) {
+for (NodeEntry& e : node->inputs) {
+  if (need_offline(e.node)) {
+std::string node_name = e.node->attrs.name;
+if (!entry_var.count(e)) {
+  entry_var[e] = CreateNode("nullptr", node_name + 
node_suffixs[e.index]);
+}
+e.node = entry_var[e];
+e.index = 0;
+e.version = 0;
+  }
+}
+  });
+  return outputs;
+}
+
+inline bool NeedQuantize(NodePtr node, const std::unordered_set 
excluded_nodes) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  return quantized_op_map.count(node->op()) && !excluded_nodes.count(node);
+}
+
+Graph QuantizeGraph(Graph &) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  static auto& need_requantize_map = 
Op::GetAttr("FNeedRequantize");
+  auto offline_params = 
src.GetAttr("offline_params");
+  auto excluded_nodes = 
src.GetAttr("excluded_nodes");
+
+  // mirror_map stores the mapping from the currently visited graph
+  // to the newly created quantized graph. Key is the currently
+  // visited graph's node pointer, and value is a copied node of
+  // the key node. The existing key's value may be updated with
+  // the newly created quantize/dequantize op.
+  std::unordered_map mirror_map;
+  DFSVisit(src.outputs, [&](const NodePtr& node) {
+NodePtr new_node = Node::Create();
+// If the currently visited node need quantization,
+// insert a quantize op node before the current node
+// and replace the current node with the quantized version
+// in the new graph.
+if (NeedQuantize(node, excluded_nodes)) {
+  auto fquantized_op = quantized_op_map[node->op()];
+  // If the currently visited node's op registered
+  // FQuantizedOp property, new_node is a quantizated
+  // version of a that op, such as quantized_conv2d.
+  new_node = fquantized_op(node->attrs);
+
+  // add data into quantized op input
+  for (const auto& e : node->inputs) {
+NodePtr mirror_node = mirror_map.at(e.node.get());
+NodeEntry mirror_entry = NodeEntry{
+  mirror_node, e.index, 

[GitHub] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064056
 
 

 ##
 File path: example/quantization/imagenet_inference.py
 ##
 @@ -0,0 +1,175 @@
+# 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.
+
+import argparse
+import logging
+import os
+import time
+import mxnet as mx
+from mxnet.contrib.quantization import *
+
+
+def download_dataset(dataset_url, dataset_dir, logger=None):
+if logger is not None:
+logger.info('Downloading dataset for inference from %s to %s' % 
(dataset_url, dataset_dir))
+mx.test_utils.download(dataset_url, dataset_dir)
+
+
+def load_model(symbol_file, param_file, logger=None):
+cur_path = os.path.dirname(os.path.realpath(__file__))
+symbol_file_path = os.path.join(cur_path, symbol_file)
+if logger is not None:
+logger.info('Loading symbol from file %s' % symbol_file_path)
+symbol = mx.sym.load(symbol_file_path)
+
+param_file_path = os.path.join(cur_path, param_file)
+if logger is not None:
+logger.info('Loading params from file %s' % param_file_path)
+save_dict = nd.load(param_file_path)
+arg_params = {}
+aux_params = {}
+for k, v in save_dict.items():
+tp, name = k.split(':', 1)
+if tp == 'arg':
+arg_params[name] = v
+if tp == 'aux':
+aux_params[name] = v
+return symbol, arg_params, aux_params
+
+
+def advance_data_iter(data_iter, n):
+assert n >= 0
+if n == 0:
+return data_iter
+has_next_batch = True
+while has_next_batch:
+try:
+data_iter.next()
+n -= 1
+if n == 0:
+return data_iter
+except StopIteration:
+has_next_batch = False
+
+
+def score(sym, arg_params, aux_params, data, devs, label_name, 
max_num_examples, logger=None):
+metrics = [mx.metric.create('acc'),
+   mx.metric.create('top_k_accuracy', top_k=5)]
+if not isinstance(metrics, list):
+metrics = [metrics, ]
+mod = mx.mod.Module(symbol=sym, context=devs, label_names=[label_name, ])
+mod.bind(for_training=False,
+ data_shapes=data.provide_data,
+ label_shapes=data.provide_label)
+mod.set_params(arg_params, aux_params)
+
+tic = time.time()
+num = 0
+for batch in data:
+mod.forward(batch, is_train=False)
+for m in metrics:
+mod.update_metric(m, batch.label)
+num += batch_size
+if max_num_examples is not None and num >= max_num_examples:
+break
+
+speed = num / (time.time() - tic)
+
+if logger is not None:
+logger.info('Finished inference with %d images' % num)
+logger.info('Finished with %f images per second', speed)
+for m in metrics:
+logger.info(m.get())
+
+
+if __name__ == '__main__':
+parser = argparse.ArgumentParser(description='Score a model on a dataset')
+parser.add_argument('--symbol-file', type=str, required=True, help='symbol 
file path')
+parser.add_argument('--param-file', type=str, required=True, help='param 
file path')
+parser.add_argument('--batch-size', type=int, default=32)
+parser.add_argument('--label-name', type=str, default='softmax_label')
+parser.add_argument('--dataset', type=str, required=True, help='dataset 
path')
+parser.add_argument('--rgb-mean', type=str, default='0,0,0')
+parser.add_argument('--image-shape', type=str, default='3,224,224')
+parser.add_argument('--data-nthreads', type=int, default=60, help='number 
of threads for data decoding')
+parser.add_argument('--num-skipped-batches', type=int, default=0, 
help='skip the number of batches for inference')
+parser.add_argument('--num-inference-batches', type=int, required=True, 
help='number of images used for inference')
+parser.add_argument('--shuffle-dataset', action='store_true', default=True,
+help='shuffle the calibration dataset')
+parser.add_argument('--shuffle-chunk-seed', type=int, default=3982304,
+help='shuffling chunk 

[GitHub] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064414
 
 

 ##
 File path: include/mxnet/op_attr_types.h
 ##
 @@ -261,6 +260,20 @@ using FInferStorageType = std::function* in_attrs,
   std::vector* out_attrs)>;
 
+/*!
+ * \brief Resiger an quantized node creation function based on the attrs of 
the node
 
 Review comment:
   an -> a


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064407
 
 

 ##
 File path: include/mxnet/op_attr_types.h
 ##
 @@ -261,6 +260,20 @@ using FInferStorageType = std::function* in_attrs,
   std::vector* out_attrs)>;
 
+/*!
+ * \brief Resiger an quantized node creation function based on the attrs of 
the node
 
 Review comment:
   Resiger -> Register


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064559
 
 

 ##
 File path: python/mxnet/contrib/quantization.py
 ##
 @@ -0,0 +1,506 @@
+# 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.
+"""Quantization module for generating quantized (INT8) models from FP32 
models."""
+
+from __future__ import absolute_import
+
+try:
+from scipy import stats
+except ImportError:
+stats = None
+
+import ctypes
+import logging
+import os
+import numpy as np
+from ..base import _LIB, check_call
+from ..base import c_array, c_str, mx_uint, c_str_array
+from ..base import NDArrayHandle, SymbolHandle
+from ..symbol import Symbol
+from ..symbol import load as sym_load
+from .. import ndarray
+from ..ndarray import load as nd_load
+from ..ndarray import NDArray
+from ..io import DataIter
+from ..context import cpu, Context
+from ..module import Module
+
+
+def _quantize_params(qsym, params):
+"""Given a quantized symbol and a dict of params that have not been 
quantized,
+generate quantized params. Currently only supports quantizing the 
arg_params
+with names of `weight` or `bias`, not aux_params. If `qsym` contains 
symbols
+that are excluded from being quantized, their corresponding params will
+not be quantized, but saved together with quantized params of the symbols 
that
+have been quantized.
+
+Parameters
+--
+qsym : Symbol
+Quantized symbol from FP32 symbol.
+params : dict of str->NDArray
+"""
+inputs_name = qsym.list_arguments()
+quantized_params = {}
+for name in inputs_name:
+if name.endswith(('weight_quantize', 'bias_quantize')):
+original_name = name[:-len('_quantize')]
+param = params[original_name]
+val, vmin, vmax = ndarray.contrib.quantize(data=param,
+   
min_range=ndarray.min(param),
+   
max_range=ndarray.max(param),
+   out_type='int8')
+quantized_params[name] = val
+quantized_params[name+'_min'] = vmin
+quantized_params[name+'_max'] = vmax
+elif name in params:
+quantized_params[name] = params[name]
+return quantized_params
+
+
+def _quantize_symbol(sym, excluded_symbols=None, offline_params=None):
+"""Given a symbol object representing a neural network of data type FP32,
+quantize it into a INT8 network.
+
+Parameters
+--
+sym : Symbol
+FP32 neural network symbol.
+excluded_symbols : list of symbols
+Nodes in the network that users do not want to replace with a symbol 
of INT8 data type.
+offline_params : list of strs
+Names of the parameters that users want to quantize offline. It's 
always recommended to
+quantize parameters offline so that quantizing parameters during the 
inference can be
+avoided.
+"""
+num_excluded_symbols = 0
+excluded_handles = []
+if excluded_symbols is not None:
+assert isinstance(excluded_symbols, list)
+num_excluded_symbols = len(excluded_symbols)
+for s in excluded_symbols:
+excluded_handles.append(s.handle)
+
+num_offline = 0
+offline = []
+if offline_params is not None:
+num_offline = len(offline_params)
+for k in offline_params:
+offline.append(c_str(k))
+
+out = SymbolHandle()
+check_call(_LIB.MXQuantizeSymbol(sym.handle,
+ ctypes.byref(out),
+ mx_uint(num_excluded_symbols),
+ c_array(SymbolHandle, excluded_handles),
+ mx_uint(num_offline),
+ c_array(ctypes.c_char_p, offline)))
+return Symbol(out)
+
+
+class _LayerOutputCollector(object):
+"""Saves layer output NDArray in a dict with layer names as keys and lists 
of NDArrays as
+values. The collected NDArrays will be 

[GitHub] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064592
 
 

 ##
 File path: python/mxnet/contrib/quantization.py
 ##
 @@ -0,0 +1,506 @@
+# 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.
+"""Quantization module for generating quantized (INT8) models from FP32 
models."""
+
+from __future__ import absolute_import
+
+try:
+from scipy import stats
+except ImportError:
+stats = None
+
+import ctypes
+import logging
+import os
+import numpy as np
+from ..base import _LIB, check_call
+from ..base import c_array, c_str, mx_uint, c_str_array
+from ..base import NDArrayHandle, SymbolHandle
+from ..symbol import Symbol
+from ..symbol import load as sym_load
+from .. import ndarray
+from ..ndarray import load as nd_load
+from ..ndarray import NDArray
+from ..io import DataIter
+from ..context import cpu, Context
+from ..module import Module
+
+
+def _quantize_params(qsym, params):
+"""Given a quantized symbol and a dict of params that have not been 
quantized,
+generate quantized params. Currently only supports quantizing the 
arg_params
+with names of `weight` or `bias`, not aux_params. If `qsym` contains 
symbols
+that are excluded from being quantized, their corresponding params will
+not be quantized, but saved together with quantized params of the symbols 
that
+have been quantized.
+
+Parameters
+--
+qsym : Symbol
+Quantized symbol from FP32 symbol.
+params : dict of str->NDArray
+"""
+inputs_name = qsym.list_arguments()
+quantized_params = {}
+for name in inputs_name:
+if name.endswith(('weight_quantize', 'bias_quantize')):
+original_name = name[:-len('_quantize')]
+param = params[original_name]
+val, vmin, vmax = ndarray.contrib.quantize(data=param,
+   
min_range=ndarray.min(param),
+   
max_range=ndarray.max(param),
+   out_type='int8')
+quantized_params[name] = val
+quantized_params[name+'_min'] = vmin
+quantized_params[name+'_max'] = vmax
+elif name in params:
+quantized_params[name] = params[name]
+return quantized_params
+
+
+def _quantize_symbol(sym, excluded_symbols=None, offline_params=None):
+"""Given a symbol object representing a neural network of data type FP32,
+quantize it into a INT8 network.
+
+Parameters
+--
+sym : Symbol
+FP32 neural network symbol.
+excluded_symbols : list of symbols
+Nodes in the network that users do not want to replace with a symbol 
of INT8 data type.
+offline_params : list of strs
+Names of the parameters that users want to quantize offline. It's 
always recommended to
+quantize parameters offline so that quantizing parameters during the 
inference can be
+avoided.
+"""
+num_excluded_symbols = 0
+excluded_handles = []
+if excluded_symbols is not None:
+assert isinstance(excluded_symbols, list)
+num_excluded_symbols = len(excluded_symbols)
+for s in excluded_symbols:
+excluded_handles.append(s.handle)
+
+num_offline = 0
+offline = []
+if offline_params is not None:
+num_offline = len(offline_params)
+for k in offline_params:
+offline.append(c_str(k))
+
+out = SymbolHandle()
+check_call(_LIB.MXQuantizeSymbol(sym.handle,
+ ctypes.byref(out),
+ mx_uint(num_excluded_symbols),
+ c_array(SymbolHandle, excluded_handles),
+ mx_uint(num_offline),
+ c_array(ctypes.c_char_p, offline)))
+return Symbol(out)
+
+
+class _LayerOutputCollector(object):
+"""Saves layer output NDArray in a dict with layer names as keys and lists 
of NDArrays as
+values. The collected NDArrays will be 

[GitHub] szha opened a new pull request #9986: gluon language modeling dataset and text token reader

2018-03-04 Thread GitBox
szha opened a new pull request #9986: gluon language modeling dataset and text 
token reader
URL: https://github.com/apache/incubator-mxnet/pull/9986
 
 
   ## Description ##
   gluon language modeling dataset and text token reader
   
   ## Checklist ##
   ### Essentials ###
   - [x] Passed code style checking (`make lint`)
   - [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
   - [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] add text token reader and wikitext2 language model
   
   ## Comments ##
   - for this dataset, word tokenization happens in the data reader because the 
result of tokenization could decide the sample boundary.
   


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 #9986: gluon language modeling dataset and text token reader

2018-03-04 Thread GitBox
szha commented on issue #9986: gluon language modeling dataset and text token 
reader
URL: https://github.com/apache/incubator-mxnet/pull/9986#issuecomment-370260251
 
 
   the PR for vocabulary, vectors, and model zoo models will happen separately.


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] kenfehling commented on issue #9501: LSTM doesn't work with Symbols and HybridBlocks

2018-03-04 Thread GitBox
kenfehling commented on issue #9501: LSTM doesn't work with Symbols and 
HybridBlocks
URL: 
https://github.com/apache/incubator-mxnet/issues/9501#issuecomment-370260239
 
 
   I have a question about its performance. The documentation says:
   
   > Recurrent cells allows fine-grained control when defining recurrent 
models. User can explicit step and unroll to construct complex networks. It 
provides more flexibility **but is slower** than recurrent layers.
   
   So LSTMCell is slower than LSTM, but I'm wondering if hybridized then it's 
faster?


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 #9977: Cpu lstm inference

2018-03-04 Thread GitBox
szha commented on issue #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#issuecomment-370261454
 
 
   @pengzhao-intel will the elman RNN be part of your PR? cudnn currently 
supports it, so we support it as part of the RNN layers.


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] KellenSunderland commented on a change in pull request #9918: Update mkldnn to the newest & Add clang build test with mkldnn.

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9918: Update mkldnn to 
the newest & Add clang build test with mkldnn.
URL: https://github.com/apache/incubator-mxnet/pull/9918#discussion_r172063496
 
 

 ##
 File path: tests/ci_build/Dockerfile.cpu_clang
 ##
 @@ -19,3 +19,9 @@ RUN wget -O - http://apt.llvm.org/llvm-snapshot.gpg.key | 
apt-key add - && \
 apt-get install -y clang-3.9 clang-5.0 && \
 clang-3.9 --version && \
 clang-5.0 --version
+
+# Add MKLML library, compatiable with Ubuntu16.04
+RUN wget --no-check-certificate -O /tmp/mklml.tgz 
https://github.com/intel/mkl-dnn/releases/download/v0.12/mklml_lnx_2018.0.1.20171227.tgz
 
 Review comment:
   Nit: we could put this in the same RUN to save some space.


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063626
 
 

 ##
 File path: src/operator/quantization/quantize_graph_pass.cc
 ##
 @@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2016 by Contributors
+ * \file quantization.cc
+ * \brief
+ */
+#include 
+#include 
+#include 
+#include 
+
+namespace mxnet {
+namespace op {
+
+using nnvm::Symbol;
+using nnvm::Node;
+using nnvm::NodePtr;
+using nnvm::NodeEntry;
+using nnvm::Graph;
+
+NodePtr CreateNode(std::string op_name, std::string node_name) {
+  NodePtr node = Node::Create();
+  node->attrs.name = node_name;
+  if (op_name == "nullptr") {
+node->attrs.op = nullptr;
+// ugly workaround because VariableParam is not exposed
+node->attrs.parsed =
+  
nnvm::Symbol::CreateVariable(node->attrs.name).outputs[0].node->attrs.parsed;
+  } else {
+node->attrs.op = Op::Get(op_name);
+  }
+  return node;
+}
+
+/*!
+ * \brief Insert a node named with ndoe_name holding the op of op_name
 
 Review comment:
   Type: ndoe_name -> node_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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063832
 
 

 ##
 File path: src/operator/quantization/quantize_graph_pass.cc
 ##
 @@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2016 by Contributors
+ * \file quantization.cc
+ * \brief
+ */
+#include 
+#include 
+#include 
+#include 
+
+namespace mxnet {
+namespace op {
+
+using nnvm::Symbol;
+using nnvm::Node;
+using nnvm::NodePtr;
+using nnvm::NodeEntry;
+using nnvm::Graph;
+
+NodePtr CreateNode(std::string op_name, std::string node_name) {
+  NodePtr node = Node::Create();
+  node->attrs.name = node_name;
+  if (op_name == "nullptr") {
+node->attrs.op = nullptr;
+// ugly workaround because VariableParam is not exposed
+node->attrs.parsed =
+  
nnvm::Symbol::CreateVariable(node->attrs.name).outputs[0].node->attrs.parsed;
+  } else {
+node->attrs.op = Op::Get(op_name);
+  }
+  return node;
+}
+
+/*!
+ * \brief Insert a node named with ndoe_name holding the op of op_name
+ * before the node current and after the node previous.
+ */
+NodePtr InsertNode(std::string op_name,
+std::string node_name, NodePtr current, NodeEntry previous) {
+  NodePtr node = CreateNode(op_name, node_name);
+  node->inputs.emplace_back(previous);
+  current->inputs.emplace_back(NodeEntry{node, 0, 0});
+  return node;
+}
+
+std::vector OfflineParams(std::vector&& outputs,
+ std::unordered_set&& 
offline_params) {
+  std::string node_suffixs[3] = {"", "_min", "_max"};
+  std::unordered_map mirror_map;
+  nnvm::NodeEntryMap entry_var;
+  auto need_offline = [&](NodePtr n) {
+return n->op() &&
+   (n->op()->name == "_contrib_quantize") &&
+   n->inputs[0].node->is_variable() &&
+   offline_params.count(n->inputs[0].node->attrs.name);
+  };
+  DFSVisit(outputs, [&](const NodePtr& node) {
+for (NodeEntry& e : node->inputs) {
+  if (need_offline(e.node)) {
+std::string node_name = e.node->attrs.name;
+if (!entry_var.count(e)) {
+  entry_var[e] = CreateNode("nullptr", node_name + 
node_suffixs[e.index]);
+}
+e.node = entry_var[e];
+e.index = 0;
+e.version = 0;
+  }
+}
+  });
+  return outputs;
+}
+
+inline bool NeedQuantize(NodePtr node, const std::unordered_set 
excluded_nodes) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  return quantized_op_map.count(node->op()) && !excluded_nodes.count(node);
+}
+
+Graph QuantizeGraph(Graph &) {
+  static auto& quantized_op_map = 
Op::GetAttr("FQuantizedOp");
+  static auto& need_requantize_map = 
Op::GetAttr("FNeedRequantize");
+  auto offline_params = 
src.GetAttr("offline_params");
+  auto excluded_nodes = 
src.GetAttr("excluded_nodes");
+
+  // mirror_map stores the mapping from the currently visited graph
+  // to the newly created quantized graph. Key is the currently
+  // visited graph's node pointer, and value is a copied node of
+  // the key node. The existing key's value may be updated with
+  // the newly created quantize/dequantize op.
+  std::unordered_map mirror_map;
+  DFSVisit(src.outputs, [&](const NodePtr& node) {
+NodePtr new_node = Node::Create();
+// If the currently visited node need quantization,
+// insert a quantize op node before the current node
+// and replace the current node with the quantized version
+// in the new graph.
+if (NeedQuantize(node, excluded_nodes)) {
+  auto fquantized_op = quantized_op_map[node->op()];
+  // If the currently visited node's op registered
+  // FQuantizedOp property, new_node is a quantizated
+  // version of a that op, such as quantized_conv2d.
+  new_node = fquantized_op(node->attrs);
+
+  // add data into quantized op input
+  for (const auto& e : node->inputs) {
+NodePtr mirror_node = mirror_map.at(e.node.get());
+NodeEntry mirror_entry = NodeEntry{
+  mirror_node, e.index, 

[GitHub] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172063979
 
 

 ##
 File path: example/quantization/imagenet_gen_qsym.py
 ##
 @@ -0,0 +1,194 @@
+# 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.
+
+import argparse
+import os
+import logging
+from common import modelzoo
+import mxnet as mx
+from mxnet.contrib.quantization import *
+
+
+def download_calib_dataset(dataset_url, calib_dataset, logger=None):
+if logger is not None:
+logger.info('Downloading calibration dataset from %s to %s' % 
(dataset_url, calib_dataset))
+mx.test_utils.download(dataset_url, calib_dataset)
+
+
+def download_model(model_name, logger=None):
+dir_path = os.path.dirname(os.path.realpath(__file__))
+model_path = os.path.join(dir_path, 'model')
+if logger is not None:
+logger.info('Downloading model %s... into path %s' % (model_name, 
model_path))
+return modelzoo.download_model(args.model, os.path.join(dir_path, 'model'))
+
+
+def save_symbol(fname, sym, logger=None):
+if logger is not None:
+logger.info('Saving symbol into file at %s' % fname)
+sym.save(fname)
+
+
+def save_params(fname, arg_params, aux_params, logger=None):
+if logger is not None:
+logger.info('Saving params into file at %s' % fname)
+save_dict = {('arg:%s' % k): v.as_in_context(cpu()) for k, v in 
arg_params.items()}
+save_dict.update({('aux:%s' % k): v.as_in_context(cpu()) for k, v in 
aux_params.items()})
+mx.nd.save(fname, save_dict)
+
+
+if __name__ == '__main__':
+parser = argparse.ArgumentParser(description='Generate a calibrated 
quantized model from a FP32 model')
+parser.add_argument('--model', type=str, required=True,
+help='currently only supports imagenet1k-resnet-152 or 
imagenet1k-inception-bn')
+parser.add_argument('--batch-size', type=int, default=32)
+parser.add_argument('--label-name', type=str, default='softmax_label')
+parser.add_argument('--calib-dataset', type=str, 
default='data/val_256_q90.rec',
+help='path of the calibration dataset')
+parser.add_argument('--image-shape', type=str, default='3,224,224')
+parser.add_argument('--data-nthreads', type=int, default=60,
+help='number of threads for data decoding')
+parser.add_argument('--num-calib-batches', type=int, default=10,
+help='number of batches for calibration')
+parser.add_argument('--exclude-first-conv', action='store_true', 
default=True,
+help='excluding quantizing the first conv layer since 
the'
+ ' number of channels is usually not a multiple of 
4 in that layer'
+ ' which does not satisfy the requirement of 
cuDNN')
+parser.add_argument('--shuffle-dataset', action='store_true', default=True,
+help='shuffle the calibration dataset')
+parser.add_argument('--shuffle-chunk-seed', type=int, default=3982304,
+help='shuffling chunk seed, see'
+ ' 
https://mxnet.incubator.apache.org/api/python/io/io.html?highlight=imager#mxnet.io.ImageRecordIter'
+ ' for more details')
+parser.add_argument('--shuffle-seed', type=int, default=48564309,
+help='shuffling seed, see'
+ ' 
https://mxnet.incubator.apache.org/api/python/io/io.html?highlight=imager#mxnet.io.ImageRecordIter'
+ ' for more details')
+parser.add_argument('--calib-mode', type=str, default='entropy',
+help='calibration mode used for generating calibration 
table for the quantized symbol; supports'
+ ' 1. none: no calibration will be used. The 
thresholds for quantization will be calculated'
+ ' on the fly. This will result in inference speed 
slowdown and loss of accuracy'
+ ' in general.'
+ ' 2. naive: simply 

[GitHub] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064304
 
 

 ##
 File path: example/quantization/launch_quantize.sh
 ##
 @@ -0,0 +1,40 @@
+#! /bin/sh
 
 Review comment:
   Don't think there should be a space in the shebang.


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064298
 
 

 ##
 File path: example/quantization/launch_inference.sh
 ##
 @@ -0,0 +1,44 @@
+#! /bin/sh
 
 Review comment:
   Don't think there should be a space in the shebang.


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064374
 
 

 ##
 File path: include/mxnet/c_api.h
 ##
 @@ -1386,8 +1386,37 @@ MXNET_DLL int MXSymbolInferType(SymbolHandle sym,
 const int **aux_type_data,
 int *complete);
 
-
-
+/*!
+ * \brief Convert a symbol into a quantized symbol where FP32 operators are 
replaced with INT8
+ * \param sym_handle symbol to be converted
+ * \param ret_sym_handle quantized symbol result
+ * \param num_excluded_symbols number of layers excluded from being quantized 
in the input symbol
+ * \param excluded_symbols array of symbols to be excluded from being quantized
+ * \param num_offline number of parameters that are quantized offline
+ * \param offline_params array of c strings representing the names of params 
quantized offline
+ */
+MXNET_DLL int MXQuantizeSymbol(SymbolHandle sym_handle,
+   SymbolHandle *ret_sym_handle,
+   mx_uint num_excluded_symbols,
+   SymbolHandle *excluded_symbols,
 
 Review comment:
   Are excluded_symbols mutable?  Just wondering if they should be const 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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064452
 
 

 ##
 File path: include/mxnet/op_attr_types.h
 ##
 @@ -261,6 +260,20 @@ using FInferStorageType = std::function* in_attrs,
   std::vector* out_attrs)>;
 
+/*!
+ * \brief Resiger an quantized node creation function based on the attrs of 
the node
+ * \note Register under "FQuantizedOp" for non-quantized operators
+ */
+using FQuantizedOp = std::function;
+
+/*!
+ * \brief Resiger an function to determine if the output a quantized operator
 
 Review comment:
   output a quantized -> output of a quantized


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] KellenSunderland commented on a change in pull request #9552: [REQUEST FOR REVIEW | DO NOT MERGE] Model Quantization with Calibration

2018-03-04 Thread GitBox
KellenSunderland commented on a change in pull request #9552: [REQUEST FOR 
REVIEW | DO NOT MERGE] Model Quantization with Calibration
URL: https://github.com/apache/incubator-mxnet/pull/9552#discussion_r172064433
 
 

 ##
 File path: include/mxnet/op_attr_types.h
 ##
 @@ -261,6 +260,20 @@ using FInferStorageType = std::function* in_attrs,
   std::vector* out_attrs)>;
 
+/*!
+ * \brief Resiger an quantized node creation function based on the attrs of 
the node
+ * \note Register under "FQuantizedOp" for non-quantized operators
+ */
+using FQuantizedOp = std::function;
+
+/*!
+ * \brief Resiger an function to determine if the output a quantized operator
 
 Review comment:
   Resiger an -> Register a


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] Ldpe2G commented on a change in pull request #9984: add reshape predicator function to c_predict_api

2018-03-04 Thread GitBox
Ldpe2G commented on a change in pull request #9984: add reshape predicator 
function to c_predict_api
URL: https://github.com/apache/incubator-mxnet/pull/9984#discussion_r172070078
 
 

 ##
 File path: src/c_api/c_predict_api.cc
 ##
 @@ -243,6 +249,95 @@ int MXPredCreatePartialOut(const char* symbol_json_str,
   API_END_HANDLE_ERROR(delete ret);
 }
 
+int MXPredReshape(mx_uint num_input_nodes,
 
 Review comment:
   during testing, when you need to change the input shape of an already 
created predicator.


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] cgraywang commented on issue #9572: Return a NaN when using operator ( ** ) on Windows version with GPU

2018-03-04 Thread GitBox
cgraywang commented on issue #9572: Return a NaN when using operator ( ** ) on 
Windows version with GPU
URL: 
https://github.com/apache/incubator-mxnet/issues/9572#issuecomment-370275278
 
 
   @Feywell ok, I will be looking into it. Since I will need to setting up the 
windows, it may take sometime. But I will get back to you asap.


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 #9803: R Metrics

2018-03-04 Thread GitBox
jeremiedb commented on issue #9803: R Metrics
URL: https://github.com/apache/incubator-mxnet/pull/9803#issuecomment-370259029
 
 
   Added a metric_cpu option defaulting to TRUE to have metrics computed on 
CPU, but allowing computation on GPU when needed. There's now no performance 
tradeoff. 
   
   An option has been added to perdiodically calll gc() in model.rnn. There's 
clearly a memory leak (both with FeedforwardCreate and model.rnn) but I cannot 
see how to fix this at root. This is a pre-existing issue, to be fixed in the 
future. 


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] samhodge commented on issue #9989: Cannot train example gluon style transfer

2018-03-04 Thread GitBox
samhodge commented on issue #9989: Cannot train example gluon style transfer
URL: 
https://github.com/apache/incubator-mxnet/issues/9989#issuecomment-370296496
 
 
   @zhanghang1989 Do you have any input?


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] wkcn commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
wkcn commented on issue #9939: add multi proposal operator (cpu version) and 
fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370320488
 
 
   For the cpu/gpu consistency, it's interesting that the number of valid 
anchors in the CPU implementation and the GPU implementation may be different.
   The reason is that the float precision in CPU's and GPU's is different.
   The size of some anchors may be near the margin of the minimal valid anchor 
or the maximal valid anchor.
   
   The margin of the minimal valid anchor:
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L159
   
   The margin of the maximal valid anchor.
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L89
   
   I want to create testing sample to avoid these margins.
   


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] wkcn commented on a change in pull request #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
wkcn commented on a change in pull request #9981: [fix issue#9976] The 
assignment problem in NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#discussion_r172075822
 
 

 ##
 File path: tests/python/unittest/test_ndarray.py
 ##
 @@ -1099,6 +1099,39 @@ def test_assign_float_value_to_ndarray():
 b[0] = a[0]
 assert same(a, b.asnumpy())
 
+@with_seed()
+def test_ndarray_assignment():
+H, W = 10, 10
+a_np = np.random.random((H, W))
+a_nd = mx.nd.array(a_np)
+a_nd_id = id(a_nd)
+
+# assign directly
+a_np[0] = a_np[1]
+a_nd[0] = a_nd[1]
+assert np.allclose(a_np, a_nd.asnumpy())
+assert id(a_nd) == a_nd_id
 
 Review comment:
   My mistake. I want to check the memory address of NDArray didn't change.
   And `assert np.allclose(a_np, a_nd.asnumpy())` means checking issue #9976.


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] wkcn commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
wkcn commented on issue #9939: add multi proposal operator (cpu version) and 
fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370285916
 
 
   @pengzhao-intel @xinyu-intel 
   Thank you! I will have a try.


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] HaoLiuHust closed issue #9962: how to group 2 or more outputs in gluon just like sym.Group()

2018-03-04 Thread GitBox
HaoLiuHust closed issue #9962: how to group 2 or more outputs in gluon just 
like sym.Group()
URL: https://github.com/apache/incubator-mxnet/issues/9962
 
 
   


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] HaoLiuHust commented on issue #9962: how to group 2 or more outputs in gluon just like sym.Group()

2018-03-04 Thread GitBox
HaoLiuHust commented on issue #9962: how to group 2 or more outputs in gluon 
just like sym.Group()
URL: 
https://github.com/apache/incubator-mxnet/issues/9962#issuecomment-370288388
 
 
   @orchidmajumder Thanks, get your point


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] TaoLv commented on issue #9977: Cpu lstm inference

2018-03-04 Thread GitBox
TaoLv commented on issue #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#issuecomment-370287856
 
 
   @szha To make it easier to review, we would like to split the whole RNN 
implementation on CPU into several PRs. Firstly, We will submit code for 
single-layer and unidirectional LSTM/GRU. Then, multi-layer and bidirectional 
support will be added for LSTM/GRU. Vanilla RNN (maybe elman RNN in your words) 
will be supported after we finish LSTM/GRU. Actually, we have implemented fused 
vanilla RNN, but I think it should be a low priority to integrated it into 
mxnet, compared with LSTM/GRU.
   
   @szha What about your opinion? We can set a detailed plan for this PRs if 
needed.
   @pengzhao-intel Correct me if I missed anything. 


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] samhodge opened a new issue #9989: Cannot train example gluon style transfer

2018-03-04 Thread GitBox
samhodge opened a new issue #9989: Cannot train example gluon style transfer
URL: https://github.com/apache/incubator-mxnet/issues/9989
 
 
   Note: Providing complete information in the most concise form is the best 
way to get help. This issue template serves as the checklist for essential 
information to most of the technical issues and bug reports. For non-technical 
issues and feature requests, feel free to present the information in what you 
believe is the best form.
   
   For Q & A and discussion, please start a discussion thread at 
https://discuss.mxnet.io 
   
   ## Description
   Cannot train gluon style transfer, needs to be outside of autograd.record() 
block or need to call backward.
   
   ## Environment info (Required)
   --Python Info--
   ('Version  :', '2.7.10')
   ('Compiler :', 'GCC 4.1.2')
   ('Build:', ('default', 'Jun 29 2015 12:45:31'))
   ('Arch :', ('64bit', 'ELF'))
   Pip Info---
   No corresponding pip install for current python.
   --MXNet Info---
   
/asset/common/software/thirdparty/mxnet/1.0.0-build1/python2.7/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:', 
'/asset/common/software/thirdparty/mxnet/1.0.0-build1/python2.7/mxnet')
   Hashtag not found. Not installed from pre-built package.
   --System Info--
   ('Platform :', 
'Linux-3.10.105-1.el6.elrepo.x86_64-x86_64-with-centos-6.2-Final')
   ('system   :', 'Linux')
   ('node :', 'bladerunner')


   ('release  :', '3.10.105-1.el6.elrepo.x86_64')   


   ('version  :', '#1 SMP Fri Feb 10 10:48:08 EST 2017')


   --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:1 


   Core(s) per socket:6 


   Socket(s): 2 

   

[GitHub] szha commented on issue #9977: Cpu lstm inference

2018-03-04 Thread GitBox
szha commented on issue #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#issuecomment-370295377
 
 
   @TaoLv sounds good. What timeline are we looking at for feature parity with 
cudnn?


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] sergeykolychev commented on issue #9988: [Perl] Sparse feature.

2018-03-04 Thread GitBox
sergeykolychev commented on issue #9988: [Perl] Sparse feature.
URL: https://github.com/apache/incubator-mxnet/pull/9988#issuecomment-370290337
 
 
   @tlby Rob, please take a look, 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] wkcn commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
wkcn commented on issue #9939: add multi proposal operator (cpu version) and 
fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370320488
 
 
   For the cpu/gpu consistency test, it's interesting that the number of valid 
anchors in the CPU implementation and the GPU implementation may be different.
   The reason is that the float precision in CPU's and GPU's is different.
   The size of some anchors may be near the margin of the minimal valid anchor 
and overlap.
   
   The margin of the minimal valid anchor:
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L141
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L159
   
   Overlap:
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L273
   
   I want to create testing sample to avoid these margins.
   


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 #9887: Non-blocking row_sparse_pull. Fix incorrect indices generated by device kvstore.row_sparse_pull

2018-03-04 Thread GitBox
ZiyueHuang commented on issue #9887: Non-blocking row_sparse_pull. Fix 
incorrect indices generated by device kvstore.row_sparse_pull
URL: https://github.com/apache/incubator-mxnet/pull/9887#issuecomment-370323043
 
 
   @eric-haibin-lin Thanks for the fix!
   
   Looks good.


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] asitstands opened a new pull request #9991: Random shuffle implementation

2018-03-04 Thread GitBox
asitstands opened a new pull request #9991: Random shuffle implementation
URL: https://github.com/apache/incubator-mxnet/pull/9991
 
 
   ## Description ##
   
   This operator randomly shuffles the elements of an NDArray along the last 
axis, i.e., it performs a batched shuffling. For example, if you shuffle a 
matrix (2D array), the elements in each row are shuffled inside the row and the 
shufflings in different rows are statistically independent. So, for each 
element, all indices except the last one perserved but the last one changes 
randomly. 
   
   ### Motivation
   
   1. Random shuffle is a widely used operation so that most of the random 
number APIs, such as those of numpy or C++, have it while mxnet lacks the 
operation. It would be useful especially for the shuffling of the arrays in gpu.
   2. If this PR is accepted I'd like to open a separate PR which removes the 
dependency of mxnet python API on the *global* random number generators of 
python and numpy. I think that the dependency is not sound because it 
introduces an unnecessary coupling between mxnet and other parts of a user's 
environment. This can be a source of subtle technical or mathematical bugs. 
Personally I have spent some amount of meaningless time to figure out the 
source of an unexpected randomness due to the coupling. The dependency has 
confused many newcomers, for example, see #9978, #9335, #7124 and #736. To 
replace random number generations through the global generators of python and 
numpy with the generations through the operations of `mx.nd.random`, we need to 
copy the data back and forth between a numpy array and an NDArray or a python 
list and an NDArray, but I think that the overhead from this is not significant 
and avoiding the coupling is much more important. When I actually tried the 
replacing, the only obstacle was that mxnet does not support the random 
shuffling. So I implemented it. However, I think that the suffling is useful 
regardless of this motivation and so opened a separate PR.
   
   ### Implementation
   
   1. In cpu, the shuffling is delegated to `__gnu_parallel::random_shuffle` 
for gcc and to `std::shuffle` for other compilers. The shufflings in a batch 
are processed sequentially one by one.
   2. In gpu, a random key of `double` type is generated for each element of 
the array and then the elements are sorted by the keys. The keys for `n`-th 
batch is distributed in the range `[n, n + 1]` and the entire array is sorted 
by the keys at once. The sorting is delegated to mxnet's `SortByKey` which 
again delegates the call to thrust's `sort_by_key`. If both of the batch size 
and the length of the last axis are very large, the qualitiy of the shuffling 
could be bad in this implementation, i.e., the outcomes may not be uniformly 
distributed. It is because, as `n` grows,  the number of floating point numbers 
in `[n, n+1]` becomes smaller and so the probability that there are duplicated 
keys grows. However, I believe that it hardly causes a problem in practice. If 
this is not acceptable, an alternative implementation is to process the 
shufflings in a batch sequentially. However, this alternative shows very poor 
performance comparing to the current implementation even when the batch size is 
some hundreds.
   3. Inplace shuffling is allowed. 
   
   ## Checklist ##
   ### Essentials ###
   - [x] Passed code style checking (`make lint`)
   - [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
   - [x] 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] piiswrong commented on issue #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
piiswrong commented on issue #9981: [fix issue#9976] The assignment problem in 
NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#issuecomment-370287669
 
 
   @marcoabreu retrigger test?


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] TaoLv commented on issue #9977: Cpu lstm inference

2018-03-04 Thread GitBox
TaoLv commented on issue #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#issuecomment-370287856
 
 
   @szha To make it easier to review, we would like to split the whole RNN 
implementation on CPU into several PRs. Firstly, We will submit code for 
single-layer and unidirectional LSTM/GRU. Then, multi-layer and bidirectional 
support will be added for LSTM/GRU. Vanilla RNN (maybe elman RNN in your words) 
will be supported after we finish LSTM/GRU. Actually, we have already done the 
code for fused vanilla RNN, but I think it should be a low priority compared 
with LSTM/GRU.
   
   @szha What about your opinion? We can set a detailed plan for this PRs if 
needed.
   @pengzhao-intel Correct me if I missed anything. 


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] arundasan91 opened a new pull request #9990: Update tensorboard.py

2018-03-04 Thread GitBox
arundasan91 opened a new pull request #9990: Update tensorboard.py
URL: https://github.com/apache/incubator-mxnet/pull/9990
 
 
   Adding `step` to Tensorboard. Currently only horizontal axis setting of 
`Relative` or `Wall` are available without any info about the epoch.
   
   ## Description ##
   Current Tensorboard integration with `dmlc/tensorboard` allows good 
visualizations for MXNET training/eval procedure. However, visualizations are 
limited to horizontal axis setting of `Relative` or `Wall` and do not include 
any information about current epoch OR relation between the plots and epoch. 
   
   After spending some time on the code at `dmlc/tensorboard`, I found that the 
[add_scalar](https://github.com/dmlc/tensorboard/blob/master/python/tensorboard/writer.py#L264)
 function have an argument named `global_step` to log `step` or `epoch` 
information. I added a simple fix to the 
[tensorboard.py](https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/contrib/tensorboard.py)
 in MXNET to address this lost information. Kindly take a look at the figures 
below:
   
   Tensorboard plots BEFORE FIX:
   1. Horizontal axis: `step`
   ![screen shot 2018-03-04 at 8 27 12 
pm](https://user-images.githubusercontent.com/14352842/36955429-1680d56e-1fee-11e8-86a6-132d7accb079.jpg)
   2. Horizontal axis: `Relative`
   ![screen shot 2018-03-04 at 8 27 22 
pm](https://user-images.githubusercontent.com/14352842/36955442-30b042e4-1fee-11e8-967b-13c2e79b5c3e.jpg)
   
   Tensorboard plot AFTER FIX:
   Horizontal axis: `step`
   ![screen shot 2018-03-04 at 8 35 39 
pm](https://user-images.githubusercontent.com/14352842/36955448-3beaaf96-1fee-11e8-8d7b-a294798319e2.jpg)
   
   
   
   ## Checklist ##
   ### Essentials ###
   - [x] Passed code style checking (`make lint`)
   - [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
   - [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 ###
   None: No major changes.
   
   ## Comments ##
   I am getting discontinuous tensorboard plots while plotting training graph 
on Tensorboard w/ my own dataset as you can see in the figure. I am pretty sure 
it has to do with the data and not the fix. 


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] wkcn commented on a change in pull request #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
wkcn commented on a change in pull request #9981: [fix issue#9976] The 
assignment problem in NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#discussion_r172075839
 
 

 ##
 File path: tests/python/unittest/test_ndarray.py
 ##
 @@ -1099,6 +1099,39 @@ def test_assign_float_value_to_ndarray():
 b[0] = a[0]
 assert same(a, b.asnumpy())
 
+@with_seed()
+def test_ndarray_assignment():
 
 Review comment:
   Okay. 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] wkcn commented on issue #9939: add multi proposal operator (cpu version) and fix the bug in proposal op (gpu version)

2018-03-04 Thread GitBox
wkcn commented on issue #9939: add multi proposal operator (cpu version) and 
fix the bug in proposal op (gpu version)
URL: https://github.com/apache/incubator-mxnet/pull/9939#issuecomment-370320488
 
 
   For the cpu/gpu consistency test, it's interesting that the number of valid 
anchors in the CPU implementation and the GPU implementation may be different.
   The reason is that the float precision in CPU's and GPU's is different.
   The size of some anchors may be near the margin of the minimal valid anchor 
or the maximal valid anchor.
   
   The margin of the minimal valid anchor:
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L159
   
   The margin of the maximal valid anchor.
   
https://github.com/wkcn/incubator-mxnet/blob/add_multi_proposal_cpu_version/src/operator/contrib/multi_proposal.cc#L89
   
   I want to create testing sample to avoid these margins.
   


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] wkcn commented on issue #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
wkcn commented on issue #9981: [fix issue#9976] The assignment problem in 
NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#issuecomment-370288088
 
 
   @piiswrong Thank you! I will change the code and then trigger test.


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] wkcn commented on a change in pull request #9981: [fix issue#9976] The assignment problem in NDArray

2018-03-04 Thread GitBox
wkcn commented on a change in pull request #9981: [fix issue#9976] The 
assignment problem in NDArray
URL: https://github.com/apache/incubator-mxnet/pull/9981#discussion_r172075502
 
 

 ##
 File path: tests/python/unittest/test_ndarray.py
 ##
 @@ -1099,6 +1099,39 @@ def test_assign_float_value_to_ndarray():
 b[0] = a[0]
 assert same(a, b.asnumpy())
 
+@with_seed()
+def test_ndarray_assignment():
+H, W = 10, 10
+a_np = np.random.random((H, W))
+a_nd = mx.nd.array(a_np)
+a_nd_id = id(a_nd)
+
+# assign directly
+a_np[0] = a_np[1]
+a_nd[0] = a_nd[1]
+assert np.allclose(a_np, a_nd.asnumpy())
 
 Review comment:
   Thank you! I will modify 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] eric-haibin-lin commented on issue #9887: Non-blocking row_sparse_pull

2018-03-04 Thread GitBox
eric-haibin-lin commented on issue #9887: Non-blocking row_sparse_pull 
URL: https://github.com/apache/incubator-mxnet/pull/9887#issuecomment-370303541
 
 
   This PR also fixed a bug in the original GPU "Unique" implementation, where 
the same pointer is passed as both the input/output to cub::unique. This 
results in undefined output when the number of rowids is large. Fixed by 
storing the input/output in separate buffers. 


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 #9887: Non-blocking row_sparse_pull

2018-03-04 Thread GitBox
eric-haibin-lin commented on a change in pull request #9887: Non-blocking 
row_sparse_pull 
URL: https://github.com/apache/incubator-mxnet/pull/9887#discussion_r172087240
 
 

 ##
 File path: tests/python/unittest/test_kvstore.py
 ##
 @@ -76,7 +76,7 @@ def check_row_sparse_pull(kv, count):
 for i in range(count):
 vals.append(mx.nd.zeros(shape).tostype('row_sparse'))
 row_id = np.random.randint(num_rows, size=num_rows)
-row_ids.append(mx.nd.array(row_id))
+row_ids.append(mx.nd.array(row_id).reshape((2,2)))
 
 Review comment:
   You are right.. Made it less hard-coded by calculating the dimension based 
on total number of elements. 
   I'm testing 2-D rowids explicitly because that seems to be a common use 
case, many users forget to reshape it to 1-D before passing it to kvstore. It 
is now supported with 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] eric-haibin-lin commented on issue #9887: Non-blocking row_sparse_pull

2018-03-04 Thread GitBox
eric-haibin-lin commented on issue #9887: Non-blocking row_sparse_pull 
URL: https://github.com/apache/incubator-mxnet/pull/9887#issuecomment-370303883
 
 
   @ZiyueHuang please view the changes for gpu unique


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 #9887: Non-blocking row_sparse_pull

2018-03-04 Thread GitBox
eric-haibin-lin commented on issue #9887: Non-blocking row_sparse_pull 
URL: https://github.com/apache/incubator-mxnet/pull/9887#issuecomment-370303541
 
 
   This PR also fixed a bug in the original GPU "Unique" implementation, where 
the same pointer is passed as both the input/output to cub::unique. This 
results in incorrect output when the number of rowids is large. Fixed by 
storing the input/output in separate buffers. 


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] II-Matto closed issue #9012: Implementing New Operators

2018-03-04 Thread GitBox
II-Matto closed issue #9012: Implementing New Operators
URL: https://github.com/apache/incubator-mxnet/issues/9012
 
 
   


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] xinyu-intel commented on a change in pull request #9918: Update mkldnn to the newest & Add clang build test with mkldnn.

2018-03-04 Thread GitBox
xinyu-intel commented on a change in pull request #9918: Update mkldnn to the 
newest & Add clang build test with mkldnn.
URL: https://github.com/apache/incubator-mxnet/pull/9918#discussion_r172090005
 
 

 ##
 File path: tests/ci_build/Dockerfile.cpu_clang
 ##
 @@ -19,3 +19,9 @@ RUN wget -O - http://apt.llvm.org/llvm-snapshot.gpg.key | 
apt-key add - && \
 apt-get install -y clang-3.9 clang-5.0 && \
 clang-3.9 --version && \
 clang-5.0 --version
+
+# Add MKLML library, compatiable with Ubuntu16.04
+RUN wget --no-check-certificate -O /tmp/mklml.tgz 
https://github.com/intel/mkl-dnn/releases/download/v0.12/mklml_lnx_2018.0.1.20171227.tgz
 
 Review comment:
   Thanks for comments! Do these dockerfiles run at the same time and share the 
same /tmp dir?


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] xinyu-intel commented on a change in pull request #9918: Update mkldnn to the newest & Add clang build test with mkldnn.

2018-03-04 Thread GitBox
xinyu-intel commented on a change in pull request #9918: Update mkldnn to the 
newest & Add clang build test with mkldnn.
URL: https://github.com/apache/incubator-mxnet/pull/9918#discussion_r172092504
 
 

 ##
 File path: tests/ci_build/Dockerfile.cpu_clang
 ##
 @@ -19,3 +19,9 @@ RUN wget -O - http://apt.llvm.org/llvm-snapshot.gpg.key | 
apt-key add - && \
 apt-get install -y clang-3.9 clang-5.0 && \
 clang-3.9 --version && \
 clang-5.0 --version
+
+# Add MKLML library, compatiable with Ubuntu16.04
+RUN wget --no-check-certificate -O /tmp/mklml.tgz 
https://github.com/intel/mkl-dnn/releases/download/v0.12/mklml_lnx_2018.0.1.20171227.tgz
 
 Review comment:
   I've tried to put mklml script in a single run file, but get the following 
error:
   ```
   Step 13/14 : RUN /install/ubuntu_install_mklml.sh
---> Running in 53baae7b5275
   /bin/sh: 1: /install/ubuntu_install_mklml.sh: Permission denied
   The command '/bin/sh -c /install/ubuntu_install_mklml.sh' returned a 
non-zero code: 126
   ERROR: docker build failed.
   ```
   Can you help me fix it? By the way, I think the run file should first judge 
whether the file already exists. Thank you.


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] sergeykolychev opened a new pull request #9988: [Perl] Sparse feature.

2018-03-04 Thread GitBox
sergeykolychev opened a new pull request #9988: [Perl] Sparse feature.
URL: https://github.com/apache/incubator-mxnet/pull/9988
 
 
   ## Description ##
   (Brief description on what this PR is about)
   
   ## Checklist ##
   ### Essentials ###
   - [ ] Passed code style checking (`make lint`)
   - [ ] 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
   - [ ] 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