[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on a change in pull request #20767: Fix identity fuse for oneDNN

[GitBox <gi...@apache.org>]

PawelGlomski-Intel commented on a change in pull request #20767:
URL: https://github.com/apache/incubator-mxnet/pull/20767#discussion_r766527196



##########
File path: src/operator/subgraph/dnnl/dnnl_identity_property.h
##########
@@ -65,10 +66,11 @@ class SgDNNLIdentitySelector : public SubgraphSelectorV2 {
   }
 
   bool SelectInput(const BiDirectedNode& n, const BiDirectedNode& input_node) override {
-    if (input_node.node->is_variable()) {
+    if (pattern_found || input_node.node->is_variable()) {

Review comment:
       You could use `matched_list_` like you did in the `Filter` function.
   ```suggestion
       if (matched_list_.size() == 2 || input_node.node->is_variable()) {
   ```
   If not, then I would also use `pattern_found` in the `Filter` function, to show, that these conditions are connected.
   https://github.com/apache/incubator-mxnet/blob/7e163157cf789d7ec5e7272532e04495e57bd0ba/src/operator/subgraph/dnnl/dnnl_identity_property.h#L85
   Change to :
   ```
       if (pattern_found && candidates.size() == matched_list_.size()) { 
   ```




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