Review

Author: Bartlomiej Gawrych

src/operator/nn/dnnl/dnnl_reduce-inl.h

@@ -67,9 +67,9 @@ class DNNLReduceFwd {
 };
 
 template <class T>
-NumpyReduceAxesParam ConvertParamsToNumpy(const T& original_param,
-                                          const NDArray& in_data,
-                                          const NDArray& out_data);
+NumpyReduceAxesParam ConvertReduceParamsToNumpy(const T& original_param,
+                                                const NDArray& in_data,
+                                                const NDArray& out_data);
 
 void DNNLReduceForwardImpl(const NumpyReduceAxesParam& param,
                            const OpContext& ctx,
@@ -85,7 +85,7 @@ void DNNLReduceForward(const nnvm::NodeAttrs& attrs,
                        const OpReqType& req,
                        const NDArray& out_data) {
   const ParamType& org_param = nnvm::get<ParamType>(attrs.parsed);
-  auto param                 = ConvertParamsToNumpy<ParamType>(org_param, in_data, out_data);
+  auto param                 = ConvertReduceParamsToNumpy<ParamType>(org_param, in_data, out_data);
   DNNLReduceForwardImpl(param, ctx, in_data, req, out_data, reduction_alg);
 }
 
@@ -98,7 +98,7 @@ bool SupportDNNLReduce(const nnvm::NodeAttrs& attrs,
                        const NDArray& in_data,
                        const NDArray& out_data) {
   const T& org_param = nnvm::get<T>(attrs.parsed);
-  auto param         = ConvertParamsToNumpy<T>(org_param, in_data, out_data);
+  auto param         = ConvertReduceParamsToNumpy<T>(org_param, in_data, out_data);
   return SupportDNNLReduceImpl(param, in_data, out_data);
 }
 

src/operator/nn/dnnl/dnnl_reduce.cc

@@ -31,9 +31,10 @@ namespace mxnet {
 namespace op {
 
 template <>
-NumpyReduceAxesParam ConvertParamsToNumpy<ReduceAxesParam>(const ReduceAxesParam& original_param,
-                                                           const NDArray& input,
-                                                           const NDArray& output) {
+NumpyReduceAxesParam ConvertReduceParamsToNumpy<ReduceAxesParam>(
+    const ReduceAxesParam& original_param,
+    const NDArray& input,
+    const NDArray& output) {
   NumpyReduceAxesParam numpy_param;
   numpy_param.axis = dmlc::optional<mxnet::Tuple<int>>();
   if (original_param.axis.has_value()) {
@@ -61,7 +62,7 @@ NumpyReduceAxesParam ConvertParamsToNumpy<ReduceAxesParam>(const ReduceAxesParam
 }
 
 template <>
-NumpyReduceAxesParam ConvertParamsToNumpy<NumpyReduceAxesParam>(
+NumpyReduceAxesParam ConvertReduceParamsToNumpy<NumpyReduceAxesParam>(
     const NumpyReduceAxesParam& original_param,
     const NDArray& input,
     const NDArray& output) {
@@ -73,8 +74,7 @@ mxnet::Tuple<int> CanonicalizeAndSortAxes(const NDArray& input,
                                           mxnet::Tuple<int> original_axes) {
   int in_ndim = input.shape().ndim();
   mxnet::Tuple<int> axes(param.axis.value());
-  // canonicalize
-  for (index_t i = 0; i < axes.ndim(); i++) {
+  for (int i = 0; i < axes.ndim(); i++) {
     if (axes[i] < 0) {
       axes[i] += in_ndim;
     }
@@ -94,7 +94,7 @@ bool SupportDNNLReduceImpl(const NumpyReduceAxesParam& param,
     auto axes    = CanonicalizeAndSortAxes(input, param, param.axis.value());
     int last_dim = *(axes.end() - 1);
     if (last_dim != input.shape().ndim() - 1) {
-      // oneDNN not optimized case
+      // oneDNN (v2.3.2) not optimized case
       return false;
     } else {
       for (int i = 0; i < axes.ndim(); i++) {
@@ -106,7 +106,7 @@ bool SupportDNNLReduceImpl(const NumpyReduceAxesParam& param,
       }
     }
 
-    // if axis = () it's identity op and it is not supported by oneDNN
+    // if `axis = ()` it is identity op and it is not supported by oneDNN
     param_supported = param.axis.value().ndim() > 0;
   }
   // initial value not supported by oneDNN

src/operator/nn/dnnl/dnnl_transpose-inl.h

@@ -51,7 +51,7 @@ class DNNLTransposeFwd {
 DNNLTransposeFwd& GetTransposeForward(const NumpyTransposeParam& param, const NDArray& data);
 
 template <class ParamType>
-NumpyTransposeParam ConvertParamsToNumpy(const ParamType& param);
+NumpyTransposeParam ConvertTransposeParamsToNumpy(const ParamType& param);
 
 template <class ParamType>
 void DNNLTransposeForward(const nnvm::NodeAttrs& attrs,
@@ -60,7 +60,7 @@ void DNNLTransposeForward(const nnvm::NodeAttrs& attrs,
                           const OpReqType& req,
                           const NDArray& output) {
   const ParamType& org_param = nnvm::get<ParamType>(attrs.parsed);
-  auto param                 = ConvertParamsToNumpy<ParamType>(org_param);
+  auto param                 = ConvertTransposeParamsToNumpy<ParamType>(org_param);
   auto fwd                   = GetTransposeForward(param, data);
   fwd.SetNewMem(data, output);
   fwd.Execute();

src/operator/nn/dnnl/dnnl_transpose.cc

@@ -123,14 +123,15 @@ DNNLTransposeFwd& GetTransposeForward(const NumpyTransposeParam& param, const ND
 }
 
 template <>
-NumpyTransposeParam ConvertParamsToNumpy<NumpyTransposeParam>(const NumpyTransposeParam& param) {
+NumpyTransposeParam ConvertTransposeParamsToNumpy<NumpyTransposeParam>(
+    const NumpyTransposeParam& param) {
   NumpyTransposeParam numpy_param;
   numpy_param.axes = common::CanonicalizeAxes(param.axes);
   return numpy_param;
 }
 
 template <>
-NumpyTransposeParam ConvertParamsToNumpy<TransposeParam>(const TransposeParam& param) {
+NumpyTransposeParam ConvertTransposeParamsToNumpy<TransposeParam>(const TransposeParam& param) {
   NumpyTransposeParam numpy_param;
   if (param.axes.ndim() == 0) {
     numpy_param.axes = mxnet::TShape(-1, 0);