You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2021/02/15 10:45:12 UTC

[GitHub] [incubator-mxnet] mozga-intel opened a new pull request #19896: [FEATURE] OneDNN adaptive padding support was added to mxnet.

mozga-intel opened a new pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896


   ## Description ##
   OneDNN adaptive padding operator was added to MxNet. There is a fallback to a naive impl. of this kernel - just in case the paddings are 0. Additionally, there is a mechanism to check the correctness of this one'. 
   
   ## Checklist ##
   ### Essentials ###
   - [ ] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
   - [ ] Changes are complete (i.e. I finished coding on this PR)
   - [ ] All changes have test coverage
   - [ ] Code is well-documented
   
   ### Changes ###
   - [ ] Feature1, tests, (and when applicable, API doc)
   - [ ] Feature2, tests, (and when applicable, API doc)
   
   ## Comments ##
   - If this change is a backwards-incompatible change, why must this change be made.
   - Interesting edge cases to note here
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
anko-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-869658281


   @mozga-intel  please provide  performance results showing performance improvement of the new approach.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-879166460


   @TaoLv Yes, this a classic average pooling.   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r650936103



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,81 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);

Review comment:
       Done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-883168817


   > > @TaoLv Yes, this a classic average pooling.
   > 
   > If so, please consider if it's possible to integrate oneDNN pooling primitive once and support both mxnet pooling and adaptive pooling operators.
   
   Thanks! I thought about that, there are a few things to do and we decided to do it in the next path. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-782073687


   @mxnet-bot run ci sanity


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -219,6 +295,11 @@ The pooling kernel and stride sizes are automatically chosen for desired output
 .set_attr<FCompute>("FCompute<cpu>", AdaptiveAvgPoolOpForward<cpu>)
 .set_attr<nnvm::FGradient>("FGradient",
   ElemwiseGradUseNone{"_backward_contrib_AdaptiveAvgPooling2D"})
