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 2022/05/09 17:14:10 UTC

[GitHub] [incubator-mxnet] bartekkuncer opened a new pull request, #21020: [FEATURE] Add property removing duplicate Cast operations

bartekkuncer opened a new pull request, #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020

   … subgraph backend
   
   ## Description ##
   (Brief description on what this PR is about)
   
   ## 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 backward incompatible change, why must this change be made.
   - Interesting edge cases to note here
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] DominikaJedynak commented on a diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
DominikaJedynak commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r884977232


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   Maybe it could be:
   ```suggestion
       if (output_node.node->op() == Op::Get("Cast")
           && output_node.node->attrs.dict["dtype"] == boolStr) {
   ```



##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;

Review Comment:
   Is kStart status actually necessary? It seems that it is not used, maybe we could just set status_ = kFail in here?



##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {
+        if (status_ == kExpand) {
+          if (output_node.node->num_outputs() == 1) {

Review Comment:
   ```suggestion
           if (status_ == kExpand && output_node.node->num_outputs() == 1) {
   ```



-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
anko-intel commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882550653


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {

Review Comment:
   Could it will be done of dtype's? Probably we can save dtype in Select and use it for input and output selector



##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   can we used parsed parameter in this place ?



-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882555018


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {

Review Comment:
   Select selects expand_dims which does not have dtype. This fuse is very specific therefore I did not even try to make it more generic.



-- 
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 pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#issuecomment-1156323819

   @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 #21020: [FEATURE] Add property removing duplicate Cast operations

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

   Jenkins CI successfully triggered : [edge, windows-gpu, clang, centos-gpu, centos-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.

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 #21020: [FEATURE] Add property removing duplicate Cast operations

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

   Hey @bartekkuncer , 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, windows-cpu, centos-cpu, sanity, clang, unix-gpu, unix-cpu, windows-gpu, website, miscellaneous, 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.

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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r885271522


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   Checking second condition only makes sense when the first one is true, therefore it is done in this way.



-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r885271522


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   I did it in this way to underline the fact that checking second condition only makes sense when the first one is true.



-- 
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 pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#issuecomment-1156060102

   @mxnet-bot run ci [unix-cpu, unix-gpu, website]


-- 
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 pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#issuecomment-1147336016

   @mxnet-bot run ci [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] bartekkuncer commented on pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#issuecomment-1138319000

   @mxnet-bot run ci [miscellaneous, unix-cpu, unix-gpu, website]


-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
anko-intel commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r886417166


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kExpand, kCast, kSuccess, kFail };
+  CastStatus status_ = kFail;
+  int castDtype      = -1;

Review Comment:
   I think it will be good to add some comments what this variable is for. Something like: "It is used to check if Cast dtype on expand_dims input has the same dtype of Cast on expand_dism output"



-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882584228


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {

Review Comment:
   I will, thanks for your input :)



-- 
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 #21020: [FEATURE] Add property removing duplicate Cast operations

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


-- 
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 pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#issuecomment-1145784636

   @mxnet-bot run ci [centos-cpu, centos-gpu, windows-gpu, edge, 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.

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 #21020: [FEATURE] Add property removing duplicate Cast operations

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

   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] anko-intel commented on a diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
anko-intel commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882550653


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {

Review Comment:
   Could it will be done for all dtype's? Probably we can save dtype in Select and use it for input and output selector



-- 
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 #21020: [FEATURE] Add property removing duplicate Cast operations

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

   Jenkins CI successfully triggered : [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] mxnet-bot commented on pull request #21020: [FEATURE] Add property removing duplicate Cast operations

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

   Jenkins CI successfully triggered : [unix-gpu, unix-cpu, website]


-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r885352921


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;

Review Comment:
   Removed kStart.



-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
anko-intel commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882569950


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   I encourage to use  parsed version only if they are already parsed- what I hope is already done, if not of course there is no sense parse it 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.

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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
anko-intel commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882571427


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {

Review Comment:
   So maybe you can  notice the type of the cast in SelectInput and check it in SelectOutput (or vice versa)



-- 
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 #21020: [FEATURE] Add property removing duplicate Cast operations

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

   Jenkins CI successfully triggered : [unix-cpu, miscellaneous, unix-gpu, website]


-- 
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 diff in pull request #21020: [FEATURE] Add property removing duplicate Cast operations

Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on code in PR #21020:
URL: https://github.com/apache/incubator-mxnet/pull/21020#discussion_r882556495


##########
src/operator/subgraph/dnnl/dnnl_remove_casts_property.h:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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_remove_casts_property.h
+ * \brief Graph property for removing two unnecessary Cast operations
+ *
+ * ... -> Cast(bool) -> expand_dims -> Cast(bool) -> Cast(bool) -> ...
+ *                                  ||
+ *                                  \/
+ *                ... -> Cast(bool) -> expand_dims -> ...
+ */
+
+#ifndef MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+#define MXNET_OPERATOR_SUBGRAPH_DNNL_DNNL_REMOVE_CASTS_PROPERTY_H_
+
+#if MXNET_USE_ONEDNN == 1
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "operator/subgraph/common.h"
+#include "dnnl_subgraph_base-inl.h"
+
+namespace mxnet {
+namespace op {
+
+class SgDNNLRemoveCastsSelector : public SubgraphSelectorV2 {
+ private:
+  enum CastStatus { kStart, kExpand, kCast, kSuccess, kFail };
+  CastStatus status_  = kStart;
+  const char* boolStr = "bool";
+
+ public:
+  bool Select(const BiDirectedNode& seed_node,
+              const std::shared_ptr<NodeAttr>& node_attr) override {
+    if (seed_node.node->op() == Op::Get("expand_dims") && seed_node.node->num_inputs() == 1 &&
+        seed_node.node->num_outputs() == 1) {
+      status_ = kExpand;
+      return true;
+    }
+    return false;
+  }
+
+  bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
+    if (input_node.node->op() != Op::Get("Cast")) {
+      status_ = kFail;
+    } else if (input_node.node->attrs.dict["dtype"] != boolStr) {
+      status_ = kFail;
+    }
+    return false;
+  }
+
+  bool SelectOutput(const BiDirectedNode& n, const BiDirectedNode& output_node) override {
+    if (status_ == kFail || status_ == kSuccess || output_node.node->is_variable()) {
+      return false;
+    }
+    if (output_node.node->op() == Op::Get("Cast")) {
+      if (output_node.node->attrs.dict["dtype"] == boolStr) {

Review Comment:
   I guess we could use parsed parameter here but is it really needed? To me parsing attrs to get one parameter just for a simple comparison is a waste of time.



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