[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-30 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164930373
 
 

 ##
 File path: tests/python/gpu/test_gluon_model_zoo.py
 ##
 @@ -0,0 +1,163 @@
+# 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.
+
+from __future__ import print_function
+import mxnet as mx
+import numpy as np
+import copy
+from mxnet import autograd
+from mxnet.gluon.model_zoo.vision import get_model
+from mxnet.test_utils import assert_almost_equal
+import sys
+
+def eprint(*args, **kwargs):
+print(*args, file=sys.stderr, **kwargs)
+
+VAL_DATA='data/val-5k-256.rec'
+def download_data():
+return mx.test_utils.download(
+'http://data.mxnet.io/data/val-5k-256.rec', VAL_DATA)
+
+def test_inference():
+all_models = ['resnet50_v1', 'vgg19_bn', 'alexnet', #'inceptionv3',
+  'densenet201', 'squeezenet1.0', 'mobilenet0.25']
+
+batch_size = 10
+download_data()
+for model_name in all_models:
+eprint('testing inference on %s'%model_name)
+
+data_shape = (3, 224, 224) if 'inception' not in model_name else (3, 
299, 299)
+dataIter = mx.io.ImageRecordIter(
+path_imgrec= VAL_DATA,
+label_width= 1,
+preprocess_threads = 1,
+batch_size = batch_size,
+data_shape = data_shape,
+label_name = 'softmax_label',
+rand_crop  = False,
+rand_mirror= False)
+data_batch = dataIter.next()
+data = data_batch.data[0]
+label = data_batch.label[0]
+gpu_data = data.as_in_context(mx.gpu())
+gpu_label = label.as_in_context(mx.gpu())
+
+# This is to create a model and run the model once to initialize
+# all parameters.
+cpu_model = get_model(model_name)
+cpu_model.collect_params().initialize(ctx=mx.cpu())
+cpu_model(mx.nd.array(data, ctx=mx.cpu()))
+gpu_model = get_model(model_name)
+gpu_model.collect_params().initialize(ctx=mx.gpu())
+gpu_model(mx.nd.array(data, ctx=mx.gpu()))
+
+# Force the two models have the same parameters.
+cpu_params = cpu_model.collect_params()
+gpu_params = gpu_model.collect_params()
+for k in cpu_params.keys():
+k = k.replace(cpu_params.prefix, '')
+cpu_param = cpu_params.get(k)
+gpu_param = gpu_params.get(k)
+gpu_param.set_data(cpu_param.data().as_in_context(mx.gpu()))
+
+# Run inference.
+with autograd.record(train_mode=False):
+cpu_out = cpu_model(mx.nd.array(data, ctx=mx.cpu()))
+gpu_out = gpu_model(gpu_data)
+out = cpu_out.asnumpy()
+max_val = np.max(out)
+assert_almost_equal(out / max_val, gpu_out.asnumpy() / max_val, 
rtol=1e-2, atol=1e-2)
+
+def get_nn_model(name):
+if "densenet" in name:
+return get_model(name, dropout=0)
+else:
+return get_model(name)
+
+def test_training():
+# We use network models without dropout for testing.
+# TODO(zhengda) mobilenet can't pass this test even without MKLDNN.
 
 Review comment:
   I'll look into the problem later. It's not related to this PR. The native 
MXNet operators can't pass the test with mobilenet. Most likely it's caused by 
depth-wise convolution.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-30 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164921462
 
 

 ##
 File path: tests/python/gpu/test_gluon_model_zoo.py
 ##
 @@ -0,0 +1,163 @@
+# 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.
+
+from __future__ import print_function
+import mxnet as mx
+import numpy as np
+import copy
+from mxnet import autograd
+from mxnet.gluon.model_zoo.vision import get_model
+from mxnet.test_utils import assert_almost_equal
+import sys
+
+def eprint(*args, **kwargs):
+print(*args, file=sys.stderr, **kwargs)
+
+VAL_DATA='data/val-5k-256.rec'
+def download_data():
+return mx.test_utils.download(
+'http://data.mxnet.io/data/val-5k-256.rec', VAL_DATA)
+
+def test_inference():
+all_models = ['resnet50_v1', 'vgg19_bn', 'alexnet', #'inceptionv3',
+  'densenet201', 'squeezenet1.0', 'mobilenet0.25']
+
+batch_size = 10
+download_data()
+for model_name in all_models:
+eprint('testing inference on %s'%model_name)
+
+data_shape = (3, 224, 224) if 'inception' not in model_name else (3, 
299, 299)
+dataIter = mx.io.ImageRecordIter(
+path_imgrec= VAL_DATA,
+label_width= 1,
+preprocess_threads = 1,
+batch_size = batch_size,
+data_shape = data_shape,
+label_name = 'softmax_label',
+rand_crop  = False,
+rand_mirror= False)
+data_batch = dataIter.next()
+data = data_batch.data[0]
+label = data_batch.label[0]
+gpu_data = data.as_in_context(mx.gpu())
+gpu_label = label.as_in_context(mx.gpu())
+
+# This is to create a model and run the model once to initialize
+# all parameters.
+cpu_model = get_model(model_name)
+cpu_model.collect_params().initialize(ctx=mx.cpu())
+cpu_model(mx.nd.array(data, ctx=mx.cpu()))
+gpu_model = get_model(model_name)
+gpu_model.collect_params().initialize(ctx=mx.gpu())
+gpu_model(mx.nd.array(data, ctx=mx.gpu()))
+
+# Force the two models have the same parameters.
+cpu_params = cpu_model.collect_params()
+gpu_params = gpu_model.collect_params()
+for k in cpu_params.keys():
+k = k.replace(cpu_params.prefix, '')
+cpu_param = cpu_params.get(k)
+gpu_param = gpu_params.get(k)
+gpu_param.set_data(cpu_param.data().as_in_context(mx.gpu()))
+
+# Run inference.
+with autograd.record(train_mode=False):
+cpu_out = cpu_model(mx.nd.array(data, ctx=mx.cpu()))
+gpu_out = gpu_model(gpu_data)
+out = cpu_out.asnumpy()
+max_val = np.max(out)
+assert_almost_equal(out / max_val, gpu_out.asnumpy() / max_val, 
rtol=1e-2, atol=1e-2)
+
+def get_nn_model(name):
+if "densenet" in name:
+return get_model(name, dropout=0)
+else:
+return get_model(name)
+
+def test_training():
+# We use network models without dropout for testing.
+# TODO(zhengda) mobilenet can't pass this test even without MKLDNN.
+all_models = ['resnet18_v1', 'densenet121']
+
+batch_size = 10
+label = mx.nd.random.uniform(low=0, high=10, 
shape=(batch_size)).astype('int32')
+
+download_data()
+dataIter = mx.io.ImageRecordIter(
+path_imgrec= VAL_DATA,
+label_width= 1,
+preprocess_threads = 1,
+batch_size = batch_size,
+data_shape = (3, 224, 224),
+label_name = 'softmax_label',
+rand_crop  = False,
+rand_mirror= False)
+data_batch = dataIter.next()
+data = data_batch.data[0]
+label = data_batch.label[0]
+gpu_data = data.as_in_context(mx.gpu())
+gpu_label = label.as_in_context(mx.gpu())
+softmax_cross_entropy = mx.gluon.loss.SoftmaxCrossEntropyLoss()
+
+for model_name in all_models:
+eprint('testing %s'%model_name)
+#data = mx.nd.random.uniform(shape=(100, 3, 224, 224))
+
+# This is to create a model and run the model once to initialize
+  

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-30 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164905297
 
 

 ##
 File path: src/operator/nn/fully_connected.cc
 ##
 @@ -23,58 +23,165 @@
  * \brief fully connect operator
 */
 #include "./fully_connected-inl.h"
+#include "./mkldnn/mkldnn_ops-inl.h"
+#include "./mkldnn/mkldnn_base-inl.h"
 #if MXNET_USE_NNPACK == 1
 #include "./nnpack/nnpack_fully_connected-inl.h"
 #endif  // MXNET_USE_NNPACK
 
 namespace mxnet {
 namespace op {
-template<>
-Operator* CreateOp(FullyConnectedParam param, int dtype,
-std::vector *in_shape,
-std::vector *out_shape,
-Context ctx) {
-  Operator *op = NULL;
-#if MXNET_USE_NNPACK == 1
-  const size_t batch_size = (*in_shape)[0][0];
-  // nnp_fully_connected_inference will do optimization for batch-size = 1
-  // nnp_fully_connected_output will do optimization for batch-size > 1
-  switch (dtype) {
-  case mshadow::kFloat32:
-return new NNPACKFullyConnectedOp(param);
-  default:
-break;
+
+static bool FullyConnectedShape(const nnvm::NodeAttrs& attrs,
+std::vector *in_shape,
+std::vector *out_shape) {
+  const FullyConnectedParam& param = 
nnvm::get(attrs.parsed);
+  using namespace mshadow;
+  if (!param.no_bias) {
+CHECK_EQ(in_shape->size(), 3U) << "Input:[data, weight, bias]";
+  } else {
+CHECK_EQ(in_shape->size(), 2U) << "Input:[data, weight]";
+  }
+  CHECK_EQ(out_shape->size(), 1U);
+  TShape dshape = (*in_shape)[fullc::kData];
+  TShape oshape = (*out_shape)[0];
+  // require data to be known
+  if (dshape.ndim() ==  0) return false;
+
+  index_t num_input;
+  if (!param.flatten) {
+num_input = dshape[dshape.ndim()-1];
+  } else {
+num_input = dshape.ProdShape(1, dshape.ndim());
+  }
+  SHAPE_ASSIGN_CHECK(*in_shape, fullc::kWeight, Shape2(param.num_hidden, 
num_input));
+  if (!param.no_bias) {
+SHAPE_ASSIGN_CHECK(*in_shape, fullc::kBias, Shape1(param.num_hidden));
+  }
+
+  if (!param.flatten) {
+TShape result_shape(dshape);
+result_shape[dshape.ndim()-1] = param.num_hidden;
+SHAPE_ASSIGN_CHECK(*out_shape, 0, result_shape);
+  } else {
+SHAPE_ASSIGN_CHECK(*out_shape, 0, Shape2(dshape[0], param.num_hidden));
+  }
+  if (oshape.ndim() != 0) {
+dshape[0] = oshape[0];
+SHAPE_ASSIGN_CHECK(*in_shape, fullc::kData, dshape);
+  }
+  return true;
+}
+
+void FullyConnectedCompute_CPU(const nnvm::NodeAttrs& attrs, const OpContext 
,
+const std::vector , const std::vector ,
+const std::vector ) {
+#if MXNET_USE_MKLDNN == 1
+  if (SupportMKLDNN(inputs[0])) {
+MKLDNNFCForward(attrs, ctx, inputs, req, outputs);
+return;
+  }
+#endif
+  std::vector in_blobs(inputs.size());
+  for (size_t i = 0; i < in_blobs.size(); i++)
+in_blobs[i] = inputs[i].data();
+  std::vector out_blobs(outputs.size());
+  for (size_t i = 0; i < out_blobs.size(); i++)
+out_blobs[i] = outputs[i].data();
+  FullyConnectedCompute(attrs, ctx, in_blobs, req, out_blobs);
+}
+
+void FullyConnectedGradCompute_CPU(const nnvm::NodeAttrs& attrs,
+const OpContext , const std::vector ,
+const std::vector , const std::vector ) {
+#if MXNET_USE_MKLDNN == 1
+  if (SupportMKLDNN(inputs[0])) {
+MKLDNNFCBackward(attrs, ctx, inputs, req, outputs);
+return;
   }
 #endif
-  switch (dtype) {
-  case mshadow::kFloat32:
-op = new FullyConnectedOp(param);
-break;
-  case mshadow::kFloat64:
-op = new FullyConnectedOp(param);
-break;
-  case mshadow::kFloat16:
-LOG(FATAL) << "float16 fully connected layer is currently"
-  "only supported by CuDNN version.";
-break;
-  default:
-LOG(FATAL) << "Unsupported type " << dtype;
+  std::vector in_blobs(inputs.size());
+  for (size_t i = 0; i < in_blobs.size(); i++)
+in_blobs[i] = inputs[i].data();
+  std::vector out_blobs(outputs.size());
+  for (size_t i = 0; i < out_blobs.size(); i++)
+out_blobs[i] = outputs[i].data();
+  FullyConnectedGradCompute(attrs, ctx, in_blobs, req, out_blobs);
+}
+
+static bool FullyConnectedType(const nnvm::NodeAttrs& attrs,
+   std::vector *in_type, std::vector 
*out_type) {
+  CHECK_GE(in_type->size(), 1U);
+  return ElemwiseAttr(
+  attrs, in_type, out_type, -1);
+}
+
+struct FullyConnectedGrad {
+  const char *op_name;
+  std::vector operator()(const nnvm::NodePtr& n,
+  const std::vector& 
ograds) const {
+std::vector heads(ograds.begin(), ograds.end());
+heads.push_back(n->inputs[fullc::kData]);
+heads.push_back(n->inputs[fullc::kWeight]);
+return MakeGradNode(op_name, n, heads, n->attrs.dict);
   }
+};
 
-  return op;
+inline static bool FCStorageType(const nnvm::NodeAttrs& attrs,
+ 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-30 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164904958
 
 

 ##
 File path: Makefile
 ##
 @@ -40,11 +40,11 @@ endif
 # use customized config file
 include $(config)
 
-ifeq ($(USE_MKL2017), 1)
-# must run ./prepare_mkl before including mshadow.mk
-   RETURN_STRING := $(shell ./prepare_mkl.sh $(MKLML_ROOT))
-   MKLROOT := $(firstword $(RETURN_STRING))
-   export USE_MKLML = $(lastword $(RETURN_STRING))
+ifeq ($(USE_MKLDNN), 1)
 
 Review comment:
   I have changed cmake to compile with MKLDNN.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-26 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164232807
 
 

 ##
 File path: src/kvstore/kvstore_dist.h
 ##
 @@ -32,11 +32,6 @@
 #include "mxnet/engine.h"
 #include "ps/ps.h"
 #include "./kvstore_dist_server.h"
-#if MKL_EXPERIMENTAL == 1
 
 Review comment:
   I tried `../../tools/launch.py --launcher local -n 3 python train_cifar10.py 
--kv-store dist_sync`. I let it run for a long time. It seems to converge well.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-26 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r164203240
 
 

 ##
 File path: src/kvstore/kvstore_dist.h
 ##
 @@ -32,11 +32,6 @@
 #include "mxnet/engine.h"
 #include "ps/ps.h"
 #include "./kvstore_dist_server.h"
-#if MKL_EXPERIMENTAL == 1
 
 Review comment:
   Thank you for your instructions. I run the test on a GPU instance and it 
seems to pass the test. It seems the test is specifically written for sparse 
formats and gradient compression.
   
   Currently, MKLDNN always uses the default layout for weight arrays during 
the training. I guess it may make more sense to train a model in a distributed 
environment?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-23 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r163404234
 
 

 ##
 File path: tests/python/gpu/test_gluon_model_zoo.py
 ##
 @@ -33,6 +33,8 @@ def download_data():
 'http://data.mxnet.io/data/val-5k-256.rec', VAL_DATA)
 
 def test_models():
+# We use network models without dropout for testing.
+# TODO(zhengda) mobilenet can't pass this test even without MKLDNN.
 all_models = ['resnet18_v1', 'densenet121', 'mobilenet1.0']
 
 Review comment:
   Does it run the model in the inference mode?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162523269
 
 

 ##
 File path: src/operator/tensor/elemwise_binary_op_basic.cc
 ##
 @@ -46,6 +103,41 @@ The storage type of ``elemwise_add`` output depends on 
storage types of inputs
 // this must differ from elemwise_add to prevent add to optimization in 
forward pass.
 MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU(_grad_add, 
op::mshadow_op::plus);
 
+static void _backward_ElemwiseAddEx(const nnvm::NodeAttrs& attrs,
+const OpContext& ctx,
+const std::vector& inputs,
+const std::vector& req,
+const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 2U);
+#if MXNET_USE_MKLDNN == 1
+  if (inputs[0].IsMKLDNNData()) {
+MKLDNNCopy(attrs, ctx, inputs[0], req[0], outputs[0]);
+MKLDNNCopy(attrs, ctx, inputs[0], req[1], outputs[1]);
+return;
+  }
+#endif
+  ElemwiseBinaryOp::BackwardUseNoneEx(
+  attrs, ctx, inputs, req, outputs);
+}
+
+static inline bool _backward_ElemwiseAddStorageType(const nnvm::NodeAttrs& 
attrs,
 
 Review comment:
   right. i need to change the 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162523057
 
 

 ##
 File path: src/operator/tensor/cast_storage.cc
 ##
 @@ -25,10 +25,50 @@
 #include "./cast_storage-inl.h"
 #include "../elemwise_op_common.h"
 #include "../tensor/elemwise_unary_op.h"
+#include "../nn/mkldnn/mkldnn_base-inl.h"
 
 namespace mxnet {
 namespace op {
 
+#if MXNET_USE_MKLDNN == 1
+
+static inline int get_type_size(int dtype) {
+  MSHADOW_TYPE_SWITCH(dtype, DType, {return sizeof(DType);});
+  return -1;
+}
+
+void CastStorageMKLDnsImpl(const OpContext& ctx, const NDArray& src, const 
NDArray _arr) {
+  TBlob dns = dst_arr.data();
+  CHECK_EQ(ctx.run_ctx.ctx.dev_mask(), Context::kCPU);
+  CHECK(src.shape() == dns.shape_);
+  if (src.dtype() != dns.type_flag_) {
+// If the input and output have different data types, we have to convert
+// the source array into the default layout, cast the data type and copy
+// data to the destination array.
+const TBlob _blob = src.data();
+CHECK(src.ctx() == dst_arr.ctx());
+ndarray::Copy(src.data(), , src.ctx(), dst_arr.ctx(), 
ctx.run_ctx);
+  } else {
+// This converts the source data to the default format and write the data 
to
+// the destination directly.
+std::vector net;
+auto src_mkldnn = src.GetMKLDNNData();
+auto src_pd = src_mkldnn->get_primitive_desc();
+auto def_format = GetDefaultFormat(src_pd.desc());
+if (def_format != src_pd.desc().data.format) {
+  auto dst_pd = GetPrimitiveDesc(src_pd, def_format);
+  mkldnn::memory dst_mkldnn(dst_pd, dns.dptr_);
+  net.push_back(mkldnn::reorder(*src_mkldnn, dst_mkldnn));
+  mkldnn::stream(mkldnn::stream::kind::eager).submit(net).wait();
+} else {
+  const TBlob _blob = src.data();
+  memcpy(dns.dptr_, src_blob.dptr_, src.shape().Size() * 
get_type_size(dns.type_flag_));
+}
+  }
+}
+
+#endif
+
 
 Review comment:
   good point. I guess we should use kFComputeEx for all storage type?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162522557
 
 

 ##
 File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
 ##
 @@ -0,0 +1,429 @@
+/*
+ * 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 2016-2017 Intel Corporation
+*
+* Licensed 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.
+*
+* \file mkldnn_base-inl.h
+* \brief
+* \author young.jin@intel.com
+* ashok.em...@intel.com
+* deepthi.kark...@intel.com
+* louis.f...@intel.com
+* adam.d.st...@intel.com
+* zhengda1...@gmail.com
+*
+***/
+
+#ifndef MXNET_OPERATOR_NN_MKLDNN_MKLDNN_BASE_INL_H_
+#define MXNET_OPERATOR_NN_MKLDNN_MKLDNN_BASE_INL_H_
+
+#if MXNET_USE_MKLDNN == 1
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "mkldnn.hpp"
+#include "mxnet/ndarray.h"
+#include "mxnet/resource.h"
+#include "mxnet/op_attr_types.h"
+using namespace mkldnn;
+namespace mxnet {
+extern bool EnableMkldnnWarnGenerated();
+// =  CpuEngine ===
+// cpu_engine singleton
+class CpuEngine {
+ public:
+  static CpuEngine *Get() {
+// I's thread-safe in C++11.
+static thread_local CpuEngine myInstance;
+return 
+  }
+  CpuEngine(CpuEngine const &) = delete; // Copy construct
+  CpuEngine(CpuEngine &&) = delete;  // Move construct
+  CpuEngine =(CpuEngine const &) = delete;  // Copy assign
+  CpuEngine =(CpuEngine &&) = delete;   // Move assign
+
+  mkldnn::engine _engine() { return _cpu_engine; }
+
+ protected:
+  CpuEngine() : _cpu_engine(mkldnn::engine::cpu, 0) {}
+  ~CpuEngine() {}
+
+ private:
+  mkldnn::engine _cpu_engine;
+};
+
+// type enumerator
+template 
+struct data_type_enum {};
+
+template <>
+struct data_type_enum {
+  enum { type = mkldnn::memory::data_type::f32 };
+};
+
+template <>
+struct data_type_enum {
+  enum { type = mkldnn::memory::data_type::s32 };
+};
+
+template <>
+struct data_type_enum {
+  enum { type = mkldnn::memory::data_type::s16 };
+};
+
+template <>
+struct data_type_enum {
+  enum { type = mkldnn::memory::data_type::s8 };
+};
+
+template <>
+struct data_type_enum {
+  enum { type = mkldnn::memory::data_type::u8 };
+};
+
+static inline bool SupportMKLDNNArray(int dtype, const TShape ) {
+  int ndim = shape.ndim();
+  bool support = ndim == 1 || ndim == 2 || ndim == 4;
+  support = support && (dtype == mshadow::kFloat32 || dtype == mshadow::kInt32
+|| dtype == mshadow::kInt8 || dtype == 
mshadow::kUint8);
+  return support;
+}
+
+static inline bool SupportStorageMKLDNN(int stype) {
+  return stype == kDefaultStorage;
+}
+
+static inline bool SupportMKLDNN(int dtype, const TShape ) {
+  int ndim = shape.ndim();
+  return dtype == mshadow::kFloat32 && (ndim == 1 || ndim == 2 || ndim == 4);
+}
+
+static inline bool SupportMKLDNN(const NDArray ) {
+  return SupportMKLDNN(input.dtype(), input.shape())
+  && SupportStorageMKLDNN(input.storage_type());
+}
+
+static inline bool SupportMKLDNNConv(const NDArray ) {
+  return input.dtype() == mshadow::kFloat32 && input.shape().ndim() == 4;
+}
+
+namespace op {
+struct ActivationParam;
+bool SupportMKLDNNAct(const op::ActivationParam& param);
+}
+
+static int GetTypeSize(int dtype) {
+  int size = -1;
+  MSHADOW_TYPE_SWITCH(dtype, DType, {
+size = sizeof(DType);
+  });
+  return size;
+}
+
+static inline size_t GetArraySize(const NDArray ) {
+  return arr.shape().Size() * 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162506606
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -184,11 +135,17 @@ class NDArray {
   const TBlob , const std::vector _data, int dev_id)
   : ptr_(std::make_shared(stype, data, aux_data, dev_id)), 
shape_(shape),
 dtype_(data.type_flag_), storage_type_(stype), entry_({nullptr, 0, 0}) 
{
-#if MKL_EXPERIMENTAL == 1
-Mkl_mem_ = std::make_shared();
-#endif
   }
 
+  inline bool IsView() const {
 
 Review comment:
   i'll add comments to explain why we need 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162502678
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -181,6 +322,408 @@ void NDArray::set_fresh_out_grad(bool state) const {
   info.fresh_out_grad = state;
 }
 
+#if MXNET_USE_MKLDNN == 1
+static inline bool same_shape(const TShape , mkldnn_dims_t dims, int 
ndims) {
+  if (shape.ndim() != (size_t)ndims)
+return false;
+  for (int i = 0; i < ndims; i++)
+if (shape[i] != dims[i])
+  return false;
+  return true;
+}
+
+static inline bool same_shape(const TShape , int dtype, 
mkldnn::memory::desc desc) {
+  return same_shape(shape, desc.data.dims, desc.data.ndims)
+  && get_mkldnn_type(dtype) == desc.data.data_type;
+}
+
+bool NDArray::Chunk::IsMKLDNN() const {
+  if (storage_type != kDefaultStorage)
+return false;
+  if (mkl_mem_ == nullptr)
+return false;
+  auto desc = mkl_mem_->get_primitive_desc().desc();
+  return desc.data.format != GetDefaultFormat(desc);
+}
+
+bool NDArray::Chunk::IsDefault() const {
+  if (storage_type != kDefaultStorage)
+return false;
+  // If we don't have mkldnn memory yet, we just assume it's not the default
+  // format.
+  if (mkl_mem_ == nullptr)
+return true;
+  auto desc = mkl_mem_->get_primitive_desc().desc();
+  return desc.data.format == GetDefaultFormat(desc);
+}
+
+void NDArray::Chunk::Reorder2Default() {
+  if (mkl_mem_ == nullptr)
+return;
+
+  auto format = GetDefaultFormat(mkl_mem_->get_primitive_desc().desc());
+  CHECK(format != mkl_mem_->get_primitive_desc().desc().data.format);
+
+  auto def_pd = GetPrimitiveDesc(mkl_mem_->get_primitive_desc(), format);
+  mkldnn_mem_ptr def_mem(new mkldnn::memory(def_pd));
+  // This may be called in MKLDNN operators. We can't use MKLDNNStream here.
+  std::vector net;
+  net.push_back(mkldnn::reorder(*mkl_mem_, *def_mem));
+  mkldnn::stream(mkldnn::stream::kind::eager).submit(net).wait();
+
+  CHECK(shandle.size >= def_pd.get_size());
+  CheckAndAlloc(def_pd.get_size());
+  // TODO(zhengda) We need to avoid memory copy here.
+  memcpy(shandle.dptr, def_mem->get_data_handle(), def_pd.get_size());
+  mkl_mem_.reset(new mkldnn::memory(def_pd, shandle.dptr));
+}
+
+void NDArray::Chunk::SetMKLMem(const TShape , int dtype) {
+  // The shape of the array and the one of the MKL memory may mismatch.
+  // For example, if the array stores parameters, the MKL memory may store data
+  // in 5 dimensions while the NDArray stores data in 4 dimensions.
+  if (mkl_mem_ && mkl_mem_->get_data_handle() == shandle.dptr
+  && same_shape(shape, dtype, mkl_mem_->get_primitive_desc().desc())) {
+return;
+  }
+
+  mkldnn::memory::dims dims;
+  // These are shapes supprted by MKLDNN.
+  if (shape.ndim() == 1 || shape.ndim() == 2 || shape.ndim() == 4
+  || shape.ndim() == 5) {
+dims.resize(shape.ndim());
+for (size_t i = 0; i < dims.size(); i++)
+  dims[i] = shape[i];
+  } else if (shape.ndim() == 3) {
+// If there are 3 dimensions, we'll force it to 4 dimensions.
+dims.resize(shape.ndim() + 1);
+dims[0] = 1;
+for (size_t i = 0; i < shape.ndim(); i++)
+  dims[i + 1] = shape[i];
+  } else {
+LOG(FATAL) << "MKLDNN doesn't support " << shape.ndim() << " dimensions";
+  }
+  mkldnn::memory::format layout = mkldnn::memory::format::format_undef;
+  switch (dims.size()) {
+case 1: layout = mkldnn::memory::format::x; break;
+case 2: layout = mkldnn::memory::format::nc; break;
+case 4: layout = mkldnn::memory::format::nchw; break;
+// This isn't the right layout when the data has 5 dimensions in MXNet.
+// MXNet interprets 5 dimensions as ncdhw, but MKLDNN doesn't have
+// a corresponding format.
+case 5: layout = mkldnn::memory::format::goihw; break;
+  }
+  mkldnn::memory::desc data_md{dims, get_mkldnn_type(dtype), layout};
+  auto cpu_engine = CpuEngine::Get()->get_engine();
+  if (shandle.dptr == nullptr) {
+CHECK(delay_alloc);
+CheckAndAlloc();
+  }
+  mkldnn::memory::primitive_desc pd(data_md, cpu_engine);
+  CHECK(shandle.size >= pd.get_size());
+  mkl_mem_.reset(new mkldnn::memory(pd, shandle.dptr));
+}
+
+/*
+ * Here we want to get MKLDNN memory whose primitive desc is exactly the same 
as
+ * the given one. operator== can't guarantee that. == can return true even if
+ * the formats are different. I need to double check its format.
+ */
+static inline mkldnn::memory *GetMKLDNNExact(
+const mkldnn::memory *mem, mkldnn::memory::primitive_desc desc) {
+  auto src_desc = mem->get_primitive_desc();
+  if (desc == src_desc && desc.desc().data.format == 
src_desc.desc().data.format) {
+return const_cast(mem);
+  } else {
+std::shared_ptr ret(new mkldnn::memory(
+desc, mem->get_data_handle()));
+MKLDNNStream::Get()->RegisterMem(ret);
+return ret.get();
+  }
+}
+
+const mkldnn::memory *NDArray::GetMKLDNNData(
+const 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162502091
 
 

 ##
 File path: src/operator/nn/mkldnn/mkldnn_sum.cc
 ##
 @@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file mkldnn_sum.cc
+ * \brief
+ * \author Da Zheng
+*/
+#include 
+
+#include "./mkldnn_ops-inl.h"
+#include "./mkldnn_base-inl.h"
+
+#if MXNET_USE_MKLDNN == 1
+namespace mxnet {
+namespace op {
+
+void Sum(const mkldnn::memory , const mkldnn::memory ,
+ const mkldnn::memory ) {
+  std::vector input_pds(2);
+  std::vector scales(2);
+  std::vector inputs;
+  input_pds[0] = arr1.get_primitive_desc();
+  input_pds[1] = arr2.get_primitive_desc();
+  CHECK(input_pds[0] == input_pds[1]);
+  scales[0] = 1;
+  scales[1] = 1;
+  inputs.push_back(arr1);
+  inputs.push_back(arr2);
+  // TODO(zhengda) I need to reorder memory here.
 
 Review comment:
   not yet.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162501336
 
 

 ##
 File path: src/operator/tensor/elemwise_unary_op_basic.cc
 ##
 @@ -108,12 +109,66 @@ 
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU(_backward_sigmoid,

unary_bwd);
 
 // copy
+static void CopyEx(const nnvm::NodeAttrs& attrs,
+   const OpContext& ctx,
+   const std::vector& inputs,
+   const std::vector& req,
+   const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  const auto in_stype = inputs[0].storage_type();
+  const auto out_stype = outputs[0].storage_type();
+#if MXNET_USE_MKLDNN == 1
+  if (inputs[0].IsMKLDNNData()) {
+MKLDNNCopy(attrs, ctx, inputs[0], req[0], outputs[0]);
+return;
+  } else if (in_stype == kDefaultStorage && out_stype == kDefaultStorage) {
+// This happens if inputs are supposed to be in MKLDNN format
+// but MKLDNN doesn't support the data type or the shape. We're
+// forced to convert it to the default format.
+std::vector in_blobs(1);
+std::vector out_blobs(1);
+in_blobs[0] = inputs[0].data();
+out_blobs[0] = outputs[0].data();
+UnaryOp::IdentityCompute(attrs, ctx, in_blobs, req, out_blobs);
+return;
+  }
+#endif
+  UnaryOp::IdentityComputeEx(attrs, ctx, inputs, req, outputs);
+}
+
+static inline bool CopyStorageType(const nnvm::NodeAttrs& attrs,
 
 Review comment:
   I'm not sure if ElemwiseStorageType should be used for other mkldnn ops. is 
it a right thing to do?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162501065
 
 

 ##
 File path: src/operator/tensor/matrix_op.cc
 ##
 @@ -180,6 +182,51 @@ If the argument `reverse` is set to 1, then the special 
values are inferred from
 .add_argument("data", "NDArray-or-Symbol", "Input data to reshape.")
 .add_arguments(ReshapeParam::__FIELDS__());
 
+static void FlattenEx(const nnvm::NodeAttrs& attrs,
+  const OpContext& ctx,
+  const std::vector& inputs,
+  const std::vector& req,
+  const std::vector& outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+#if MXNET_USE_MKLDNN == 1
+  const auto in_stype = inputs[0].storage_type();
+  const auto out_stype = outputs[0].storage_type();
+  if (inputs[0].IsMKLDNNData()) {
+MKLDNNCopy(attrs, ctx, inputs[0], req[0], outputs[0]);
+// If the output is a special MKLDNN layout and the number of dimensions
+// is larger than 2, we should use the default layout.
+if (outputs[0].IsMKLDNNData() && inputs[0].shape().ndim() > 2)
+  const_cast(outputs[0]).Reorder2Default();
+return;
+  } else {
+// This happens if inputs are supposed to be in MKLDNN format
+// but MKLDNN doesn't support the data type or the shape. We're
+// forced to convert it to the default format.
+FallBackCompute(UnaryOp::IdentityCompute, attrs, ctx, inputs, req, 
outputs);
 
 Review comment:
   it's defined in src/operator/nn/mkldnn/mkldnn_base-inl.h


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-18 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162432577
 
 

 ##
 File path: src/operator/nn/pooling.cc
 ##
 @@ -32,67 +33,304 @@
 #if MXNET_USE_NNPACK == 1
 #include "./nnpack/nnpack_pooling-inl.h"
 #endif  // MXNET_USE_NNPACK
+#if MXNET_USE_MKLDNN == 1
+#include "./mkldnn/mkldnn_pooling-inl.h"
+#endif  // MXNET_USE_MKLDNN
 
 namespace mxnet {
 namespace op {
 
-template<>
-Operator *CreateOp(PoolingParam param, int dtype) {
-  Operator *op = NULL;
-#if MXNET_USE_MKL2017 == 1
-if (param.kernel.ndim() == 2
-  && ((param.pool_type == pool_enum::kMaxPooling)
-  || (param.pool_type == pool_enum::kAvgPooling))) {
-  switch (dtype) {
-  case mshadow::kFloat32:
-return new MKLPoolingOp(param);
-  case mshadow::kFloat64:
-return new MKLPoolingOp(param);
-  default:
-break;
+static void PoolingParamParser(nnvm::NodeAttrs *attrs) {
+  using namespace mshadow;
+  PoolingParam param_;
+  param_.Init(attrs->dict);
+  if (param_.kernel.ndim() == 1) {
+if (param_.stride.ndim() == 0) param_.stride = Shape1(1);
+if (param_.pad.ndim() == 0) param_.pad = Shape1(0);
+  } else if (param_.kernel.ndim() == 2) {
+if (param_.stride.ndim() == 0) param_.stride = Shape2(1, 1);
+if (param_.pad.ndim() == 0) param_.pad = Shape2(0, 0);
+  } else {
+CHECK_EQ(param_.kernel.ndim(), 3U) << param_.kernel.ndim()
+   << "D pooling not supported";
+if (param_.stride.ndim() == 0) param_.stride = Shape3(1, 1, 1);
+if (param_.pad.ndim() == 0) param_.pad = Shape3(0, 0, 0);
+  }
+  CHECK_EQ(param_.stride.ndim(), param_.kernel.ndim())
+  << "stride and kernel should have the same length";
+  CHECK_EQ(param_.pad.ndim(), param_.kernel.ndim())
+  << "pad and kernel should have the same length";
+  attrs->parsed = std::move(param_);
+}
+
+int GetNumOutputs(const PoolingParam ) {
+#if MXNET_USE_MKLDNN == 1
+  return MKLDNNRequireWorkspace(param) && SupportMKLDNNPooling(param) ? 2 : 1;
+#else
+  return 1;
+#endif
+}
+
+int GetNumBackInputs(const PoolingParam ) {
+#if MXNET_USE_MKLDNN == 1
+  return MKLDNNRequireWorkspace(param) && SupportMKLDNNPooling(param) ? 5 : 3;
+#else
+  return 3;
+#endif
+}
+
+static bool PoolingType(const nnvm::NodeAttrs& attrs,
+std::vector *in_attrs,
+std::vector *out_attrs) {
+  out_attrs->at(0) = in_attrs->at(0);
+#if MXNET_USE_MKLDNN == 1
+  const PoolingParam  = nnvm::get(attrs.parsed);
+  if (MKLDNNRequireWorkspace(param) && SupportMKLDNNPooling(param)) {
+CHECK_GT(out_attrs->size(), 1U);
+out_attrs->at(1) = mshadow::kInt32;
+  }
+#endif
+  return true;
+}
+
+static bool PoolingShape(const nnvm::NodeAttrs ,
+ std::vector *in_shape,
+ std::vector *out_shape) {
+  const PoolingParam _ = nnvm::get(attrs.parsed);
+  CHECK_EQ(in_shape->size(), 1U);
+  const TShape  = (*in_shape)[0];
+  CHECK_GE(dshape.ndim(), 3U)
+  << "Pooling: Input data should be  3D in (batch, channel, x)"
+  << " Or 4D in (batch, channel, y, x) "
+  << " Or 5D in (batch, channel, d, y, x)";
+  TShape oshape = dshape;
+  if (dshape.ndim() == 0) return false;
+  if (param_.kernel.ndim() == 1) {
+CHECK_EQ(dshape.ndim(), 3U)
+<< "Pooling: Input data should be 3D in (batch, channel, x)";
+if (param_.global_pool) {
+  oshape[2] = 1;
+} else {
+  CHECK(param_.kernel[0] <= dshape[2] + 2 * param_.pad[0])
+  << "kernel size (" << param_.kernel[0] << ") exceeds input ("
+  << dshape[2] << " padded to " << (dshape[2] + 2 * param_.pad[0])
+  << ")";
+  if (param_.pooling_convention == pool_enum::kValid) {
+oshape[2] = 1 +
+(dshape[2] + 2 * param_.pad[0] - param_.kernel[0]) /
+param_.stride[0];
+  } else {
+oshape[2] = 1 + static_cast(ceil(
+static_cast(dshape[2] + 2 * param_.pad[0] -
+   param_.kernel[0]) /
+param_.stride[0]));
   }
 }
+out_shape->clear();
+out_shape->push_back(oshape);  // save output shape
+#if MXNET_USE_MKLDNN == 1
+if (MKLDNNRequireWorkspace(param_) && SupportMKLDNNPooling(param_))
+  out_shape->push_back(oshape);   // for workspace
 #endif
-#if MXNET_USE_NNPACK == 1
-  // NNPACK only support max-pooling with kernel = 2, stride = 2, 
pooling_convention
-  // = kFull(note that the default value is kValid in MXNet)
-  if ((param.pool_type == pool_enum::kMaxPooling)
-&& (param.pooling_convention == pool_enum::kFull)
-&& (param.kernel.ndim() == 2) && (param.stride.ndim() == 2)
-&& (param.kernel[0] == 2) && (param.kernel[1] == 2)
-&& (param.stride[0] == 2) && (param.stride[1] == 2)) {
-switch (dtype) {
-case 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-17 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162148807
 
 

 ##
 File path: src/operator/lrn.cc
 ##
 @@ -1,76 +0,0 @@
-/*
 
 Review comment:
   src/operator/nn/lrn.cc


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-17 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162141699
 
 

 ##
 File path: example/image-classification/common/data.py
 ##
 @@ -112,7 +112,8 @@ def get_rec_iter(args, kv=None):
 image_shape = tuple([int(l) for l in args.image_shape.split(',')])
 if 'benchmark' in args and args.benchmark:
 data_shape = (args.batch_size,) + image_shape
-train = SyntheticDataIter(args.num_classes, data_shape, 500, 
np.float32)
+train = SyntheticDataIter(args.num_classes, data_shape,
+args.num_examples / args.batch_size, np.float32)
 
 Review comment:
   Not really. It seems to me this is a bug, so I fixed 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-17 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r162132278
 
 

 ##
 File path: tests/python/unittest/test_executor.py
 ##
 @@ -140,22 +140,20 @@ def test_dot():
 
 def test_reshape():
 x = mx.sym.Variable('x')
-y = mx.sym.FullyConnected(x, num_hidden=4)
+y = mx.sym.Dropout(x, p=0.2)
 
 Review comment:
   it was modified because FullyConnected originally output an MKLDNN array. It 
didn't work with reshape. With the new design of NDArray, it's not a problem 
any more. I'll revert the changes 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161940072
 
 

 ##
 File path: src/storage/cpu_device_storage.h
 ##
 @@ -54,7 +54,11 @@ class CPUDeviceStorage {
   /*!
* \brief Alignment of allocation.
*/
+#if MXNET_USE_MKLDNN == 1
+  static constexpr size_t alignment_ = 4096;
 
 Review comment:
   MKLDNN requires memory to have alignment > 16. I'm not entirely sure what is 
the best alignment. The MKLDNN library uses 4096, so I use 4096.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161939043
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -611,6 +565,64 @@ class NDArray {
  << "CheckAndAllocAuxData is not intended for kDefaultStorage";
 ptr_->CheckAndAllocAuxData(i, aux_shape);
   }
+
+#if MXNET_USE_MKLDNN == 1
+  bool IsMKLDNN() const {
+return ptr_->IsMKLDNN();
+  }
+  bool IsDefault() const {
+return ptr_->IsDefault();
+  }
+  /*
+   * All functions below return a raw pointer to mkldnn memory. Actually there
+   * is a shared pointer that hold the memory either in NDArray or in MKLDNN
+   * stream. As long as we call these functions inside an operator, the return
+   * memory is always valid.
+   */
+
+  /*
+   * This function returns mkldnn::memory with the default primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNData() const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc
+   * as long as the array size meets the required size in the given 
primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNData(
+  const mkldnn::memory::primitive_desc ) const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc.
+   * The returned mkldnn::memory will have the same physical layout as
+   * the given primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNDataReorder(
+  const mkldnn::memory::primitive_desc ) const;
+
+  void CopyFrom(const mkldnn::memory );
+  mkldnn::memory *CreateMKLDNNData(
+  const mkldnn::memory::primitive_desc );
+
+  /*
+   * Reorder the memory to the specified layout.
+   */
+  void Reorder(const mkldnn::memory::primitive_desc );
+  void Reorder2Default() {
+CHECK_EQ(storage_type(), kDefaultStorage);
+ptr_->Reorder2Default();
+  }
+
+  void InvalidateData() {
+// When we invalidate data, we don't need to care about the MKLDNN format.
+ptr_->Mkl_mem_ = nullptr;
+  }
+
+  /*
+   * This function is used inside operators to reshape an array.
+   * It's used by FullyConnected right now.
+   */
+  NDArray ReshapeMKLDNN(const TShape ) const;
 
 Review comment:
   If the array stores data in a special layout, Reshape will cause the data in 
the array to be converted to the default layout, which allocates memory from 
malloc directly.
   ReshapeMKLDNN won't change the layout of the original array and always uses 
the temporary memory buffer to store the reordered data.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161937980
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -611,6 +565,64 @@ class NDArray {
  << "CheckAndAllocAuxData is not intended for kDefaultStorage";
 ptr_->CheckAndAllocAuxData(i, aux_shape);
   }
+
+#if MXNET_USE_MKLDNN == 1
+  bool IsMKLDNN() const {
+return ptr_->IsMKLDNN();
+  }
+  bool IsDefault() const {
+return ptr_->IsDefault();
+  }
+  /*
+   * All functions below return a raw pointer to mkldnn memory. Actually there
+   * is a shared pointer that hold the memory either in NDArray or in MKLDNN
+   * stream. As long as we call these functions inside an operator, the return
+   * memory is always valid.
+   */
+
+  /*
+   * This function returns mkldnn::memory with the default primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNData() const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc
+   * as long as the array size meets the required size in the given 
primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNData(
+  const mkldnn::memory::primitive_desc ) const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc.
+   * The returned mkldnn::memory will have the same physical layout as
+   * the given primitive_desc.
+   */
+  const mkldnn::memory *GetMKLDNNDataReorder(
+  const mkldnn::memory::primitive_desc ) const;
+
+  void CopyFrom(const mkldnn::memory );
+  mkldnn::memory *CreateMKLDNNData(
+  const mkldnn::memory::primitive_desc );
+
+  /*
+   * Reorder the memory to the specified layout.
+   */
+  void Reorder(const mkldnn::memory::primitive_desc );
+  void Reorder2Default() {
+CHECK_EQ(storage_type(), kDefaultStorage);
+ptr_->Reorder2Default();
+  }
+
+  void InvalidateData() {
+// When we invalidate data, we don't need to care about the MKLDNN format.
+ptr_->Mkl_mem_ = nullptr;
 
 Review comment:
   Maybe I should rename it as ResetMKLDNNData.
   Mkl_mem_ always references to the data in shandle and only provides the 
information of the data layout and data shape. removing Mkl_mem_ means the 
NDArray will store data in the default format.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161936090
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -534,15 +491,12 @@ class NDArray {
 CHECK_GE(ptr_->shandle.size,
  shape.Size() * mshadow::mshadow_sizeof(dtype))
 << "NDArray.AsArray: target memory size is bigger";
-#if MKL_EXPERIMENTAL == 1
-if (Mkl_mem_ != nullptr) {
-  // convert prv to cpu
-  Mkl_mem_->check_and_prv_to_cpu(ptr_->shandle.dptr);
-}
-#endif
+// We can't reuse memory in a view.
+CHECK(!IsView());
 NDArray ret = *this;
 ret.shape_ = shape;
 ret.dtype_ = dtype;
+ret.reuse_ = true;
 
 Review comment:
   I searched for it in the entire repo, including submodules, and only find 
three locations it's used:
   ```
   src/executor/graph_executor.cc:  data_entry_[i] = src.AsArray(vshape[i], 
vdtype[i]);
   src/imperative/imperative_utils.h:*arrays[i] = 
buff.AsArray(shapes[i], dtypes[i]);
   src/imperative/imperative_utils.h:*arrays[i] = 
arrays[mem_plan[i].sid]->AsArray(shapes[i], dtypes[i]);
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161921308
 
 

 ##
 File path: src/common/exec_utils.h
 ##
 @@ -43,19 +43,61 @@ namespace common {
   indices are not recorded
  * \return true if any source NDArray need to cast storage
  */
-inline bool SetupDefaultBlobs(const std::vector& src,
-  std::vector *blobs,
-  std::vector *temp_src,
-  std::vector *temp_dst,
-  std::unordered_map *idx_map 
= nullptr) {
+inline bool SetupDefaultBlobsIn(const std::vector& src,
+const std::vector *bufs,
+std::vector *blobs,
+std::vector *temp_src,
+std::vector *temp_dst,
+std::unordered_map 
*idx_map) {
   bool require_cast = false;
   for (size_t i = 0; i < src.size(); i++) {
 auto& nd = src[i];
-if (nd.storage_type() != kDefaultStorage) {
-  if (idx_map != nullptr) {
-(*idx_map)[i] = temp_dst->size();
-  }
-  NDArray temp(nd.shape(), nd.ctx(), false, nd.dtype());
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefault();
+#endif
+if (!is_default) {
+  (*idx_map)[i] = temp_dst->size();
+  NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(), 
nd.ctx(),
+ true, nd.dtype());
+#if MXNET_USE_MKLDNN == 1
+  CHECK(temp.IsDefault());
+#endif
+  temp_src->emplace_back(nd);
+  temp_dst->emplace_back(temp);
+  blobs->emplace_back(temp.data());
+  require_cast = true;
+} else {
+  blobs->push_back(nd.data());
+}
+  }
+  return require_cast;
+}
+
+inline bool SetupDefaultBlobsOut(const std::vector& src,
+ const std::vector ,
+ const std::vector *bufs,
+ std::vector *blobs,
+ std::vector *temp_src,
+ std::vector *temp_dst) {
+  bool require_cast = false;
+  for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// If it's writeTo, we don't need to worry whether it contains valid data.
+if (req[i] == kWriteTo && is_default)
 
 Review comment:
   The goal is to remove Mkl_ptr_ in the NDArray when an NDArray is reused.
   When we use WriteTo, the data in the output array shouldn't have any valid 
data. We should notify the NDArray of this. Otherwise, NDArray always thinks 
the data should be stored in a special layout.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-16 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161921308
 
 

 ##
 File path: src/common/exec_utils.h
 ##
 @@ -43,19 +43,61 @@ namespace common {
   indices are not recorded
  * \return true if any source NDArray need to cast storage
  */
-inline bool SetupDefaultBlobs(const std::vector& src,
-  std::vector *blobs,
-  std::vector *temp_src,
-  std::vector *temp_dst,
-  std::unordered_map *idx_map 
= nullptr) {
+inline bool SetupDefaultBlobsIn(const std::vector& src,
+const std::vector *bufs,
+std::vector *blobs,
+std::vector *temp_src,
+std::vector *temp_dst,
+std::unordered_map 
*idx_map) {
   bool require_cast = false;
   for (size_t i = 0; i < src.size(); i++) {
 auto& nd = src[i];
-if (nd.storage_type() != kDefaultStorage) {
-  if (idx_map != nullptr) {
-(*idx_map)[i] = temp_dst->size();
-  }
-  NDArray temp(nd.shape(), nd.ctx(), false, nd.dtype());
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// We have to make sure it's default storage and default layout.
+is_default = nd.IsDefault();
+#endif
+if (!is_default) {
+  (*idx_map)[i] = temp_dst->size();
+  NDArray temp = bufs != nullptr ? bufs->at(i) : NDArray(nd.shape(), 
nd.ctx(),
+ true, nd.dtype());
+#if MXNET_USE_MKLDNN == 1
+  CHECK(temp.IsDefault());
+#endif
+  temp_src->emplace_back(nd);
+  temp_dst->emplace_back(temp);
+  blobs->emplace_back(temp.data());
+  require_cast = true;
+} else {
+  blobs->push_back(nd.data());
+}
+  }
+  return require_cast;
+}
+
+inline bool SetupDefaultBlobsOut(const std::vector& src,
+ const std::vector ,
+ const std::vector *bufs,
+ std::vector *blobs,
+ std::vector *temp_src,
+ std::vector *temp_dst) {
+  bool require_cast = false;
+  for (size_t i = 0; i < src.size(); i++) {
+auto& nd = src[i];
+bool is_default = nd.storage_type() == kDefaultStorage;
+#if MXNET_USE_MKLDNN == 1
+// If it's writeTo, we don't need to worry whether it contains valid data.
+if (req[i] == kWriteTo && is_default)
 
 Review comment:
   The goal is to remove Mkl_ptr_ in the NDArray when an NDArray is reused.
   When we use WriteTo, the data in the output array shouldn't have any valid 
data. We should notify the NDArray of this. Otherwise, NDArray always thinks 
the data is stored in a special layout.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-15 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161619748
 
 

 ##
 File path: .gitmodules
 ##
 @@ -22,3 +22,7 @@
 [submodule "3rdparty/googletest"]
path = 3rdparty/googletest
url = https://github.com/google/googletest.git
+[submodule "3rdparty/mkldnn"]
+   path = 3rdparty/mkldnn
+   url = https://github.com/ashokei/mkl-dnn.git
 
 Review comment:
   that's right. It's mainly used for testing. The main repo of MKLDNN uses 
"curl" to download MKLML library and the Jenkins docker doesn't contain curl. I 
can't build a new docker image, as you know. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-15 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161619569
 
 

 ##
 File path: Jenkinsfile
 ##
 @@ -4,6 +4,7 @@
 
 // mxnet libraries
 mx_lib = 'lib/libmxnet.so, lib/libmxnet.a, dmlc-core/libdmlc.a, 
nnvm/lib/libnnvm.a'
+mx_mkldnn_lib = 'lib/libmxnet.so, lib/libmxnet.a, lib/libiomp5.so, 
lib/libmklml_gnu.so, lib/libmkldnn.so, lib/libmkldnn.so.0, 
lib/libmklml_intel.so, dmlc-core/libdmlc.a, nnvm/lib/libnnvm.a'
 
 Review comment:
   These libraries are provided by Intel people. They link the library in such 
a way. But you are right. Maybe we shouldn't link both Intel's OMP library and 
GNU OMP library.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2018-01-15 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r161619303
 
 

 ##
 File path: Jenkinsfile
 ##
 @@ -204,42 +205,40 @@ try {
 }
   }
 },
-'CPU: MKLML': {
+'CPU: MKLDNN': {
   node('mxnetlinux-cpu') {
-ws('workspace/build-mklml-cpu') {
+ws('workspace/build-mkldnn-cpu') {
   init_git()
   def flag = """ \
 DEV=1 \
 USE_PROFILER=1\
 USE_CPP_PACKAGE=1 \
 USE_BLAS=openblas \
-USE_MKL2017=1 \
-USE_MKL2017_EXPERIMENTAL=1\
+USE_MKLDNN=1  \
 -j\$(nproc)
 """
   make("cpu_mklml", flag)
-  pack_lib('mklml_cpu')
+  pack_lib('mkldnn_cpu', mx_mkldnn_lib)
 }
   }
 },
-'GPU: MKLML': {
+'GPU: MKLDNN': {
   node('mxnetlinux-cpu') {
-ws('workspace/build-mklml-gpu') {
+ws('workspace/build-mkldnn-gpu') {
   init_git()
   def flag = """ \
 DEV=1 \
 USE_PROFILER=1\
 USE_CPP_PACKAGE=1 \
 USE_BLAS=openblas \
-USE_MKL2017=1 \
-USE_MKL2017_EXPERIMENTAL=1\
+USE_MKLDNN=1  \
 USE_CUDA=1\
 USE_CUDA_PATH=/usr/local/cuda \
 USE_CUDNN=1   \
 -j\$(nproc)
 """
   make("build_cuda", flag)
 
 Review comment:
   it works fine. except that using MKLDNN and GPU in Jenkins still fails. It's 
not a linking problem. It's most likely caused by the conflict of MKLDNN and 
GPU in Jenkins.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156520457
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -64,17 +158,147 @@ nnvm::Symbol NDArray::get_autograd_symbol() const {
   return ret;
 }
 
+#if MXNET_USE_MKLDNN == 1
+
+static inline mkldnn_memory_format_t GetDefaultFormat(mkldnn::memory::desc 
desc) {
+  if (desc.data.ndims == 1) {
+return desc.data.format;
+  } else if (desc.data.ndims == 2) {
+if (desc.data.format == mkldnn_io)
+  return mkldnn_oi;
+else
+  return desc.data.format;
+  } else if (desc.data.ndims == 4) {
+switch (desc.data.format) {
+  case mkldnn_nchw:
+  case mkldnn_nhwc:
+  case mkldnn_chwn:
+  case mkldnn_nChw8c:
+  case mkldnn_nChw16c:
+return mkldnn_nchw;
+  case mkldnn_oihw:
+  case mkldnn_ihwo:
+  case mkldnn_hwio:
+  case mkldnn_OIhw8i8o:
+  case mkldnn_OIhw16i16o:
+  case mkldnn_OIhw8i16o2i:
+  case mkldnn_OIhw8o16i2o:
+  case mkldnn_OIhw8o8i:
+  case mkldnn_OIhw16o16i:
+  case mkldnn_IOhw16o16i:
+  case mkldnn_Oihw8o:
+  case mkldnn_Oihw16o:
+  case mkldnn_Ohwi8o:
+  case mkldnn_Ohwi16o:
+  case mkldnn_OhIw16o4i:
+return mkldnn_oihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else if (desc.data.ndims == 5) {
+switch (desc.data.format) {
+  case mkldnn_goihw:
+  case mkldnn_gOIhw8i8o:
+  case mkldnn_gOIhw16i16o:
+  case mkldnn_gOIhw8i16o2i:
+  case mkldnn_gOIhw8o16i2o:
+  case mkldnn_gOIhw8o8i:
+  case mkldnn_gOIhw16o16i:
+  case mkldnn_gIOhw16o16i:
+  case mkldnn_gOihw8o:
+  case mkldnn_gOihw16o:
+  case mkldnn_gOhwi8o:
+  case mkldnn_gOhwi16o:
+  case mkldnn_gOhIw16o4i:
+return mkldnn_goihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else {
+LOG(FATAL) << "Unsupported dimensions: " << desc.data.ndims;
+return mkldnn_format_undef;
+  }
+}
+
+static inline mkldnn_mem_ptr Reorder2Default(mkldnn_mem_ptr mem,
+ bool submit_now = true) {
+  auto format = GetDefaultFormat(mem->get_primitive_desc().desc());
+  if (format == mem->get_primitive_desc().desc().data.format)
+return mem;
+
+  auto pd = mem->get_primitive_desc();
+  mkldnn::memory::dims dims(pd.desc().data.ndims);
+  for (size_t i = 0; i < dims.size(); i++)
+dims[i] = pd.desc().data.dims[i];
+  mkldnn::memory::format cpp_format = 
static_cast(format);
+  mkldnn::memory::data_type cpp_type = static_cast(
+  pd.desc().data.data_type);
+  mkldnn::memory::desc data_md(dims, cpp_type, cpp_format);
+  mkldnn_mem_ptr def_mem(new 
mkldnn::memory(mkldnn::memory::primitive_desc(data_md,
+  pd.get_engine(;
+
+  MKLDNNStream  = MKLDNNStream::Instance();
+  stream.RegisterMem(mem);
+  stream.RegisterMem(def_mem);
+  stream.RegisterPrim(mkldnn::reorder(*mem, *def_mem));
+  if (submit_now)
+stream.Submit();
+  return def_mem;
+}
+
+NDArray NDArray::ReshapeMKLDNN(const TShape ) const {
+  CHECK(!is_none()) << "NDArray is not initialized";
+  CHECK_GE(shape_.Size(), shape.Size())
+<< "NDArray.Reshape: target shape size is larger current shape";
+  if (storage_type() == kDefaultStorage) {
+NDArray ret = this->Detach();
+ret.shape_ = shape;
+return ret;
+  } else if (storage_type() == kMKLDNNStorage) {
+NDArray ret(kMKLDNNStorage, shape, ctx(), ptr_->delay_alloc, dtype());
 
 Review comment:
   Actually, we should avoid allocating memory here. Should I always use true 
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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156519807
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -64,17 +158,147 @@ nnvm::Symbol NDArray::get_autograd_symbol() const {
   return ret;
 }
 
+#if MXNET_USE_MKLDNN == 1
+
+static inline mkldnn_memory_format_t GetDefaultFormat(mkldnn::memory::desc 
desc) {
+  if (desc.data.ndims == 1) {
+return desc.data.format;
+  } else if (desc.data.ndims == 2) {
+if (desc.data.format == mkldnn_io)
+  return mkldnn_oi;
+else
+  return desc.data.format;
+  } else if (desc.data.ndims == 4) {
+switch (desc.data.format) {
+  case mkldnn_nchw:
+  case mkldnn_nhwc:
+  case mkldnn_chwn:
+  case mkldnn_nChw8c:
+  case mkldnn_nChw16c:
+return mkldnn_nchw;
+  case mkldnn_oihw:
+  case mkldnn_ihwo:
+  case mkldnn_hwio:
+  case mkldnn_OIhw8i8o:
+  case mkldnn_OIhw16i16o:
+  case mkldnn_OIhw8i16o2i:
+  case mkldnn_OIhw8o16i2o:
+  case mkldnn_OIhw8o8i:
+  case mkldnn_OIhw16o16i:
+  case mkldnn_IOhw16o16i:
+  case mkldnn_Oihw8o:
+  case mkldnn_Oihw16o:
+  case mkldnn_Ohwi8o:
+  case mkldnn_Ohwi16o:
+  case mkldnn_OhIw16o4i:
+return mkldnn_oihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else if (desc.data.ndims == 5) {
+switch (desc.data.format) {
+  case mkldnn_goihw:
+  case mkldnn_gOIhw8i8o:
+  case mkldnn_gOIhw16i16o:
+  case mkldnn_gOIhw8i16o2i:
+  case mkldnn_gOIhw8o16i2o:
+  case mkldnn_gOIhw8o8i:
+  case mkldnn_gOIhw16o16i:
+  case mkldnn_gIOhw16o16i:
+  case mkldnn_gOihw8o:
+  case mkldnn_gOihw16o:
+  case mkldnn_gOhwi8o:
+  case mkldnn_gOhwi16o:
+  case mkldnn_gOhIw16o4i:
+return mkldnn_goihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else {
+LOG(FATAL) << "Unsupported dimensions: " << desc.data.ndims;
+return mkldnn_format_undef;
+  }
+}
+
+static inline mkldnn_mem_ptr Reorder2Default(mkldnn_mem_ptr mem,
+ bool submit_now = true) {
+  auto format = GetDefaultFormat(mem->get_primitive_desc().desc());
+  if (format == mem->get_primitive_desc().desc().data.format)
+return mem;
+
+  auto pd = mem->get_primitive_desc();
+  mkldnn::memory::dims dims(pd.desc().data.ndims);
+  for (size_t i = 0; i < dims.size(); i++)
+dims[i] = pd.desc().data.dims[i];
+  mkldnn::memory::format cpp_format = 
static_cast(format);
+  mkldnn::memory::data_type cpp_type = static_cast(
+  pd.desc().data.data_type);
+  mkldnn::memory::desc data_md(dims, cpp_type, cpp_format);
+  mkldnn_mem_ptr def_mem(new 
mkldnn::memory(mkldnn::memory::primitive_desc(data_md,
+  pd.get_engine(;
+
+  MKLDNNStream  = MKLDNNStream::Instance();
+  stream.RegisterMem(mem);
+  stream.RegisterMem(def_mem);
+  stream.RegisterPrim(mkldnn::reorder(*mem, *def_mem));
+  if (submit_now)
+stream.Submit();
+  return def_mem;
+}
+
+NDArray NDArray::ReshapeMKLDNN(const TShape ) const {
+  CHECK(!is_none()) << "NDArray is not initialized";
+  CHECK_GE(shape_.Size(), shape.Size())
+<< "NDArray.Reshape: target shape size is larger current shape";
+  if (storage_type() == kDefaultStorage) {
+NDArray ret = this->Detach();
+ret.shape_ = shape;
+return ret;
+  } else if (storage_type() == kMKLDNNStorage) {
+NDArray ret(kMKLDNNStorage, shape, ctx(), ptr_->delay_alloc, dtype());
+CHECK(ptr_->Mkl_mem_ != nullptr);
+// We shouldn't submit the reorder primitive here because submit will
+// be called in operators.
+ret.ptr_->Mkl_mem_ = Reorder2Default(ptr_->Mkl_mem_, false);
+return ret;
+  }
+  LOG(FATAL) << "Reshape for storage type " << storage_type() << " is not 
implemented yet";
+  return NDArray();
+}
+
+#endif
+
 NDArray NDArray::Reshape(const TShape ) const {
   CHECK(!is_none()) << "NDArray is not initialized";
-  auto stype = storage_type();
-  // reshape is not supported for non-default ndarray with dismatching shapes
-  CHECK((shape_ == shape) || stype == kDefaultStorage)
-<< "Reshape for storage type " << stype << " is not implemented yet";
   CHECK_GE(shape_.Size(), shape.Size())
 << "NDArray.Reshape: target shape size is larger current shape";
-  NDArray ret = this->Detach();
-  ret.shape_ = shape;
-  return ret;
+  if (storage_type() == kDefaultStorage) {
+NDArray ret = this->Detach();
+ret.shape_ = shape;
+return ret;
+#if MXNET_USE_MKLDNN == 1
+  } else if (storage_type() == kMKLDNNStorage) {
+NDArray ret(kMKLDNNStorage, shape, ctx(), ptr_->delay_alloc, dtype());
+// We need to convert the MKL memory to the default layout.
+

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156519245
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -64,17 +158,147 @@ nnvm::Symbol NDArray::get_autograd_symbol() const {
   return ret;
 }
 
+#if MXNET_USE_MKLDNN == 1
+
+static inline mkldnn_memory_format_t GetDefaultFormat(mkldnn::memory::desc 
desc) {
+  if (desc.data.ndims == 1) {
+return desc.data.format;
+  } else if (desc.data.ndims == 2) {
+if (desc.data.format == mkldnn_io)
+  return mkldnn_oi;
+else
+  return desc.data.format;
+  } else if (desc.data.ndims == 4) {
+switch (desc.data.format) {
+  case mkldnn_nchw:
+  case mkldnn_nhwc:
+  case mkldnn_chwn:
+  case mkldnn_nChw8c:
+  case mkldnn_nChw16c:
+return mkldnn_nchw;
+  case mkldnn_oihw:
+  case mkldnn_ihwo:
+  case mkldnn_hwio:
+  case mkldnn_OIhw8i8o:
+  case mkldnn_OIhw16i16o:
+  case mkldnn_OIhw8i16o2i:
+  case mkldnn_OIhw8o16i2o:
+  case mkldnn_OIhw8o8i:
+  case mkldnn_OIhw16o16i:
+  case mkldnn_IOhw16o16i:
+  case mkldnn_Oihw8o:
+  case mkldnn_Oihw16o:
+  case mkldnn_Ohwi8o:
+  case mkldnn_Ohwi16o:
+  case mkldnn_OhIw16o4i:
+return mkldnn_oihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else if (desc.data.ndims == 5) {
+switch (desc.data.format) {
+  case mkldnn_goihw:
+  case mkldnn_gOIhw8i8o:
+  case mkldnn_gOIhw16i16o:
+  case mkldnn_gOIhw8i16o2i:
+  case mkldnn_gOIhw8o16i2o:
+  case mkldnn_gOIhw8o8i:
+  case mkldnn_gOIhw16o16i:
+  case mkldnn_gIOhw16o16i:
+  case mkldnn_gOihw8o:
+  case mkldnn_gOihw16o:
+  case mkldnn_gOhwi8o:
+  case mkldnn_gOhwi16o:
+  case mkldnn_gOhIw16o4i:
+return mkldnn_goihw;
+  default:
+LOG(FATAL) << "Unknown MKLDNN format for 4 dimensions: " << 
desc.data.format;
+return mkldnn_format_undef;
+}
+  } else {
+LOG(FATAL) << "Unsupported dimensions: " << desc.data.ndims;
+return mkldnn_format_undef;
+  }
+}
+
+static inline mkldnn_mem_ptr Reorder2Default(mkldnn_mem_ptr mem,
+ bool submit_now = true) {
+  auto format = GetDefaultFormat(mem->get_primitive_desc().desc());
+  if (format == mem->get_primitive_desc().desc().data.format)
+return mem;
+
+  auto pd = mem->get_primitive_desc();
+  mkldnn::memory::dims dims(pd.desc().data.ndims);
+  for (size_t i = 0; i < dims.size(); i++)
+dims[i] = pd.desc().data.dims[i];
+  mkldnn::memory::format cpp_format = 
static_cast(format);
+  mkldnn::memory::data_type cpp_type = static_cast(
+  pd.desc().data.data_type);
+  mkldnn::memory::desc data_md(dims, cpp_type, cpp_format);
+  mkldnn_mem_ptr def_mem(new 
mkldnn::memory(mkldnn::memory::primitive_desc(data_md,
+  pd.get_engine(;
+
+  MKLDNNStream  = MKLDNNStream::Instance();
+  stream.RegisterMem(mem);
+  stream.RegisterMem(def_mem);
+  stream.RegisterPrim(mkldnn::reorder(*mem, *def_mem));
+  if (submit_now)
+stream.Submit();
+  return def_mem;
+}
+
+NDArray NDArray::ReshapeMKLDNN(const TShape ) const {
+  CHECK(!is_none()) << "NDArray is not initialized";
+  CHECK_GE(shape_.Size(), shape.Size())
+<< "NDArray.Reshape: target shape size is larger current shape";
+  if (storage_type() == kDefaultStorage) {
+NDArray ret = this->Detach();
+ret.shape_ = shape;
+return ret;
+  } else if (storage_type() == kMKLDNNStorage) {
+NDArray ret(kMKLDNNStorage, shape, ctx(), ptr_->delay_alloc, dtype());
+CHECK(ptr_->Mkl_mem_ != nullptr);
+// We shouldn't submit the reorder primitive here because submit will
+// be called in operators.
 
 Review comment:
   I'm not sure what you mean. We can't reshape on mkldnn memory if it stores 
data in a special format. In this case, the reshaped memory won't use the same 
memory. It's not a problem in this function, but it might be a problem in 
Reshape().


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156510303
 
 

 ##
 File path: src/operator/nn/activation-inl.h
 ##
 @@ -61,158 +62,127 @@ struct ActivationParam : public 
dmlc::Parameter {
   }
 };
 
-/**
- * \brief This is the implementation of activation operator.
- * \tparam xpu The device that the op will be executed on.
- */
 template
-class ActivationOp : public Operator {
- public:
-  virtual void Forward(const OpContext ,
-   const std::vector _data,
-   const std::vector ,
-   const std::vector _data,
-   const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(in_data.size(), 1U);
-CHECK_EQ(out_data.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& input = in_data[activation::kData];
-const size_t sz = input.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kOut], Req, {
-mxnet_op::Kernel, xpu>::Launch(
-  s, sz,
-  out_data[activation::kOut].dptr(),
-  input.dptr());
-  });
-}
+void ActivationForward(const OpContext , const TBlob _data,
+   const OpReqType , const TBlob _data) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = in_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, xpu>::Launch(
+s, sz,
+out_data.dptr(),
+in_data.dptr());
+});
   }
+}
 
-  virtual void Backward(const OpContext ,
-const std::vector _grad,
-const std::vector _data,
-const std::vector _data,
-const std::vector ,
-const std::vector _grad,
-const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(out_grad.size(), 1U);
-CHECK(in_data.size() == 1 && in_grad.size() == 1);
-CHECK_EQ(req.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& m_out_grad = out_grad[activation::kOut];
-const TBlob& m_out_data = out_data[activation::kOut];
-const TBlob&  m_in_grad = in_grad[activation::kData];
-const size_t sz = m_out_data.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kData], Req, {
-mxnet_op::Kernel, 
xpu>::Launch(
-  s, sz,
-  m_in_grad.dptr(),
-  m_out_grad.dptr(),
-  m_out_data.dptr());
-  });
-}
+template
+void ActivationBackward(const OpContext , const TBlob _grad,
+const TBlob _data, const OpReqType ,
+const TBlob _grad) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = out_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, 
xpu>::Launch(
+s, sz,
+in_grad.dptr(),
+out_grad.dptr(),
+out_data.dptr());
+});
   }
-};  // class ActivationOp
+}
 
-// Declare Factory function, used for dispatch specialization
 template
-Operator* CreateOp(ActivationParam type, int dtype, const TShape& dshape);
-
-#if DMLC_USE_CXX11
-class ActivationProp : public OperatorProperty {
- public:
-  void Init(const std::vector >& kwargs) 
override {
-param_.Init(kwargs);
-  }
-
-  std::map GetParams() const override {
-return param_.__DICT__();
-  }
-
-  bool InferShape(std::vector *in_shape,
-  std::vector *out_shape,
-  std::vector *aux_shape) const override {
-using namespace mshadow;
-CHECK_EQ(in_shape->size(), 1U) << "Input:[data]";
-const TShape  = in_shape->at(activation::kData);
-if (dshape.ndim() == 0) return false;
-out_shape->clear();
-out_shape->push_back(dshape);
-return true;
-  }
-
-  bool InferType(std::vector *in_type,
- std::vector *out_type,
- std::vector *aux_type) const override {
-CHECK_GE(in_type->size(), 1U);
-int dtype = (*in_type)[0];
-CHECK_NE(dtype, -1) << "First input must have specified type";
-for (index_t i = 0; i < in_type->size(); ++i) {
-  if ((*in_type)[i] == -1) {
-  (*in_type)[i] = dtype;
-  } else {
-UNIFORM_TYPE_CHECK((*in_type)[i], dtype, ListArguments()[i]);
-  }
+void _ActivationCompute(const ActivationParam , const OpContext ,
+const TBlob , OpReqType req, 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156511872
 
 

 ##
 File path: src/operator/nn/activation.cu
 ##
 @@ -31,39 +31,73 @@
 
 namespace mxnet {
 namespace op {
+
+#if MXNET_USE_CUDNN == 1
+
+template
+static CuDNNActivationOp _cudnn_op(const ActivationParam& param) {
+#if DMLC_CXX11_THREAD_LOCAL
+  static thread_local CuDNNActivationOp cudnn_op;
+#else
+  static MX_THREAD_LOCAL CuDNNActivationOp cudnn_op;
 
 Review comment:
   it won't pass the tests in github. i guess we can do it after we upgrade the 
tests?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156510825
 
 

 ##
 File path: src/operator/nn/activation.cc
 ##
 @@ -21,65 +21,128 @@
  * Copyright (c) 2015 by Contributors
  * \file activation.cc
  * \brief activation op
- * \author Bing Xu
+ * \author Bing Xu, Da Zheng
 */
 #include "./activation-inl.h"
 #include "../mshadow_op.h"
-#if MXNET_USE_MKL2017 == 1
-#include 
-#include "../mkl/mkl_memory-inl.h"
-#include "../mkl/mkl_relu-inl.h"
-#endif  // MXNET_USE_MKL2017
+#include "../tensor/elemwise_unary_op.h"
+#if MXNET_USE_MKLDNN == 1
+#include "./mkldnn/mkldnn_base-inl.h"
+#include "./mkldnn/mkldnn_act-inl.h"
+#endif  // MXNET_USE_MKLDNN
 
 namespace mxnet {
 namespace op {
-template<>
-Operator *CreateOp(ActivationParam param, int dtype, const TShape& 
dshape) {
-  Operator *op = NULL;
-#if MXNET_USE_MKL2017 == 1
-  if (param.act_type == activation::kReLU && dshape.ndim() <= 4) {
-  switch (dtype) {
-  case mshadow::kFloat32:
-  return new MKLReluOp();
-  case mshadow::kFloat64:
-  return new MKLReluOp();
-  default:
-  break;
-  }
+
+DMLC_REGISTER_PARAMETER(ActivationParam);
+
+// This will determine the order of the inputs for backward computation.
+struct ActivationGrad {
 
 Review comment:
   I see. I just copied and pasted...


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156510303
 
 

 ##
 File path: src/operator/nn/activation-inl.h
 ##
 @@ -61,158 +62,127 @@ struct ActivationParam : public 
dmlc::Parameter {
   }
 };
 
-/**
- * \brief This is the implementation of activation operator.
- * \tparam xpu The device that the op will be executed on.
- */
 template
-class ActivationOp : public Operator {
- public:
-  virtual void Forward(const OpContext ,
-   const std::vector _data,
-   const std::vector ,
-   const std::vector _data,
-   const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(in_data.size(), 1U);
-CHECK_EQ(out_data.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& input = in_data[activation::kData];
-const size_t sz = input.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kOut], Req, {
-mxnet_op::Kernel, xpu>::Launch(
-  s, sz,
-  out_data[activation::kOut].dptr(),
-  input.dptr());
-  });
-}
+void ActivationForward(const OpContext , const TBlob _data,
+   const OpReqType , const TBlob _data) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = in_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, xpu>::Launch(
+s, sz,
+out_data.dptr(),
+in_data.dptr());
+});
   }
+}
 
-  virtual void Backward(const OpContext ,
-const std::vector _grad,
-const std::vector _data,
-const std::vector _data,
-const std::vector ,
-const std::vector _grad,
-const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(out_grad.size(), 1U);
-CHECK(in_data.size() == 1 && in_grad.size() == 1);
-CHECK_EQ(req.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& m_out_grad = out_grad[activation::kOut];
-const TBlob& m_out_data = out_data[activation::kOut];
-const TBlob&  m_in_grad = in_grad[activation::kData];
-const size_t sz = m_out_data.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kData], Req, {
-mxnet_op::Kernel, 
xpu>::Launch(
-  s, sz,
-  m_in_grad.dptr(),
-  m_out_grad.dptr(),
-  m_out_data.dptr());
-  });
-}
+template
+void ActivationBackward(const OpContext , const TBlob _grad,
+const TBlob _data, const OpReqType ,
+const TBlob _grad) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = out_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, 
xpu>::Launch(
+s, sz,
+in_grad.dptr(),
+out_grad.dptr(),
+out_data.dptr());
+});
   }
-};  // class ActivationOp
+}
 
-// Declare Factory function, used for dispatch specialization
 template
-Operator* CreateOp(ActivationParam type, int dtype, const TShape& dshape);
-
-#if DMLC_USE_CXX11
-class ActivationProp : public OperatorProperty {
- public:
-  void Init(const std::vector >& kwargs) 
override {
-param_.Init(kwargs);
-  }
-
-  std::map GetParams() const override {
-return param_.__DICT__();
-  }
-
-  bool InferShape(std::vector *in_shape,
-  std::vector *out_shape,
-  std::vector *aux_shape) const override {
-using namespace mshadow;
-CHECK_EQ(in_shape->size(), 1U) << "Input:[data]";
-const TShape  = in_shape->at(activation::kData);
-if (dshape.ndim() == 0) return false;
-out_shape->clear();
-out_shape->push_back(dshape);
-return true;
-  }
-
-  bool InferType(std::vector *in_type,
- std::vector *out_type,
- std::vector *aux_type) const override {
-CHECK_GE(in_type->size(), 1U);
-int dtype = (*in_type)[0];
-CHECK_NE(dtype, -1) << "First input must have specified type";
-for (index_t i = 0; i < in_type->size(); ++i) {
-  if ((*in_type)[i] == -1) {
-  (*in_type)[i] = dtype;
-  } else {
-UNIFORM_TYPE_CHECK((*in_type)[i], dtype, ListArguments()[i]);
-  }
+void _ActivationCompute(const ActivationParam , const OpContext ,
+const TBlob , OpReqType req, 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156509390
 
 

 ##
 File path: src/operator/nn/activation-inl.h
 ##
 @@ -61,158 +62,127 @@ struct ActivationParam : public 
dmlc::Parameter {
   }
 };
 
-/**
- * \brief This is the implementation of activation operator.
- * \tparam xpu The device that the op will be executed on.
- */
 template
-class ActivationOp : public Operator {
- public:
-  virtual void Forward(const OpContext ,
-   const std::vector _data,
-   const std::vector ,
-   const std::vector _data,
-   const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(in_data.size(), 1U);
-CHECK_EQ(out_data.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& input = in_data[activation::kData];
-const size_t sz = input.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kOut], Req, {
-mxnet_op::Kernel, xpu>::Launch(
-  s, sz,
-  out_data[activation::kOut].dptr(),
-  input.dptr());
-  });
-}
+void ActivationForward(const OpContext , const TBlob _data,
+   const OpReqType , const TBlob _data) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = in_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, xpu>::Launch(
+s, sz,
+out_data.dptr(),
+in_data.dptr());
+});
   }
+}
 
-  virtual void Backward(const OpContext ,
-const std::vector _grad,
-const std::vector _data,
-const std::vector _data,
-const std::vector ,
-const std::vector _grad,
-const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(out_grad.size(), 1U);
-CHECK(in_data.size() == 1 && in_grad.size() == 1);
-CHECK_EQ(req.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& m_out_grad = out_grad[activation::kOut];
-const TBlob& m_out_data = out_data[activation::kOut];
-const TBlob&  m_in_grad = in_grad[activation::kData];
-const size_t sz = m_out_data.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kData], Req, {
-mxnet_op::Kernel, 
xpu>::Launch(
-  s, sz,
-  m_in_grad.dptr(),
-  m_out_grad.dptr(),
-  m_out_data.dptr());
-  });
-}
+template
+void ActivationBackward(const OpContext , const TBlob _grad,
+const TBlob _data, const OpReqType ,
+const TBlob _grad) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = out_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, 
xpu>::Launch(
+s, sz,
+in_grad.dptr(),
+out_grad.dptr(),
+out_data.dptr());
+});
   }
-};  // class ActivationOp
+}
 
-// Declare Factory function, used for dispatch specialization
 template
-Operator* CreateOp(ActivationParam type, int dtype, const TShape& dshape);
-
-#if DMLC_USE_CXX11
-class ActivationProp : public OperatorProperty {
- public:
-  void Init(const std::vector >& kwargs) 
override {
-param_.Init(kwargs);
-  }
-
-  std::map GetParams() const override {
-return param_.__DICT__();
-  }
-
-  bool InferShape(std::vector *in_shape,
-  std::vector *out_shape,
-  std::vector *aux_shape) const override {
-using namespace mshadow;
-CHECK_EQ(in_shape->size(), 1U) << "Input:[data]";
-const TShape  = in_shape->at(activation::kData);
-if (dshape.ndim() == 0) return false;
-out_shape->clear();
-out_shape->push_back(dshape);
-return true;
-  }
-
-  bool InferType(std::vector *in_type,
- std::vector *out_type,
- std::vector *aux_type) const override {
-CHECK_GE(in_type->size(), 1U);
-int dtype = (*in_type)[0];
-CHECK_NE(dtype, -1) << "First input must have specified type";
-for (index_t i = 0; i < in_type->size(); ++i) {
-  if ((*in_type)[i] == -1) {
-  (*in_type)[i] = dtype;
-  } else {
-UNIFORM_TYPE_CHECK((*in_type)[i], dtype, ListArguments()[i]);
-  }
+void _ActivationCompute(const ActivationParam , const OpContext ,
+const TBlob , OpReqType req, 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156509390
 
 

 ##
 File path: src/operator/nn/activation-inl.h
 ##
 @@ -61,158 +62,127 @@ struct ActivationParam : public 
dmlc::Parameter {
   }
 };
 
-/**
- * \brief This is the implementation of activation operator.
- * \tparam xpu The device that the op will be executed on.
- */
 template
-class ActivationOp : public Operator {
- public:
-  virtual void Forward(const OpContext ,
-   const std::vector _data,
-   const std::vector ,
-   const std::vector _data,
-   const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(in_data.size(), 1U);
-CHECK_EQ(out_data.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& input = in_data[activation::kData];
-const size_t sz = input.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kOut], Req, {
-mxnet_op::Kernel, xpu>::Launch(
-  s, sz,
-  out_data[activation::kOut].dptr(),
-  input.dptr());
-  });
-}
+void ActivationForward(const OpContext , const TBlob _data,
+   const OpReqType , const TBlob _data) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = in_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, xpu>::Launch(
+s, sz,
+out_data.dptr(),
+in_data.dptr());
+});
   }
+}
 
-  virtual void Backward(const OpContext ,
-const std::vector _grad,
-const std::vector _data,
-const std::vector _data,
-const std::vector ,
-const std::vector _grad,
-const std::vector _args) {
-using namespace mshadow;
-using namespace mshadow::expr;
-CHECK_EQ(out_grad.size(), 1U);
-CHECK(in_data.size() == 1 && in_grad.size() == 1);
-CHECK_EQ(req.size(), 1U);
-Stream *s = ctx.get_stream();
-const TBlob& m_out_grad = out_grad[activation::kOut];
-const TBlob& m_out_data = out_data[activation::kOut];
-const TBlob&  m_in_grad = in_grad[activation::kData];
-const size_t sz = m_out_data.shape_.Size();
-if (sz) {
-  MXNET_ASSIGN_REQ_SWITCH(req[activation::kData], Req, {
-mxnet_op::Kernel, 
xpu>::Launch(
-  s, sz,
-  m_in_grad.dptr(),
-  m_out_grad.dptr(),
-  m_out_data.dptr());
-  });
-}
+template
+void ActivationBackward(const OpContext , const TBlob _grad,
+const TBlob _data, const OpReqType ,
+const TBlob _grad) {
+  using namespace mshadow;
+  using namespace mshadow::expr;
+  Stream *s = ctx.get_stream();
+  const size_t sz = out_data.shape_.Size();
+  if (sz) {
+MXNET_ASSIGN_REQ_SWITCH(req, Req, {
+  mxnet_op::Kernel, 
xpu>::Launch(
+s, sz,
+in_grad.dptr(),
+out_grad.dptr(),
+out_data.dptr());
+});
   }
-};  // class ActivationOp
+}
 
-// Declare Factory function, used for dispatch specialization
 template
-Operator* CreateOp(ActivationParam type, int dtype, const TShape& dshape);
-
-#if DMLC_USE_CXX11
-class ActivationProp : public OperatorProperty {
- public:
-  void Init(const std::vector >& kwargs) 
override {
-param_.Init(kwargs);
-  }
-
-  std::map GetParams() const override {
-return param_.__DICT__();
-  }
-
-  bool InferShape(std::vector *in_shape,
-  std::vector *out_shape,
-  std::vector *aux_shape) const override {
-using namespace mshadow;
-CHECK_EQ(in_shape->size(), 1U) << "Input:[data]";
-const TShape  = in_shape->at(activation::kData);
-if (dshape.ndim() == 0) return false;
-out_shape->clear();
-out_shape->push_back(dshape);
-return true;
-  }
-
-  bool InferType(std::vector *in_type,
- std::vector *out_type,
- std::vector *aux_type) const override {
-CHECK_GE(in_type->size(), 1U);
-int dtype = (*in_type)[0];
-CHECK_NE(dtype, -1) << "First input must have specified type";
-for (index_t i = 0; i < in_type->size(); ++i) {
-  if ((*in_type)[i] == -1) {
-  (*in_type)[i] = dtype;
-  } else {
-UNIFORM_TYPE_CHECK((*in_type)[i], dtype, ListArguments()[i]);
-  }
+void _ActivationCompute(const ActivationParam , const OpContext ,
+const TBlob , OpReqType req, 

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-12 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156508822
 
 

 ##
 File path: src/ndarray/ndarray.cc
 ##
 @@ -95,12 +319,36 @@ NDArray NDArray::ReshapeWithRecord(const TShape ) {
   return ret;
 }
 
-
 NDArray NDArray::Slice(index_t begin, index_t end) const {
   CHECK(!is_none()) << "NDArray is empty";
   CHECK_LE(begin, end)
   << "Invalid slicing range [" << begin << ", " << end << ")";
   CHECK_GE(shape_[0], end) << "Slice end index out of range";
+#if MXNET_USE_MKLDNN == 1
+  CHECK(storage_type() == kDefaultStorage || storage_type() == kMKLDNNStorage);
 
 Review comment:
   Slice doesn't work on sparse arrays.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-11 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156240694
 
 

 ##
 File path: src/common/utils.h
 ##
 @@ -327,17 +349,7 @@ inline std::string dispatch_mode_string(const 
DispatchMode x) {
 
 
 /*! \brief get string representation of storage_type */
-inline std::string stype_string(const int x) {
-  switch (x) {
-case kDefaultStorage:
-  return "default";
-case kCSRStorage:
-  return "csr";
-case kRowSparseStorage:
-  return "row_sparse";
-  }
-  return "unknown";
-}
+std::string stype_string(const int x);
 
 Review comment:
   there isn't a big reason for this. I didn't want to compile everything 
whenever I change a header file, so I moved the definition of the function in 
the .cc file. I'll move the code back.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-11 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156240271
 
 

 ##
 File path: python/mxnet/ndarray/sparse.py
 ##
 @@ -1138,6 +1139,8 @@ def _ndarray_cls(handle, writable=True, 
stype=_STORAGE_TYPE_UNDEFINED):
 stype = _storage_type(handle)
 if stype == _STORAGE_TYPE_DEFAULT:
 return NDArray(handle, writable=writable)
+elif stype == _STORAGE_TYPE_MKLDNN:
+return NDArray(handle, writable=writable)
 
 Review comment:
   right. i wanted to delete MKLNDArray since we don't want to expose MKLDNN to 
users. But we still want users to specify MKLDNN storage during the inference, 
so I kept the code as it was. We need to discuss how we allow users to specify 
MKLDNN storage.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-11 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156219864
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -608,6 +549,37 @@ class NDArray {
  << "CheckAndAllocAuxData is not intended for kDefaultStorage";
 ptr_->CheckAndAllocAuxData(i, aux_shape);
   }
+
+#if MXNET_USE_MKLDNN == 1
+  /*
+   * This function returns mkldnn::memory with the default primitive_desc.
+   */
+  std::shared_ptr GetMKLDNNData() const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc
+   * as long as the array size meets the required size in the given 
primitive_desc.
+   */
+  std::shared_ptr GetMKLDNNData(
+  const mkldnn::memory::primitive_desc ) const;
+  /*
+   * This function returns mkldnn::memory with the given primitive_desc.
+   * The returned mkldnn::memory will have the same physical layout as
+   * the given primitive_desc.
+   */
+  std::shared_ptr GetMKLDNNDataReorder(
+  const mkldnn::memory::primitive_desc ) const;
+
+  void CopyFrom(const mkldnn::memory );
+  std::shared_ptr CreateMKLDNNData(
+  const mkldnn::memory::primitive_desc );
+
+  /*
+   * This function is used inside operators to reshape an array.
+   * It's used by FullyConnected right now.
 
 Review comment:
   FullyConnected needs to reshape an array to 2D. The current implementation 
of Reshape for MKLDNN hangs if I call it inside an operator, because I call 
WaitToRead() in the end. Could you help me check if the implementation of 
Reshape is correct/necessary?
   
https://github.com/zheng-da/incubator-mxnet/blob/refactor/src/ndarray/ndarray.cc#L282


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-11 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r156218843
 
 

 ##
 File path: python/mxnet/ndarray/mkldnn.py
 ##
 @@ -0,0 +1,87 @@
+# 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.
+
+# coding: utf-8
+# pylint: disable=wildcard-import, unused-wildcard-import, too-many-lines
+
+"""MKLDNN NDArray API of MXNet."""
+
+from __future__ import absolute_import
+from __future__ import division
+
+import warnings
+
+__all__ = ["MKLNDArray"]
+
+from ..context import Context
+from . import _internal
+from . import op
+from .ndarray import NDArray
+from .sparse import _ndarray_cls
+
+class MKLNDArray(NDArray):
 
 Review comment:
   probably not. I'll remove the class.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155914245
 
 

 ##
 File path: src/executor/infer_graph_attr_pass.cc
 ##
 @@ -416,11 +416,6 @@ nnvm::Graph InferStorageType(nnvm::Graph&& graph,
 DispatchModeVector dispatch_modes(graph.indexed_graph().num_nodes(), 
DispatchMode::kUndefined);
 graph.attrs["dispatch_mode"] = 
std::make_shared(std::move(dispatch_modes));
   }
-  // initialize unknown values for dispatch modes
 
 Review comment:
   i remember now. because i found there is duplicated code here. The code I 
delete is exactly the same as the code above.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155907169
 
 

 ##
 File path: src/operator/nn/convolution.cc
 ##
 @@ -38,63 +36,361 @@ namespace mxnet {
 namespace op {
 DMLC_REGISTER_PARAMETER(ConvolutionParam);
 
-template<>
-Operator* CreateOp(ConvolutionParam param, int dtype,
-std::vector *in_shape,
-std::vector *out_shape,
-Context ctx) {
-  Operator *op = NULL;
-  // If 1D convolution, use MXNet implementation
-  if (param.kernel.ndim() == 1) {
-MSHADOW_REAL_TYPE_SWITCH(dtype, DType, {
-  op = new ConvolutionOp(param);
-})
-return op;
+static inline index_t AddPad(index_t dsize, index_t pad) {
+  return dsize + 2 * pad;
+}
+
+static inline std::vector ListArguments(const ConvolutionParam& 
param_) {
+  if (!param_.no_bias) {
+return {"data", "weight", "bias"};
+  } else {
+return {"data", "weight"};
   }
-#if MXNET_USE_MKL2017 == 1
-  if ((param.dilate[0] == 1 && param.dilate[1] == 1)
-  && param.kernel.ndim() == 2) {
-switch (dtype) {
-case mshadow::kFloat32:
-  return new MKLConvolutionOp(param);
-case mshadow::kFloat64:
-  return new MKLConvolutionOp(param);
-default:
-  break;
-}
+}
+
+static void ConvolutionCompute_CPU(const nnvm::NodeAttrs& attrs,
+const OpContext& ctx, const std::vector& inputs,
+const std::vector& req, const std::vector& outputs) {
+#if MXNET_USE_MKLDNN == 1
+  if (SupportMKLDNNConv(inputs[0])) {
+MKLDNNConvolution_Forward(attrs, ctx, inputs, req, outputs);
+return;
   }
 #endif
-#if MXNET_USE_NNPACK == 1
-  const size_t batch_size = (*in_shape)[0][0];
-  if ((param.dilate[0] == 1 && param.dilate[1] == 1)
-  && param.kernel.ndim() == 2 && (!param.no_bias)
-  && param.num_group == 1 && (batch_size == 1 ||
-  ((batch_size > 1) && (param.stride[0] == 1) &&
-  (param.stride[1] == 1 {
-switch (dtype) {
-case mshadow::kFloat32:
-  return new NNPACKConvolutionOp(param);
-default:
-  break;
+  std::vector in_blobs(inputs.size());
+  for (size_t i = 0; i < in_blobs.size(); i++)
+in_blobs[i] = inputs[i].data();
+  std::vector out_blobs(outputs.size());
+  for (size_t i = 0; i < out_blobs.size(); i++)
+out_blobs[i] = outputs[i].data();
+  ConvolutionCompute(attrs, ctx, in_blobs, req, out_blobs);
+}
+
+static void ConvolutionGradCompute_CPU(const nnvm::NodeAttrs& attrs,
+const OpContext& ctx, const std::vector& inputs,
+const std::vector& req, const std::vector& outputs) {
+#if MXNET_USE_MKLDNN == 1
+  if (SupportMKLDNNConv(inputs[0])) {
+MKLDNNConvolution_Backward(attrs, ctx, inputs, req, outputs);
+return;
+  }
+#endif
+  std::vector in_blobs(inputs.size());
+  for (size_t i = 0; i < in_blobs.size(); i++)
+in_blobs[i] = inputs[i].data();
+  std::vector out_blobs(outputs.size());
+  for (size_t i = 0; i < out_blobs.size(); i++)
+out_blobs[i] = outputs[i].data();
+  ConvolutionGradCompute(attrs, ctx, in_blobs, req, out_blobs);
+}
+
+static bool ConvolutionShape(const nnvm::NodeAttrs& attrs,
+ std::vector *in_shape,
+ std::vector *out_shape) {
+  using namespace mshadow;
+  const ConvolutionParam& param_ = nnvm::get(attrs.parsed);
+  if (!param_.no_bias) {
+CHECK_EQ(in_shape->size(), 3U) << "Input:[data, weight, bias]";
+  } else {
+CHECK_EQ(in_shape->size(), 2U) << "Input:[data, weight]";
+  }
+  // CHECK_EQ(out_shape->size(), 1) << "Output: [output]";
+  out_shape->resize(1, TShape());
+  const TShape  = (*in_shape)[conv::kData];
+  if (dshp.ndim() ==  0) return false;
+
+  if (param_.kernel.ndim() == 1) {
+// 1d conv
+CHECK_EQ(dshp.ndim(), 3U) << "Input data should be 3D in 
batch-num_filter-x";
+Shape<3> dshape = ConvertLayout(dshp.get<3>(), param_.layout.value(), 
kNCW);
+Shape<3> wshape = Shape3(param_.num_filter / param_.num_group, dshape[1] / 
param_.num_group,
+param_.kernel[0]);
+wshape = ConvertLayout(wshape, kNCW, param_.layout.value());
+wshape[0] *= param_.num_group;
+SHAPE_ASSIGN_CHECK(*in_shape, conv::kWeight, wshape);
+if (!param_.no_bias) {
+  SHAPE_ASSIGN_CHECK(*in_shape, conv::kBias, Shape1(param_.num_filter));
+}
+
+const index_t dilated_ksize_x = param_.DilatedKernelSize(0);
+CHECK_EQ(dshape[1] % param_.num_group, 0U) \
+  << "input num_filter must divide group size";
+CHECK_EQ(param_.num_filter % param_.num_group, 0U) \
+  << "output num_filter must divide group size";
+CHECK_GT(param_.kernel.Size(), 0U) \
+  << "incorrect kernel size: " << param_.kernel;
+CHECK_GT(param_.stride.Size(), 0U) \
+  << "incorrect stride size: " << param_.stride;
+CHECK_GT(param_.dilate.Size(), 0U) \
+  << "incorrect dilate size: " << param_.dilate;

[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155907018
 
 

 ##
 File path: src/kvstore/kvstore_dist.h
 ##
 @@ -32,11 +32,6 @@
 #include "mxnet/engine.h"
 #include "ps/ps.h"
 #include "./kvstore_dist_server.h"
-#if MKL_EXPERIMENTAL == 1
 
 Review comment:
   i haven't tried it. Where is the distributed training 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155851393
 
 

 ##
 File path: src/kvstore/kvstore_local.h
 ##
 @@ -256,7 +256,13 @@ class KVStoreLocal : public KVStore {
 auto validator = [this](const int key, const NDArray& nd) -> bool {
   auto stype = nd.storage_type();
   // valid NDArray
-  if (stype == kDefaultStorage || stype == kRowSparseStorage) return true;
+  if (stype == kDefaultStorage || stype == kRowSparseStorage
 
 Review comment:
   right. I have updated 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] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155850266
 
 

 ##
 File path: src/executor/graph_executor.cc
 ##
 @@ -54,6 +54,14 @@ GraphExecutor::~GraphExecutor() {
   }
 }
 
+inline bool SharableStorage(NDArrayStorageType stype) {
 
 Review comment:
   it's pass by value. I guess it doesn't matter that much?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155850103
 
 

 ##
 File path: python/mxnet/ndarray/mkldnn.py
 ##
 @@ -0,0 +1,113 @@
+# 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.
+
+# coding: utf-8
+# pylint: disable=wildcard-import, unused-wildcard-import, too-many-lines
+
+"""MKLDNN NDArray API of MXNet."""
+
+from __future__ import absolute_import
+from __future__ import division
+try:
+from __builtin__ import slice as py_slice
+from __builtin__ import sum as py_sum
+except ImportError:
+from builtins import slice as py_slice
+from builtins import sum as py_sum
+
+import ctypes
+import warnings
+
+__all__ = ["_ndarray_cls", "MKLNDArray"]
+
+import numpy as np
+from ..base import _LIB, numeric_types
+from ..base import c_array, mx_real_t, integer_types
+from ..base import mx_uint, NDArrayHandle, check_call
+from ..context import Context
+from . import _internal
+from . import op
+from ._internal import _set_ndarray_class
+from .ndarray import NDArray, _storage_type, _DTYPE_NP_TO_MX, _DTYPE_MX_TO_NP
+from .ndarray import _STORAGE_TYPE_STR_TO_ID, _STORAGE_TYPE_MKLDNN
+from .ndarray import _STORAGE_TYPE_UNDEFINED, _STORAGE_TYPE_DEFAULT
+from .ndarray import zeros as _zeros_ndarray
+from .ndarray import array as _array
+
+class MKLNDArray(NDArray):
+"""The base class of an NDArray stored in a MKLDNN storage format.
+"""
+
+def __repr__(self):
+"""Returns a string representation of the sparse array."""
+shape_info = 'x'.join(['%d' % x for x in self.shape])
+# The data content is not displayed since the array usually has big 
shape
+return '\n<%s %s @%s>' % (self.__class__.__name__,
+  shape_info, self.context)
+
+# TODO
+def _at(self, idx):
+raise NotSupportedForMKLNDArray(self._at, '[idx]', idx)
+
+def _slice(self, start, stop):
+return op.slice(self, begin=start, end=stop)
+
+# TODO
+def astype(self, dtype):
+"""Returns a copy of the array after casting to a specified type.
+Parameters
+--
+dtype : numpy.dtype or str
+The type of the returned array.
+Examples
+
+>>> x = mx.nd.sparse.zeros('row_sparse', (2,3), dtype='float32')
+>>> y = x.astype('int32')
+>>> y.dtype
+
+"""
+res = zeros(shape=self.shape, ctx=self.context,
+dtype=dtype, stype=self.stype)
+self.copyto(res)
+return res
+
+# TODO
+def copyto(self, other):
+"""Copies the value of this array to another array.
+
+Parameters
+--
+other : NDArray or CSRNDArray or RowSparseNDArray or Context
+The destination array or context.
+
+Returns
+---
+NDArray or CSRNDArray or RowSparseNDArray
 
 Review comment:
   conversion between mkldnn and sparse array doesn't work right now.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155850103
 
 

 ##
 File path: python/mxnet/ndarray/mkldnn.py
 ##
 @@ -0,0 +1,113 @@
+# 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.
+
+# coding: utf-8
+# pylint: disable=wildcard-import, unused-wildcard-import, too-many-lines
+
+"""MKLDNN NDArray API of MXNet."""
+
+from __future__ import absolute_import
+from __future__ import division
+try:
+from __builtin__ import slice as py_slice
+from __builtin__ import sum as py_sum
+except ImportError:
+from builtins import slice as py_slice
+from builtins import sum as py_sum
+
+import ctypes
+import warnings
+
+__all__ = ["_ndarray_cls", "MKLNDArray"]
+
+import numpy as np
+from ..base import _LIB, numeric_types
+from ..base import c_array, mx_real_t, integer_types
+from ..base import mx_uint, NDArrayHandle, check_call
+from ..context import Context
+from . import _internal
+from . import op
+from ._internal import _set_ndarray_class
+from .ndarray import NDArray, _storage_type, _DTYPE_NP_TO_MX, _DTYPE_MX_TO_NP
+from .ndarray import _STORAGE_TYPE_STR_TO_ID, _STORAGE_TYPE_MKLDNN
+from .ndarray import _STORAGE_TYPE_UNDEFINED, _STORAGE_TYPE_DEFAULT
+from .ndarray import zeros as _zeros_ndarray
+from .ndarray import array as _array
+
+class MKLNDArray(NDArray):
+"""The base class of an NDArray stored in a MKLDNN storage format.
+"""
+
+def __repr__(self):
+"""Returns a string representation of the sparse array."""
+shape_info = 'x'.join(['%d' % x for x in self.shape])
+# The data content is not displayed since the array usually has big 
shape
+return '\n<%s %s @%s>' % (self.__class__.__name__,
+  shape_info, self.context)
+
+# TODO
+def _at(self, idx):
+raise NotSupportedForMKLNDArray(self._at, '[idx]', idx)
+
+def _slice(self, start, stop):
+return op.slice(self, begin=start, end=stop)
+
+# TODO
+def astype(self, dtype):
+"""Returns a copy of the array after casting to a specified type.
+Parameters
+--
+dtype : numpy.dtype or str
+The type of the returned array.
+Examples
+
+>>> x = mx.nd.sparse.zeros('row_sparse', (2,3), dtype='float32')
+>>> y = x.astype('int32')
+>>> y.dtype
+
+"""
+res = zeros(shape=self.shape, ctx=self.context,
+dtype=dtype, stype=self.stype)
+self.copyto(res)
+return res
+
+# TODO
+def copyto(self, other):
+"""Copies the value of this array to another array.
+
+Parameters
+--
+other : NDArray or CSRNDArray or RowSparseNDArray or Context
+The destination array or context.
+
+Returns
+---
+NDArray or CSRNDArray or RowSparseNDArray
 
 Review comment:
   conversion between mkldnn and sparse array doesn't work.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155849981
 
 

 ##
 File path: python/mxnet/ndarray/mkldnn.py
 ##
 @@ -0,0 +1,113 @@
+# 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.
+
+# coding: utf-8
+# pylint: disable=wildcard-import, unused-wildcard-import, too-many-lines
+
+"""MKLDNN NDArray API of MXNet."""
+
+from __future__ import absolute_import
+from __future__ import division
+try:
+from __builtin__ import slice as py_slice
+from __builtin__ import sum as py_sum
+except ImportError:
+from builtins import slice as py_slice
+from builtins import sum as py_sum
+
+import ctypes
+import warnings
+
+__all__ = ["_ndarray_cls", "MKLNDArray"]
+
+import numpy as np
+from ..base import _LIB, numeric_types
+from ..base import c_array, mx_real_t, integer_types
+from ..base import mx_uint, NDArrayHandle, check_call
+from ..context import Context
+from . import _internal
+from . import op
+from ._internal import _set_ndarray_class
+from .ndarray import NDArray, _storage_type, _DTYPE_NP_TO_MX, _DTYPE_MX_TO_NP
+from .ndarray import _STORAGE_TYPE_STR_TO_ID, _STORAGE_TYPE_MKLDNN
+from .ndarray import _STORAGE_TYPE_UNDEFINED, _STORAGE_TYPE_DEFAULT
+from .ndarray import zeros as _zeros_ndarray
+from .ndarray import array as _array
+
+class MKLNDArray(NDArray):
+"""The base class of an NDArray stored in a MKLDNN storage format.
+"""
+
+def __repr__(self):
 
 Review comment:
   you are right.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155847361
 
 

 ##
 File path: prepare_mkldnn.sh
 ##
 @@ -0,0 +1,121 @@
+#!/bin/bash
+
+# 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.
+
+# set -ex
+#
+# All modification made by Intel Corporation: ? 2016 Intel Corporation
+#
+# All contributions by the University of California:
+# Copyright (c) 2014, 2015, The Regents of the University of California 
(Regents)
+# All rights reserved.
+#
+# All other contributions:
+# Copyright (c) 2014, 2015, the respective contributors
+# All rights reserved.
+# For the list of contributors go to 
https://github.com/BVLC/caffe/blob/master/CONTRIBUTORS.md
+#
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice,
+#   this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+#   notice, this list of conditions and the following disclaimer in the
+#   documentation and/or other materials provided with the distribution.
+# * Neither the name of Intel Corporation nor the names of its contributors
+#   may be used to endorse or promote products derived from this software
+#   without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE 
ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+
+MXNET_ROOTDIR="$(pwd)"
+MKLDNN_ROOTDIR="$MXNET_ROOTDIR/external/mkldnn"
+MKLDNN_GITHUB="https://github.com/01org/mkl-dnn.git;
+MKLDNN_TMPDIR="$MKLDNN_ROOTDIR/tmp"
+MKLDNN_SRCDIR="$MKLDNN_ROOTDIR/src"
+MKLDNN_BUILDDIR="$MKLDNN_ROOTDIR/build"
+MKLDNN_INSTALLDIR="$MKLDNN_ROOTDIR/install"
+
+# MKL DNN release tag, or commit.
+MKLDNN_COMMIT="v0.11"
+
+# MKLDNN install destination
+HOME_MKLDNN=$1
+if [ ! -z "$HOME_MKLDNN" ]; then
+  mkdir -p $HOME_MKLDNN
+  if [ ! -w $HOME_MKLDNN ]; then
+echo "MKLDNN install to $HOME_MKLDNN failed, please try with sudo" >&2
+exit 1
+  fi
+fi
+
+if [ -z $MKLDNNROOT ]; then
+if [ ! -f "$MKLDNN_INSTALLDIR/lib/libmkldnn.so" ]; then
 
 Review comment:
   sure, we can do so. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155847115
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -820,20 +798,14 @@ class NDArray {
 // storage shape is also updated
 // if data is already allocated, try reuse the storage. Otherwise, free 
the current one
 // and allocate new storage
-inline void CheckAndAllocData(const TShape , int dtype) {
-  CHECK_NE(aux_shapes.size(), 0) << "data is expected to be allocated 
after aux_data";
-  auto dbytes = shape.Size() * mshadow::mshadow_sizeof(dtype);
-  if (shandle.size < dbytes) {
-// free storage if necessary and alloc again
-if (shandle.size > 0) Storage::Get()->Free(shandle);
-// init storage
-shandle = Storage::Get()->Alloc(dbytes, ctx);
-  }
-  // init shape
-  storage_shape = shape;
-  // delay_alloc is only set when data storage handle is present
-  delay_alloc = false;
-}
+void CheckAndAllocData(const TShape , int dtype);
+
+#if MXNET_USE_MKLDNN == 1
+// Have MKL memory reference to the data in the default storage
+// or create memory for MKLDNN.
+void SetMKLMem(const TShape , int dtype);
 
 Review comment:
   MKLDNN memory has various memory layouts and it's not compatible with MXNet 
default layouts.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155846518
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -35,12 +35,12 @@
 #include 
 #include 
 #include 
+#if MXNET_USE_MKLDNN == 1
+#include 
 
 Review comment:
   I'm not sure. Both seem to work fine. Intel provides a python script to 
install MKLDNN in the system.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155846846
 
 

 ##
 File path: include/mxnet/ndarray.h
 ##
 @@ -608,6 +549,37 @@ class NDArray {
  << "CheckAndAllocAuxData is not intended for kDefaultStorage";
 ptr_->CheckAndAllocAuxData(i, aux_shape);
   }
+
+#if MXNET_USE_MKLDNN == 1
+  /*
+   * This function returns mkldnn::memory with the default primitive_desc.
+   */
+  std::shared_ptr GetMKLDNNData() const;
 
 Review comment:
   We're investigating performance issues. We'll see if shared pointers can 
cause performance issues.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #8302: Refactor operators & MKLDNN

2017-12-08 Thread GitBox
zheng-da commented on a change in pull request #8302: Refactor operators & 
MKLDNN
URL: https://github.com/apache/incubator-mxnet/pull/8302#discussion_r155845973
 
 

 ##
 File path: Makefile
 ##
 @@ -40,11 +40,11 @@ endif
 # use customized config file
 include $(config)
 
-ifeq ($(USE_MKL2017), 1)
-# must run ./prepare_mkl before including mshadow.mk
-   RETURN_STRING := $(shell ./prepare_mkl.sh $(MKLML_ROOT))
-   MKLROOT := $(firstword $(RETURN_STRING))
-   export USE_MKLML = $(lastword $(RETURN_STRING))
+ifeq ($(USE_MKLDNN), 1)
 
 Review comment:
   Not yet. I'll update.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services