+.set_attr<FInferStorageType>("FInferStorageType", AdaptivePoolingStorageType)
+#if MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling-inl.h
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling-inl.h
+*/
+#ifndef MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+#define MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+
+#if MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling-inl.h
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling-inl.h
+*/
+#ifndef MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+#define MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+
+#if MXNET_USE_MKLDNN == 1
+
+#include <mkldnn.hpp>
+#include <utility>
+#include "../../operator_common.h"
+#include "./mkldnn_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class MKLDNNAdaptivePoolingFwd {
+ public:
+  MKLDNNAdaptivePoolingFwd(const mxnet::NDArray &input,
+                           const mxnet::NDArray &output,
+                           const mkldnn::memory::dims &kernel,
+                           const mkldnn::memory::dims &strides,
+                           const mkldnn::memory::dims &pad_l,
+                           const mkldnn::memory::dims &pad_r,
+                           const mkldnn::algorithm alg_kind,
+                           const bool with_workspace, const bool is_train)
+      : with_workspace_(with_workspace), fwd_(nullptr) {
+    Init(input, output, kernel, strides, pad_l, pad_r, is_train, alg_kind);
+  }
+  ~MKLDNNAdaptivePoolingFwd() = default;
+
+ public:
+  void Execute(const NDArray &input, const OpReqType req, const NDArray &output,
+               const NDArray *workspace);
+
+ private:
+  bool with_workspace_;
+  std::shared_ptr<mkldnn::pooling_forward::primitive_desc> fwd_pd_;
+  std::shared_ptr<mkldnn::pooling_forward> fwd_;
+
+ private:
+  void Init(const mxnet::NDArray &input, const mxnet::NDArray &output,
+            const mkldnn::memory::dims &kernel,
+            const mkldnn::memory::dims &strides,
+            const mkldnn::memory::dims &pad_l,
+            const mkldnn::memory::dims &pad_r, const bool is_train,
+            const mkldnn::algorithm alg_kind);
+};
+
+template <typename T = mkldnn::memory::dims>
+void updateAdaptivePaddingKernel(T *kernel, T *strides, T *pad_l, T *pad_r,
+                                 const NDArray &in_data,
+                                 const NDArray &out_data) {
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  strides->at(0) = floor((IH << 1) / OH) - floor(IH / OH);
+  strides->at(1) = floor((IW << 1) / OW) - floor(IW / OW);
+  kernel->at(0) = ceil((IH << 1) / OH) - floor(IH / OH);
+  kernel->at(1) = ceil((IW << 1) / OW) - floor(IW / OW);
+  pad_l->at(0) = (strides->at(0) * (OH - 1) + kernel->at(0) - IH) >> 1;
+  pad_l->at(1) = (strides->at(1) * (OW - 1) + kernel->at(1) - IW) >> 1;
+}
+
+template <typename T>
+MKLDNNAdaptivePoolingFwd &GetPoolingFwd(const T &param, const bool is_train,
+                                        const NDArray &input,
+                                        const NDArray &output) {
+  if (input.shape().ndim() != 4) {
+    LOG(FATAL) << "MKLDNN Adaptive Avg Pool 2d: Expect only 2D input";
+  }
+  typedef ParamOpSign<T> MKLDNNPoolingSignature;
+#if DMLC_CXX11_THREAD_LOCAL
+  static thread_local std::unordered_map<MKLDNNPoolingSignature,
+                                         MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#else
+  static MX_THREAD_LOCAL std::unordered_map<MKLDNNPoolingSignature,
+                                            MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#endif
+  bool with_workspace = is_train;
+  MKLDNNPoolingSignature key(param);
+  key.AddSign(is_train);
+  key.AddSign(with_workspace);
+  key.AddSign(input);
+  key.AddSign(output);
+
+  auto it = pooling_fwds.find(key);
+  if (it == pooling_fwds.end()) {
+    const int kernel_ndims = input.shape().ndim();
+
+    mkldnn::memory::dims kernel(kernel_ndims);
+    mkldnn::memory::dims strides(kernel_ndims);
+    mkldnn::memory::dims pad_l(kernel_ndims);
+    mkldnn::memory::dims pad_r(kernel_ndims);
+
+    updateAdaptivePaddingKernel(&kernel, &strides, &pad_l, &pad_r, input,
+                                output);
+    mkldnn::memory::validate_dims(kernel);
+    mkldnn::memory::validate_dims(strides);
+    mkldnn::memory::validate_dims(pad_l);
+    mkldnn::memory::validate_dims(pad_r);
+
+    mkldnn::algorithm kind = mkldnn::algorithm::pooling_avg;
+    MKLDNNAdaptivePoolingFwd fwd(input, output, kernel, kernel, pad_l, pad_r,
+                                 kind, false, false);
+    it = AddToCache(&pooling_fwds, key, fwd);
+  }
+  return it->second;
+}
+
+template <typename T>
+void MKLDNNAdaptivePoolingCompute(const OpContext &ctx, const T &param,
+                                  const NDArray &in_data, const OpReqType req,
+                                  const NDArray &out_data,
+                                  const NDArray *workspace) {
+  auto &fwd = GetPoolingFwd(param, ctx.is_train, in_data, out_data);
+  fwd.Execute(in_data, req, out_data, workspace);
+}
+}  // namespace op
+}  // namespace mxnet
+#endif  // MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling.cc
+*/
+
+#if MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling.cc
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(

Review comment:
       Functions: MKLDNNAdaptivePoolingFwd::Init() and MKLDNNAdaptivePoolingFwd::Execute() are more or less the same as the one that are in the mkldnn_poolling.cc file (MKLDNNPoolingFwd::Init() && MKLDNNPoolingFwd::Execute()). What do you think about combining them together? So that the code won't be duplicated.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling.cc
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(
+    const mxnet::NDArray &input, const mxnet::NDArray &output,
+    const mkldnn::memory::dims &kernel, const mkldnn::memory::dims &strides,
+    const mkldnn::memory::dims &pad_l, const mkldnn::memory::dims &pad_r,
+    const bool is_train, const mkldnn::algorithm alg_kind) {
+  const auto src_md = input.GetMKLDNNData()->get_desc();
+  const auto dst_md = GetMemDesc(output);
+  const mkldnn::engine engine = CpuEngine::Get()->get_engine();
+
+  if (alg_kind != mkldnn::algorithm::pooling_avg &&
+      alg_kind != mkldnn::algorithm::pooling_avg_include_padding &&
+      alg_kind != mkldnn::algorithm::pooling_avg_exclude_padding) {
+    LOG(FATAL) << "MKLDNN Adaptive Pooling: algorithm is not supported";
+  }
+
+  mkldnn::prop_kind prop = mkldnn::prop_kind::forward_scoring;
+  if (is_train && alg_kind != mkldnn::algorithm::pooling_avg) {
+    prop = mkldnn::prop_kind::forward_training;
+  }
+  if (is_train && prop == mkldnn::prop_kind::forward_scoring) {
+    LOG(INFO) << "MKLDNN Pooling: training with prop_kind is forward_scoring";
+  }
+
+  const auto fwd_desc = mkldnn::pooling_forward::desc(
+      prop, alg_kind, src_md, dst_md, strides, kernel, pad_l, pad_r);
+  this->fwd_pd_.reset(
+      new mkldnn::pooling_forward::primitive_desc(fwd_desc, engine));
+  this->fwd_.reset(new mkldnn::pooling_forward(*(this->fwd_pd_)));
+}
+
+void MKLDNNAdaptivePoolingFwd::Execute(const NDArray &input,
+                                       const OpReqType req,
+                                       const NDArray &output,
+                                       const NDArray *workspace) {
+  NDArray in_buffer = input;
+  if (input.IsView() && input.IsMKLDNNData()) {
+    in_buffer = input.Reorder2Default();
+  }
+
+  auto input_mem = in_buffer.GetMKLDNNData();
+  auto output_mem_t = CreateMKLDNNMem(output, this->fwd_pd_->dst_desc(), req);
+
+  mkldnn_args_map_t args = {{MKLDNN_ARG_SRC, *input_mem},
+                            {MKLDNN_ARG_DST, *(output_mem_t.second)}};
+
+  if (this->with_workspace_) {
+    auto engine = CpuEngine::Get()->get_engine();
+    if (workspace == nullptr) {
+      LOG(FATAL) << "MKLDNN Average Pooling: incorrect worskapce input";
+    }
+    auto ws = std::make_shared<mkldnn::memory>(
+        this->fwd_pd_->workspace_desc(), engine,
+        workspace->GetMKLDNNData()->get_data_handle());
+    args[MKLDNN_ARG_WORKSPACE] = *ws;
+  }
+  if (this->fwd_) {
+    MKLDNNStream::Get()->RegisterPrimArgs(*(this->fwd_), args);
+    CommitOutput(output, output_mem_t);
+    MKLDNNStream::Get()->Submit();
+  } else {
+    LOG(FATAL) << "MKLDNN Pooling: forward primitive is nullptr";
+  }
+}
+
+}  // namespace op
+}  // namespace mxnet
+#endif  // MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel removed a comment on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel removed a comment on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-865174871


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [sanity]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [unix-gpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
anko-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r676528368



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,90 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_H = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_W = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_H = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_W = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_H * (OH - 1) + kernel_H - IH) / 2;
+  const int pad_l_left = (strides_W * (OW - 1) + kernel_W - IW) / 2;
+

