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/10/28 14:44:01 UTC

[GitHub] [incubator-mxnet] bgawrych opened a new pull request #20712: Remove identity operators from oneDNN optimized graph

bgawrych opened a new pull request #20712:
URL: https://github.com/apache/incubator-mxnet/pull/20712


   ## Description ##
   This property removes dropout and copy operators from optimized graph for oneDNN - it doesn't affect performance by a lot, but allows to fuse other patterns which wouldn't be fused if there was identity operator between.
   


-- 
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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [unix-gpu, centos-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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [centos-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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [unix-cpu, unix-gpu, windows-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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [centos-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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);
+        if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+          ret.push_back(non_const_i);
+        }
+      }
+      return ret;
+    }
+  }
+
+  void Reset() override {
+    CHECK_GE(matched_list_.size(), 1);
+    auto new_selector = SgDNNLIdentitySelector();
+    new_selector.Select(*matched_list_[0], nullptr);
+    *this = new_selector;
+  }
+};
+
+inline bool IsIdentityNode(const nnvm::ObjectPtr node) {
+  return node->op() == Op::Get("_npi_copy") || node->op() == Op::Get("Dropout");
+}
+
+class SgDNNLIdentityProperty : public SubgraphProperty {
+ public:
+  SgDNNLIdentityProperty() {}
+
+  static SubgraphPropertyPtr Create() {
+    static const std::string& name = "DNNL Identity optimization pass";
+    auto property                  = std::make_shared<SgDNNLIdentityProperty>();
+    property->SetAttr<std::string>("property_name", name);
+    property->SetAttr<bool>("inference_only", true);
+    return property;
+  }
+
+  nnvm::ObjectPtr CreateSubgraphNode(const nnvm::Symbol& sym,
+                                     const int subgraph_id = 0) const override {
+    nnvm::NodeEntry identity_node_entry;
+    for (auto nentry : sym.outputs) {
+      if (nentry.node->op() && IsIdentityNode(nentry.node)) {
+        identity_node_entry = nentry;
+      }
+    }
+
+    auto last_node = identity_node_entry.node;
+    nnvm::Symbol new_sym;
+    new_sym.outputs.emplace_back(last_node);
+
+    nnvm::ObjectPtr org_node;
+    DFSVisit(new_sym.outputs, [&](const nnvm::ObjectPtr& node) {
+      if (!IsIdentityNode(node) && node->op()) {
+        org_node = node;
+      }
+    });
+
+    // Create copy of original node
+    nnvm::ObjectPtr n = nnvm::Node::Create();
+    n->attrs          = org_node->attrs;
+    CHECK(n->attrs.op);

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.

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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);
+        if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+          ret.push_back(non_const_i);
+        }
+      }
+      return ret;
+    }
+  }
+
+  void Reset() override {
+    CHECK_GE(matched_list_.size(), 1);
+    auto new_selector = SgDNNLIdentitySelector();
+    new_selector.Select(*matched_list_[0], nullptr);
+    *this = new_selector;
+  }
+};
+
+inline bool IsIdentityNode(const nnvm::ObjectPtr node) {
+  return node->op() == Op::Get("_npi_copy") || node->op() == Op::Get("Dropout");
+}
+
+class SgDNNLIdentityProperty : public SubgraphProperty {
+ public:
+  SgDNNLIdentityProperty() {}
+
+  static SubgraphPropertyPtr Create() {
+    static const std::string& name = "DNNL Identity optimization pass";
+    auto property                  = std::make_shared<SgDNNLIdentityProperty>();
+    property->SetAttr<std::string>("property_name", name);
+    property->SetAttr<bool>("inference_only", true);
+    return property;
+  }
+
+  nnvm::ObjectPtr CreateSubgraphNode(const nnvm::Symbol& sym,
+                                     const int subgraph_id = 0) const override {
+    nnvm::NodeEntry identity_node_entry;
+    for (auto nentry : sym.outputs) {
+      if (nentry.node->op() && IsIdentityNode(nentry.node)) {

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.

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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [unix-gpu, centos-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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [centos-gpu, windows-gpu, 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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [unix-gpu, centos-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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [unix-cpu, unix-gpu, windows-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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);

Review comment:
       I've refactored this filter function




-- 
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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [unix-gpu, centos-gpu, windows-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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {

Review comment:
       As it is simple fuse I decided to remove this state machine




-- 
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] bgawrych merged pull request #20712: Remove identity operators from oneDNN optimized graph

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


   


-- 
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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [unix-gpu, centos-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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_subgraph_base-inl.h
##########
@@ -31,7 +31,7 @@ static inline bool SupportDNNLAttr(const std::shared_ptr<NodeAttr>& node_attr) {
     return (node_attr->dispatch_mode == DispatchMode::kFComputeEx) &&
            (node_attr->itype[0] == mshadow::kFloat32 ||
             node_attr->itype[0] == mshadow::kBfloat16) &&
-           (ndim == 1 || ndim == 2 || ndim == 4 || ndim == 5);
+           (ndim >= 1 && ndim <= 5);

Review comment:
       Yes, i don't see any reason why 3 should be ommited here - probably someone missed it or there was some kind of not noted bug. All tests are passing so I'm assuming it's ok




-- 
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] PawelGlomski-Intel commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on a change in pull request #20712:
URL: https://github.com/apache/incubator-mxnet/pull/20712#discussion_r753201734



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {

Review comment:
       ```suggestion
       if (in_status != InStatus::kSuccess) {
   ```

##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);
+        if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+          ret.push_back(non_const_i);
+        }
+      }
+      return ret;
+    }
+  }
+
+  void Reset() override {
+    CHECK_GE(matched_list_.size(), 1);
+    auto new_selector = SgDNNLIdentitySelector();
+    new_selector.Select(*matched_list_[0], nullptr);
+    *this = new_selector;
+  }
+};
+
+inline bool IsIdentityNode(const nnvm::ObjectPtr node) {
+  return node->op() == Op::Get("_npi_copy") || node->op() == Op::Get("Dropout");
+}
+
+class SgDNNLIdentityProperty : public SubgraphProperty {
+ public:
+  SgDNNLIdentityProperty() {}
+
+  static SubgraphPropertyPtr Create() {
+    static const std::string& name = "DNNL Identity optimization pass";
+    auto property                  = std::make_shared<SgDNNLIdentityProperty>();
+    property->SetAttr<std::string>("property_name", name);
+    property->SetAttr<bool>("inference_only", true);
+    return property;
+  }
+
+  nnvm::ObjectPtr CreateSubgraphNode(const nnvm::Symbol& sym,
+                                     const int subgraph_id = 0) const override {
+    nnvm::NodeEntry identity_node_entry;
+    for (auto nentry : sym.outputs) {
+      if (nentry.node->op() && IsIdentityNode(nentry.node)) {

Review comment:
       Include nullptr check in `IsIdentityNode`?

##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);

Review comment:
       I think it would be better to iterate over `candidates` and choose nodes, that are in `matched_list_`, just to remove this const_cast. But, I am not even sure, what is the purpose of this loop. Since we choose the common elements of `matched_list_` and `candidates`, won't this result in some invalid pattern when these vectors are different?

##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);
+        if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+          ret.push_back(non_const_i);
+        }
+      }
+      return ret;
+    }
+  }
+
+  void Reset() override {
+    CHECK_GE(matched_list_.size(), 1);
+    auto new_selector = SgDNNLIdentitySelector();
+    new_selector.Select(*matched_list_[0], nullptr);
+    *this = new_selector;
+  }
+};
+
+inline bool IsIdentityNode(const nnvm::ObjectPtr node) {
+  return node->op() == Op::Get("_npi_copy") || node->op() == Op::Get("Dropout");
+}
+
+class SgDNNLIdentityProperty : public SubgraphProperty {
+ public:
+  SgDNNLIdentityProperty() {}
+
+  static SubgraphPropertyPtr Create() {
+    static const std::string& name = "DNNL Identity optimization pass";
+    auto property                  = std::make_shared<SgDNNLIdentityProperty>();
+    property->SetAttr<std::string>("property_name", name);
+    property->SetAttr<bool>("inference_only", true);
+    return property;
+  }
+
+  nnvm::ObjectPtr CreateSubgraphNode(const nnvm::Symbol& sym,
+                                     const int subgraph_id = 0) const override {
+    nnvm::NodeEntry identity_node_entry;
+    for (auto nentry : sym.outputs) {
+      if (nentry.node->op() && IsIdentityNode(nentry.node)) {
+        identity_node_entry = nentry;
+      }
+    }
+
+    auto last_node = identity_node_entry.node;
+    nnvm::Symbol new_sym;
+    new_sym.outputs.emplace_back(last_node);
+
+    nnvm::ObjectPtr org_node;
+    DFSVisit(new_sym.outputs, [&](const nnvm::ObjectPtr& node) {
+      if (!IsIdentityNode(node) && node->op()) {

Review comment:
       As above

##########
File path: tests/python/dnnl/subgraphs/test_fc_subgraph.py
##########
@@ -200,3 +200,26 @@ def forward(self, x):
   attrs = {'fc': {}}
   net = MultiOutputFC()
   check_fusion(net, data_shape, attrs, check_quantization=flatten)
+
+
+@mx.util.use_np
+@pytest.mark.parametrize('identity_node', ['dropout', 'copy'])#DATA_SHAPE)
+def test_fc_identity_eltwise(identity_node):
+  class FCIdentityEltwise(nn.HybridBlock):
+    def __init__(self, identity_node, **kwargs):
+      super(FCIdentityEltwise, self).__init__(**kwargs)
+      self.fc = nn.Dense(units=64, use_bias=False, weight_initializer=None, flatten=True)
+      self.identity_node = identity_node
+    def forward(self, x):
+      fc_out = self.fc(x)
+      if self.identity_node == 'copy':
+        fc_out = mx.np.copy(fc_out)
+      else:
+        fc_out = mx.npx.dropout(fc_out)
+      out = mx.npx.activation(fc_out, act_type='relu')
+      return out
+
+  data_shape = (64, 4, 10, 10)
+  attrs = {'fc': {'with_eltwise': 'true'}}
+  net = FCIdentityEltwise(identity_node)
+  check_fusion(net, data_shape, attrs, check_quantization=False)

Review comment:
       Does this test whether the graph is correct?

##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 dnnl_identity_property.cc
+ * \brief Graph property for removing identity operators
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_IDENTITY_PROPERTY_H_
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "../common.h"
+#include "../../nn/dropout-inl.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
+  enum class InStatus { kFail = 0, kStart, kSuccess };
+
+ private:
+  std::vector<const BiDirectedNode*> matched_list_;
+  InStatus in_status{};
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    bool status = false;
+    if (seed_node.node->op() == Op::Get("_npi_copy")) {
+      status = true;
+    }
+
+    if (seed_node.node->op() == Op::Get("Dropout")) {
+      auto const& dropout_param = nnvm::get<DropoutParam>(seed_node.node->attrs.parsed);
+      if (dropout_param.mode == dropout::kTraining) {
+        status = true;
+      }
+    }
+
+    if (status) {
+      in_status = InStatus::kStart;
+      matched_list_.clear();
+      matched_list_.emplace_back(&seed_node);
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (in_status == InStatus::kFail || in_status == InStatus::kSuccess ||
+        input_node.node->is_variable()) {
+      return false;
+    }
+
+    switch (in_status) {
+      case InStatus::kStart:
+        if (input_node.node->op()) {
+          in_status = InStatus::kSuccess;
+          matched_list_.emplace_back(&input_node);
+          return true;
+        }
+      default:
+        return false;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    return false;
+  }
+
+  std::vector<BiDirectedNode*> Filter(const std::vector<BiDirectedNode*>& candidates) override {
+    if (in_status == InStatus::kFail || in_status != InStatus::kSuccess) {
+      return std::vector<BiDirectedNode*>(0);
+    } else {
+      std::vector<BiDirectedNode*> ret;
+      for (auto i : matched_list_) {
+        auto non_const_i = const_cast<BiDirectedNode*>(i);
+        if (std::find(candidates.begin(), candidates.end(), non_const_i) != candidates.end()) {
+          ret.push_back(non_const_i);
+        }
+      }
+      return ret;
+    }
+  }
+
+  void Reset() override {
+    CHECK_GE(matched_list_.size(), 1);
+    auto new_selector = SgDNNLIdentitySelector();
+    new_selector.Select(*matched_list_[0], nullptr);
+    *this = new_selector;
+  }
+};
+
+inline bool IsIdentityNode(const nnvm::ObjectPtr node) {
+  return node->op() == Op::Get("_npi_copy") || node->op() == Op::Get("Dropout");
+}
+
+class SgDNNLIdentityProperty : public SubgraphProperty {
+ public:
+  SgDNNLIdentityProperty() {}
+
+  static SubgraphPropertyPtr Create() {
+    static const std::string& name = "DNNL Identity optimization pass";
+    auto property                  = std::make_shared<SgDNNLIdentityProperty>();
+    property->SetAttr<std::string>("property_name", name);
+    property->SetAttr<bool>("inference_only", true);
+    return property;
+  }
+
+  nnvm::ObjectPtr CreateSubgraphNode(const nnvm::Symbol& sym,
+                                     const int subgraph_id = 0) const override {
+    nnvm::NodeEntry identity_node_entry;
+    for (auto nentry : sym.outputs) {
+      if (nentry.node->op() && IsIdentityNode(nentry.node)) {
+        identity_node_entry = nentry;
+      }
+    }
+
+    auto last_node = identity_node_entry.node;
+    nnvm::Symbol new_sym;
+    new_sym.outputs.emplace_back(last_node);
+
+    nnvm::ObjectPtr org_node;
+    DFSVisit(new_sym.outputs, [&](const nnvm::ObjectPtr& node) {
+      if (!IsIdentityNode(node) && node->op()) {
+        org_node = node;
+      }
+    });
+
+    // Create copy of original node
+    nnvm::ObjectPtr n = nnvm::Node::Create();
+    n->attrs          = org_node->attrs;
+    CHECK(n->attrs.op);

Review comment:
       ```suggestion
       CHECK(n->op());
   ```
   To make it clear that this check refers to the `n->op()` used below.




-- 
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] bartekkuncer commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: src/operator/subgraph/dnnl/dnnl_subgraph_base-inl.h
##########
@@ -31,7 +31,7 @@ static inline bool SupportDNNLAttr(const std::shared_ptr<NodeAttr>& node_attr) {
     return (node_attr->dispatch_mode == DispatchMode::kFComputeEx) &&
            (node_attr->itype[0] == mshadow::kFloat32 ||
             node_attr->itype[0] == mshadow::kBfloat16) &&
-           (ndim == 1 || ndim == 2 || ndim == 4 || ndim == 5);
+           (ndim >= 1 && ndim <= 5);

Review comment:
       Are you sure `ndim == 3` is fine as well?

##########
File path: tests/python/dnnl/subgraphs/test_fc_subgraph.py
##########
@@ -200,3 +200,26 @@ def forward(self, x):
   attrs = {'fc': {}}
   net = MultiOutputFC()
   check_fusion(net, data_shape, attrs, check_quantization=flatten)
+
+
+@mx.util.use_np
+@pytest.mark.parametrize('identity_node', ['dropout', 'copy'])#DATA_SHAPE)
+def test_fc_identity_eltwise(identity_node):
+  class FCIdentityEltwise(nn.HybridBlock):
+    def __init__(self, identity_node, **kwargs):
+      super(FCIdentityEltwise, self).__init__(**kwargs)
+      self.fc = nn.Dense(units=64, use_bias=False, weight_initializer=None, flatten=True)
+      self.identity_node = identity_node
+    def forward(self, x):
+      fc_out = self.fc(x)
+      if self.identity_node == 'copy':
+        fc_out = mx.np.copy(fc_out)
+      else:
+        fc_out = mx.npx.dropout(fc_out)
+      out = mx.npx.activation(fc_out, act_type='relu')
+      return out
+
+  data_shape = (64, 4, 10, 10)
+  attrs = {'fc': {'with_eltwise': 'true'}}
+  net = FCIdentityEltwise(identity_node)
+  check_fusion(net, data_shape, attrs, check_quantization=False)

Review comment:
       No new line.




-- 
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] bgawrych commented on a change in pull request #20712: Remove identity operators from oneDNN optimized graph

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



##########
File path: tests/python/dnnl/subgraphs/test_fc_subgraph.py
##########
@@ -200,3 +200,26 @@ def forward(self, x):
   attrs = {'fc': {}}
   net = MultiOutputFC()
   check_fusion(net, data_shape, attrs, check_quantization=flatten)
+
+
+@mx.util.use_np
+@pytest.mark.parametrize('identity_node', ['dropout', 'copy'])#DATA_SHAPE)
+def test_fc_identity_eltwise(identity_node):
+  class FCIdentityEltwise(nn.HybridBlock):
+    def __init__(self, identity_node, **kwargs):
+      super(FCIdentityEltwise, self).__init__(**kwargs)
+      self.fc = nn.Dense(units=64, use_bias=False, weight_initializer=None, flatten=True)
+      self.identity_node = identity_node
+    def forward(self, x):
+      fc_out = self.fc(x)
+      if self.identity_node == 'copy':
+        fc_out = mx.np.copy(fc_out)
+      else:
+        fc_out = mx.npx.dropout(fc_out)
+      out = mx.npx.activation(fc_out, act_type='relu')
+      return out
+
+  data_shape = (64, 4, 10, 10)
+  attrs = {'fc': {'with_eltwise': 'true'}}
+  net = FCIdentityEltwise(identity_node)
+  check_fusion(net, data_shape, attrs, check_quantization=False)

Review comment:
       This checks if relu is fused into FC - checks whether attribute 'with_eltwise' in symbol is set to true. This tests checks also results before and after fuse. If identity node won't be fused into FC this test will blow up because eltwise won't be fused either ('with_eltwise' attribute)




-- 
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 #20712: Remove identity operators from oneDNN optimized graph

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


   Hey @bgawrych , 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**: [centos-gpu, sanity, centos-cpu, windows-gpu, edge, website, miscellaneous, unix-cpu, clang, unix-gpu, windows-cpu]
   *** 
   _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.

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] bgawrych commented on pull request #20712: Remove identity operators from oneDNN optimized graph

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


   @mxnet-bot run ci [centos-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 #20712: Remove identity operators from oneDNN optimized graph

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


   Jenkins CI successfully triggered : [centos-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