Review comment:
       Could you also return false when original performance is better than OneDNN. I guess you can easy find simple, when you sort the table with results by improvement percentage. Please double check if you collect your results on server machine.
   Such boolean  value should be commented as made due to performance (not support) reason. for example:
   bool better_performance = shape()[a] * shape()[b] > DDDD; // For smaller tensor native implementation is faster.
   
   Please also consider to check whole network in test, as result in microbenchmark could be not inline with results of whole model - for example due to memory layout conversions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
anko-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r676528368



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,90 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_H = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_W = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_H = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_W = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_H * (OH - 1) + kernel_H - IH) / 2;
+  const int pad_l_left = (strides_W * (OW - 1) + kernel_W - IW) / 2;
+

Review comment:
       Could you also return false when original performance is better than OneDNN. I guess you can easy find simple rule, when you sort the table with results by improvement percentage. Please double check if you collect your results on server machine.
   Such boolean  value should be commented as made due to performance (not support) reason. for example:
   bool better_performance = shape()[a] * shape()[b] > DDDD; // For smaller tensor native implementation is faster.
   
   Please also consider to check whole network in test, as result in microbenchmark could be not inline with results of whole model - for example due to memory layout conversions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-862149001


   @mxnet-bot run ci [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-883168817


   > > @TaoLv Yes, this a classic average pooling.
   > 
   > If so, please consider if it's possible to integrate oneDNN pooling primitive once and support both mxnet pooling and adaptive pooling operators.
   
   Thanks! I thought about that, there are a few things to do and we decided to do it in the next path. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling-inl.h
##########
@@ -43,6 +43,9 @@
 #include "../operator_common.h"
 #include "../mxnet_op.h"
 #include "../mshadow_op.h"
+#if MXNET_USE_MKLDNN == 1

Review comment:
       I've missed that.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,98 @@
+/*
+ * 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_adaptive_pooling.cc
+ * \brief

Review comment:
       docstring




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive padding support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-779756651


   @mxnet-bot run ci [all]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive padding support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-779486013


   @mxnet-label-bot add [MKLDNN]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bartekkuncer commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,

Review comment:
       Please change the name to SupportMKLDNNAveragePooling or SupportMKLDNNAP to stay consistent with other implementations.
   

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling-inl.h
##########
@@ -0,0 +1,147 @@
+/*
+ * 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_adaptive_pooling-inl.h
+ * \brief
+ * \author Mateusz Ozga
+*/
+#ifndef MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+#define MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+
+#if MXNET_USE_MKLDNN == 1
+
+#include <mkldnn.hpp>
+#include <utility>
+#include "../../operator_common.h"
+#include "./mkldnn_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class MKLDNNAdaptivePoolingFwd {
+ public:
+  MKLDNNAdaptivePoolingFwd(const mxnet::NDArray &input,
+                           const mxnet::NDArray &output,
+                           const mkldnn::memory::dims &kernel,
+                           const mkldnn::memory::dims &strides,
+                           const mkldnn::memory::dims &pad_l,
+                           const mkldnn::memory::dims &pad_r,
+                           const mkldnn::algorithm alg_kind,
+                           const bool with_workspace, const bool is_train)
+      : with_workspace_(with_workspace), fwd_(nullptr) {
+    Init(input, output, kernel, strides, pad_l, pad_r, is_train, alg_kind);
+  }
+  ~MKLDNNAdaptivePoolingFwd() = default;
+
+ public:
+  void Execute(const NDArray &input, const OpReqType req, const NDArray &output,
+               const NDArray *workspace);
+
+ private:
+  bool with_workspace_;
+  std::shared_ptr<mkldnn::pooling_forward::primitive_desc> fwd_pd_;
+  std::shared_ptr<mkldnn::pooling_forward> fwd_;
+
+ private:
+  void Init(const mxnet::NDArray &input, const mxnet::NDArray &output,
+            const mkldnn::memory::dims &kernel,
+            const mkldnn::memory::dims &strides,
+            const mkldnn::memory::dims &pad_l,
+            const mkldnn::memory::dims &pad_r, const bool is_train,
+            const mkldnn::algorithm alg_kind);
+};
+
+
+template <typename T = mkldnn::memory::dims>
+void updateAdaptivePaddingKernel(T *kernel, T *strides, T *pad_l, T *pad_r,
+                                 const NDArray &in_data,
+                                 const NDArray &out_data) {
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  strides->at(0) = floor((IH << 1) / OH) - floor(IH / OH);
+  strides->at(1) = floor((IW << 1) / OW) - floor(IW / OW);
+  kernel->at(0) = ceil((IH << 1) / OH) - floor(IH / OH);
+  kernel->at(1) = ceil((IW << 1) / OW) - floor(IW / OW);
+  pad_l->at(0) = (strides->at(0) * (OH - 1) + kernel->at(0) - IH) >> 1;
+  pad_l->at(1) = (strides->at(1) * (OW - 1) + kernel->at(1) - IW) >> 1;
+}
+
+template <typename T>
+MKLDNNAdaptivePoolingFwd &GetPoolingFwd(const T &param, const bool is_train,
+                                        const NDArray &input,
+                                        const NDArray &output) {
+  if (input.shape().ndim() != 4) {
+    LOG(FATAL) << "MKLDNN Adaptive Avg Pool 2d: Expect only 2D input";
+  }
+  typedef ParamOpSign<T> MKLDNNPoolingSignature;
+#if DMLC_CXX11_THREAD_LOCAL
+  static thread_local std::unordered_map<MKLDNNPoolingSignature,
+                                         MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#else
+  static MX_THREAD_LOCAL std::unordered_map<MKLDNNPoolingSignature,
+                                            MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#endif
+  bool with_workspace = is_train && true;

Review comment:
       What does this do?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] TaoLv commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   > @TaoLv Yes, this a classic average pooling.
   
   If so, please consider if it's possible to integrate oneDNN pooling primitive once and support both mxnet pooling and adaptive pooling operators.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown merged pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
anko-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-869658281


   @mozga-intel  please provide  performance results showing performance improvement of the new approach.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-865174871






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   @mozga-intel let me know once the comments are addressed and I can take another look then.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-879691874


    @mxnet-bot run ci [unix-gpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [unix-gpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive padding support was added to mxnet.

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


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bgawrych commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling-inl.h
##########
@@ -156,5 +161,13 @@ MSHADOW_XINLINE int get_stride(Tensor<xpu, Dim, DType> tensor, int idx) {
 
 }  // namespace op
 }  // namespace mxnet
-
+namespace std {
+template <>
+struct hash<mxnet::op::AdaptiveAvgPoolParam> {

Review comment:
       Maybe it would be worth to comment why here we returning just 0? If I understand correctly only param of this OP determines output size and shape is already part of NDArray passed to AddSign function 

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_1 * (OH - 1) + kernel_1 - IH) / 2;
+  const int pad_l_left = (strides_2 * (OW - 1) + kernel_2 - IW) / 2;
+
+  return pad_l_top == 0 && pad_l_left == 0;
+}
+
+
+void AdaptiveAvgPoolComputeExCPU(const nnvm::NodeAttrs &attrs,
+                                 const OpContext &ctx,
+                                 const std::vector<NDArray> &inputs,
+                                 const std::vector<OpReqType> &req,
+                                 const std::vector<NDArray> &outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  /*
+  DNNL doesn't support adaptive pooling. 
+  Fallback is needed when padding is not equal 0;
+  */
+  if (SupportMKLDNN(inputs[0]) &&
+      CanMKLDNNSupportAveragePooling(inputs[0], outputs[0])) {
+    const AdaptiveAvgPoolParam &param =
+        nnvm::get<AdaptiveAvgPoolParam>(attrs.parsed);
+    const NDArray *workspace = nullptr;
+    MKLDNN_OPCHECK_INIT(false, 1, inputs, outputs);
+    MKLDNNAdaptivePoolingCompute(ctx, param, inputs[0], req[0], outputs[0],
+                                 workspace);
+    return;
+  }
+  FallBackCompute(AdaptiveAvgPoolOpForward<cpu>, attrs, ctx, inputs, req,
+                  outputs);
+}
+#endif
+
+
+inline static bool AdaptivePoolingStorageType(const nnvm::NodeAttrs &attrs,
+                                              const int dev_mask,
+                                              DispatchMode *dispatch_mode,
+                                              std::vector<int> *in_attrs,
+                                              std::vector<int> *out_attrs) {
+  CHECK_EQ(in_attrs->size(), 1);
+  bool dispatched = false;
+#if MXNET_USE_MKLDNN == 1
+  if (!dispatched) {
+    dispatched = MKLDNNStorageType(attrs, dev_mask, true, dispatch_mode,
+                                   in_attrs, out_attrs);
+  }
+  if (!MKLDNNEnvSet()) {

Review comment:
       MKLDNNEnvSet is already called in MKLDNNStorageType, so probably there is no need to second check 

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,98 @@
+/*
+ * 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_adaptive_pooling.cc
+ * \brief
+ * \author Mateusz Ozga
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(
+    const mxnet::NDArray &input, const mxnet::NDArray &output,
+    const mkldnn::memory::dims &kernel, const mkldnn::memory::dims &strides,
+    const mkldnn::memory::dims &pad_l, const mkldnn::memory::dims &pad_r,
+    const bool is_train, const mkldnn::algorithm alg_kind) {
+  const auto src_md = input.GetMKLDNNData()->get_desc();
+  const auto dst_md = GetMemDesc(output);
+  const mkldnn::engine engine = CpuEngine::Get()->get_engine();
+
+  if (alg_kind != mkldnn::algorithm::pooling_avg &&
+      alg_kind != mkldnn::algorithm::pooling_avg_include_padding &&
+      alg_kind != mkldnn::algorithm::pooling_avg_exclude_padding) {
+    LOG(FATAL) << "MKLDNN Adaptive Pooling: algorithm is not supported";
+  }
+
+  mkldnn::prop_kind prop = mkldnn::prop_kind::forward_scoring;
+  if (is_train && alg_kind != mkldnn::algorithm::pooling_avg) {
+    prop = mkldnn::prop_kind::forward_training;
+  }
+  if (is_train && prop == mkldnn::prop_kind::forward_scoring) {
+    LOG(INFO) << "MKLDNN Pooling: training with prop_kind is forward_scoring";
+  }
+
+  const auto fwd_desc = mkldnn::pooling_forward::desc(
+      prop, alg_kind, src_md, dst_md, strides, kernel, pad_l, pad_r);
+  this->fwd_pd_.reset(
+      new mkldnn::pooling_forward::primitive_desc(fwd_desc, engine));
+  this->fwd_.reset(new mkldnn::pooling_forward(*(this->fwd_pd_)));
+}
+
+void MKLDNNAdaptivePoolingFwd::Execute(const NDArray &input,
+                                       const OpReqType req,
+                                       const NDArray &output,
+                                       const NDArray *workspace) {
+  NDArray in_buffer = input;
+  if (input.IsView() && input.IsMKLDNNData()) {
+    in_buffer = input.Reorder2Default();
+  }
+
+  auto input_mem = in_buffer.GetMKLDNNData();
+  auto output_mem_t = CreateMKLDNNMem(output, this->fwd_pd_->dst_desc(), req);
+
+  mkldnn_args_map_t args = {{MKLDNN_ARG_SRC, *input_mem},
+                            {MKLDNN_ARG_DST, *(output_mem_t.second)}};
+
+  if (this->with_workspace_) {
+    auto engine = CpuEngine::Get()->get_engine();
+    if (workspace == nullptr) {
+      LOG(FATAL) << "MKLDNN Average Pooling: incorrect worskapce input";
+    }
+    auto ws = std::make_shared<mkldnn::memory>(
+        (*(this->fwd_pd_)).workspace_desc(), engine,

Review comment:
       Can't it be just this->fwd_pd_->workspace_desc() ?

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_1 * (OH - 1) + kernel_1 - IH) / 2;
+  const int pad_l_left = (strides_2 * (OW - 1) + kernel_2 - IW) / 2;
+
+  return pad_l_top == 0 && pad_l_left == 0;
+}
+
+
+void AdaptiveAvgPoolComputeExCPU(const nnvm::NodeAttrs &attrs,
+                                 const OpContext &ctx,
+                                 const std::vector<NDArray> &inputs,
+                                 const std::vector<OpReqType> &req,
+                                 const std::vector<NDArray> &outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  /*
+  DNNL doesn't support adaptive pooling. 

Review comment:
       oneDNN ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-883168817


   > > @TaoLv Yes, this a classic average pooling.
   > 
   > If so, please consider if it's possible to integrate oneDNN pooling primitive once and support both mxnet pooling and adaptive pooling operators.
   
   Thanks! I thought about that, there are a few things to do and we decided to do it in the next path. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling-inl.h
##########
@@ -43,6 +43,9 @@
 #include "../operator_common.h"
 #include "../mxnet_op.h"
 #include "../mshadow_op.h"
+#if MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,81 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1

Review comment:
       Please change it to MXNET_USE_ONEDNN.

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,81 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);

Review comment:
       Maybe worth considering to rename 'strides_1, strides_2' to 'strides_H/strides_W'?

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,81 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);

Review comment:
       The same as the comment above but connected with 'kernel_1/kernel2'?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r619040786



##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling-inl.h
##########
@@ -0,0 +1,147 @@
+/*
+ * 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_adaptive_pooling-inl.h
+ * \brief
+ * \author Mateusz Ozga
+*/
+#ifndef MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+#define MXNET_OPERATOR_NN_MKLDNN_MKLDNN_ADAPTIVE_POOLING_INL_H_
+
+#if MXNET_USE_MKLDNN == 1
+
+#include <mkldnn.hpp>
+#include <utility>
+#include "../../operator_common.h"
+#include "./mkldnn_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class MKLDNNAdaptivePoolingFwd {
+ public:
+  MKLDNNAdaptivePoolingFwd(const mxnet::NDArray &input,
+                           const mxnet::NDArray &output,
+                           const mkldnn::memory::dims &kernel,
+                           const mkldnn::memory::dims &strides,
+                           const mkldnn::memory::dims &pad_l,
+                           const mkldnn::memory::dims &pad_r,
+                           const mkldnn::algorithm alg_kind,
+                           const bool with_workspace, const bool is_train)
+      : with_workspace_(with_workspace), fwd_(nullptr) {
+    Init(input, output, kernel, strides, pad_l, pad_r, is_train, alg_kind);
+  }
+  ~MKLDNNAdaptivePoolingFwd() = default;
+
+ public:
+  void Execute(const NDArray &input, const OpReqType req, const NDArray &output,
+               const NDArray *workspace);
+
+ private:
+  bool with_workspace_;
+  std::shared_ptr<mkldnn::pooling_forward::primitive_desc> fwd_pd_;
+  std::shared_ptr<mkldnn::pooling_forward> fwd_;
+
+ private:
+  void Init(const mxnet::NDArray &input, const mxnet::NDArray &output,
+            const mkldnn::memory::dims &kernel,
+            const mkldnn::memory::dims &strides,
+            const mkldnn::memory::dims &pad_l,
+            const mkldnn::memory::dims &pad_r, const bool is_train,
+            const mkldnn::algorithm alg_kind);
+};
+
+
+template <typename T = mkldnn::memory::dims>
+void updateAdaptivePaddingKernel(T *kernel, T *strides, T *pad_l, T *pad_r,
+                                 const NDArray &in_data,
+                                 const NDArray &out_data) {
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  strides->at(0) = floor((IH << 1) / OH) - floor(IH / OH);
+  strides->at(1) = floor((IW << 1) / OW) - floor(IW / OW);
+  kernel->at(0) = ceil((IH << 1) / OH) - floor(IH / OH);
+  kernel->at(1) = ceil((IW << 1) / OW) - floor(IW / OW);
+  pad_l->at(0) = (strides->at(0) * (OH - 1) + kernel->at(0) - IH) >> 1;
+  pad_l->at(1) = (strides->at(1) * (OW - 1) + kernel->at(1) - IW) >> 1;
+}
+
+template <typename T>
+MKLDNNAdaptivePoolingFwd &GetPoolingFwd(const T &param, const bool is_train,
+                                        const NDArray &input,
+                                        const NDArray &output) {
+  if (input.shape().ndim() != 4) {
+    LOG(FATAL) << "MKLDNN Adaptive Avg Pool 2d: Expect only 2D input";
+  }
+  typedef ParamOpSign<T> MKLDNNPoolingSignature;
+#if DMLC_CXX11_THREAD_LOCAL
+  static thread_local std::unordered_map<MKLDNNPoolingSignature,
+                                         MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#else
+  static MX_THREAD_LOCAL std::unordered_map<MKLDNNPoolingSignature,
+                                            MKLDNNAdaptivePoolingFwd, OpHash>
+      pooling_fwds;
+#endif
+  bool with_workspace = is_train && true;

Review comment:
       The remains of a bwd.  Removed: Done.
   

##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,

Review comment:
       Done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   @mozga-intel are there any tests for this change?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-865174871


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel edited a comment on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel edited a comment on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-782073687


   @mxnet-bot run ci [sanity]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bartekkuncer commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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



##########
File path: src/operator/contrib/adaptive_avg_pooling-inl.h
##########
@@ -43,6 +43,9 @@
 #include "../operator_common.h"
 #include "../mxnet_op.h"
 #include "../mshadow_op.h"
+#if MXNET_USE_MKLDNN == 1

Review comment:
       This is a change on v1.x branch so MXNET_USE_MKLDNN is actually correct.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] akarbown commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   @mozga-intel are there any tests for this change?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r650936307



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,81 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);

Review comment:
       Done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r686123337



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,90 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool SupportMKLDNNAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_H = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_W = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_H = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_W = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_H * (OH - 1) + kernel_H - IH) / 2;
+  const int pad_l_left = (strides_W * (OW - 1) + kernel_W - IW) / 2;
+

Review comment:
       Good point! However, it could be difficult to determine precisely the lower and upper bound. Notice, that this's a random process that's a collection of "random" variables that is indexed by a lot of external things, i.e: for a small tensors the time needed to warm up threads could be significant, Just let me look at the issue I'll try to adress this issue, but in the next patch. Yes it might be faster, I'm invoking this function tenfold and furthermore the tesnor aren't storred in the cache.  




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r619036259



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_1 * (OH - 1) + kernel_1 - IH) / 2;
+  const int pad_l_left = (strides_2 * (OW - 1) + kernel_2 - IW) / 2;
+
+  return pad_l_top == 0 && pad_l_left == 0;
+}
+
+
+void AdaptiveAvgPoolComputeExCPU(const nnvm::NodeAttrs &attrs,
+                                 const OpContext &ctx,
+                                 const std::vector<NDArray> &inputs,
+                                 const std::vector<OpReqType> &req,
+                                 const std::vector<NDArray> &outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  /*
+  DNNL doesn't support adaptive pooling. 

Review comment:
       Done. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r619036093



##########
File path: src/operator/contrib/adaptive_avg_pooling-inl.h
##########
@@ -156,5 +161,13 @@ MSHADOW_XINLINE int get_stride(Tensor<xpu, Dim, DType> tensor, int idx) {
 
 }  // namespace op
 }  // namespace mxnet
-
+namespace std {
+template <>
+struct hash<mxnet::op::AdaptiveAvgPoolParam> {

Review comment:
       This function returns "pointer"  at a tuple.  This comment seems to be unnecessary. Done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel removed a comment on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel removed a comment on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-865174871


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r619037630



##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,98 @@
+/*
+ * 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_adaptive_pooling.cc
+ * \brief
+ * \author Mateusz Ozga
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(
+    const mxnet::NDArray &input, const mxnet::NDArray &output,
+    const mkldnn::memory::dims &kernel, const mkldnn::memory::dims &strides,
+    const mkldnn::memory::dims &pad_l, const mkldnn::memory::dims &pad_r,
+    const bool is_train, const mkldnn::algorithm alg_kind) {
+  const auto src_md = input.GetMKLDNNData()->get_desc();
+  const auto dst_md = GetMemDesc(output);
+  const mkldnn::engine engine = CpuEngine::Get()->get_engine();
+
+  if (alg_kind != mkldnn::algorithm::pooling_avg &&
+      alg_kind != mkldnn::algorithm::pooling_avg_include_padding &&
+      alg_kind != mkldnn::algorithm::pooling_avg_exclude_padding) {
+    LOG(FATAL) << "MKLDNN Adaptive Pooling: algorithm is not supported";
+  }
+
+  mkldnn::prop_kind prop = mkldnn::prop_kind::forward_scoring;
+  if (is_train && alg_kind != mkldnn::algorithm::pooling_avg) {
+    prop = mkldnn::prop_kind::forward_training;
+  }
+  if (is_train && prop == mkldnn::prop_kind::forward_scoring) {
+    LOG(INFO) << "MKLDNN Pooling: training with prop_kind is forward_scoring";
+  }
+
+  const auto fwd_desc = mkldnn::pooling_forward::desc(
+      prop, alg_kind, src_md, dst_md, strides, kernel, pad_l, pad_r);
+  this->fwd_pd_.reset(
+      new mkldnn::pooling_forward::primitive_desc(fwd_desc, engine));
+  this->fwd_.reset(new mkldnn::pooling_forward(*(this->fwd_pd_)));
+}
+
+void MKLDNNAdaptivePoolingFwd::Execute(const NDArray &input,
+                                       const OpReqType req,
+                                       const NDArray &output,
+                                       const NDArray *workspace) {
+  NDArray in_buffer = input;
+  if (input.IsView() && input.IsMKLDNNData()) {
+    in_buffer = input.Reorder2Default();
+  }
+
+  auto input_mem = in_buffer.GetMKLDNNData();
+  auto output_mem_t = CreateMKLDNNMem(output, this->fwd_pd_->dst_desc(), req);
+
+  mkldnn_args_map_t args = {{MKLDNN_ARG_SRC, *input_mem},
+                            {MKLDNN_ARG_DST, *(output_mem_t.second)}};
+
+  if (this->with_workspace_) {
+    auto engine = CpuEngine::Get()->get_engine();
+    if (workspace == nullptr) {
+      LOG(FATAL) << "MKLDNN Average Pooling: incorrect worskapce input";
+    }
+    auto ws = std::make_shared<mkldnn::memory>(
+        (*(this->fwd_pd_)).workspace_desc(), engine,

Review comment:
       It depends on that what is actually approved as a standard. To be fully consistent with others files, the * should be used, otherwise, your suggestion is fine.  Done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r649935956



##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling.cc
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(

Review comment:
       Personally, I think I'm fine with it.  I will propose a fix in the next patch set.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * Copyright (c) 2021 by Contributors
+ * \file mkldnn_adaptive_pooling.cc
+*/
+
+#if MXNET_USE_MKLDNN == 1
+
+#include "./mkldnn_adaptive_pooling-inl.h"
+
+namespace mxnet {
+namespace op {
+void MKLDNNAdaptivePoolingFwd::Init(

Review comment:
       Personally, I think I'm fine with it.  I will propose a fix in the next patch set or as a separate pull request.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-788738963


   @Zha0q1 @szha Could you please check this pull-request? Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] TaoLv commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   Just to confirm to PR title, oneDNN library itself doesn't support adaptive pooling, correct? Are you using normal average pooling to work around?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-784204733


   @mxnet-bot run ci [unix-gpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-865175284


   @mxnet-bot run ci [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on a change in pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#discussion_r619037814



##########
File path: src/operator/contrib/adaptive_avg_pooling.cc
##########
@@ -197,6 +198,91 @@ num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount())
   }
 }
 
+#if MXNET_USE_MKLDNN == 1
+bool CanMKLDNNSupportAveragePooling(const NDArray &in_data,
+                                    const NDArray &out_data) {
+  for (int64_t idx = 2; idx < in_data.shape().ndim(); ++idx) {
+    const int s1 = in_data.shape()[idx];
+    const int s2 = out_data.shape()[idx];
+    if (s2 == 0) {
+      return false;
+    }
+    if (s1 % s2 != 0) {
+      return false;
+    }
+  }
+  const int IH = in_data.shape()[2];
+  const int IW = in_data.shape()[3];
+  const int OH = out_data.shape()[2];
+  const int OW = out_data.shape()[3];
+
+  const int strides_1 = floor((IH << 1) / OH) - floor(IH / OH);
+  const int strides_2 = floor((IW << 1) / OW) - floor(IW / OW);
+  const int kernel_1 = ceil((IH << 1) / OH) - floor(IH / OH);
+  const int kernel_2 = ceil((IW << 1) / OW) - floor(IW / OW);
+  const int pad_l_top = (strides_1 * (OH - 1) + kernel_1 - IH) / 2;
+  const int pad_l_left = (strides_2 * (OW - 1) + kernel_2 - IW) / 2;
+
+  return pad_l_top == 0 && pad_l_left == 0;
+}
+
+
+void AdaptiveAvgPoolComputeExCPU(const nnvm::NodeAttrs &attrs,
+                                 const OpContext &ctx,
+                                 const std::vector<NDArray> &inputs,
+                                 const std::vector<OpReqType> &req,
+                                 const std::vector<NDArray> &outputs) {
+  CHECK_EQ(inputs.size(), 1U);
+  CHECK_EQ(outputs.size(), 1U);
+  /*
+  DNNL doesn't support adaptive pooling. 
+  Fallback is needed when padding is not equal 0;
+  */
+  if (SupportMKLDNN(inputs[0]) &&
+      CanMKLDNNSupportAveragePooling(inputs[0], outputs[0])) {
+    const AdaptiveAvgPoolParam &param =
+        nnvm::get<AdaptiveAvgPoolParam>(attrs.parsed);
+    const NDArray *workspace = nullptr;
+    MKLDNN_OPCHECK_INIT(false, 1, inputs, outputs);
+    MKLDNNAdaptivePoolingCompute(ctx, param, inputs[0], req[0], outputs[0],
+                                 workspace);
+    return;
+  }
+  FallBackCompute(AdaptiveAvgPoolOpForward<cpu>, attrs, ctx, inputs, req,
+                  outputs);
+}
+#endif
+
+
+inline static bool AdaptivePoolingStorageType(const nnvm::NodeAttrs &attrs,
+                                              const int dev_mask,
+                                              DispatchMode *dispatch_mode,
+                                              std::vector<int> *in_attrs,
+                                              std::vector<int> *out_attrs) {
+  CHECK_EQ(in_attrs->size(), 1);
+  bool dispatched = false;
+#if MXNET_USE_MKLDNN == 1
+  if (!dispatched) {
+    dispatched = MKLDNNStorageType(attrs, dev_mask, true, dispatch_mode,
+                                   in_attrs, out_attrs);
+  }
+  if (!MKLDNNEnvSet()) {

Review comment:
       Done.

##########
File path: src/operator/nn/mkldnn/mkldnn_adaptive_pooling.cc
##########
@@ -0,0 +1,98 @@
+/*
+ * 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_adaptive_pooling.cc
+ * \brief

Review comment:
       Done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mozga-intel commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #19896:
URL: https://github.com/apache/incubator-mxnet/pull/19896#issuecomment-879168274


   @anko-intel Please have a look at the getting results in the description. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha commented on pull request #19896: [FEATURE] OneDNN adaptive pooling support was added to mxnet.

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


   @akarbown I think the `AdaptiveAvgPooling2D` operator is tested in `tests/python/unittest/test_operator.py::test_adaptive_avg_pool_op`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org