[ONNX Importer] Resolved problem with incorrect opset version set in subgraph (#2959)
authorMateusz Bencer <mateusz.bencer@intel.com>
Tue, 10 Nov 2020 08:57:06 +0000 (09:57 +0100)
committerGitHub <noreply@github.com>
Tue, 10 Nov 2020 08:57:06 +0000 (09:57 +0100)
ngraph/frontend/onnx_import/include/onnx_import/core/graph.hpp
ngraph/frontend/onnx_import/include/onnx_import/core/model.hpp
ngraph/frontend/onnx_import/src/core/attribute.cpp
ngraph/frontend/onnx_import/src/core/graph.cpp
ngraph/frontend/onnx_import/src/core/model.cpp
ngraph/python/tests/__init__.py
ngraph/python/tests/test_onnx/test_zoo_models.py
ngraph/test/models/onnx/loop/loop_2d_mul_opset1.prototxt [new file with mode: 0644]
ngraph/test/onnx/onnx_import_controlflow.in.cpp

index 9697323..40a9da7 100644 (file)
@@ -46,6 +46,7 @@ namespace ngraph
             const std::string& get_name() const { return m_graph_proto->name(); }
             OutputVector make_ng_nodes(const Node& onnx_node) const;
             const GraphCache& get_graph_cache() const;
+            const OpsetImports& get_opset_imports() const;
 
         protected:
             Graph(const ONNX_NAMESPACE::GraphProto& proto,
index c1ee6fb..d79ee82 100644 (file)
@@ -27,6 +27,10 @@ namespace ngraph
 {
     namespace onnx_import
     {
+        /// \brief      Type of container which stores opset version and domain in ONNX format
+        using OpsetImports =
+            ::google::protobuf::RepeatedPtrField<ONNX_NAMESPACE::OperatorSetIdProto>;
+
         std::string get_node_domain(const ONNX_NAMESPACE::NodeProto& node_proto);
 
         std::int64_t get_opset_version(const ONNX_NAMESPACE::ModelProto& model_proto,
@@ -47,6 +51,7 @@ namespace ngraph
             const std::string& get_producer_name() const { return m_model_proto->producer_name(); }
             const ONNX_NAMESPACE::GraphProto& get_graph() const { return m_model_proto->graph(); }
             std::int64_t get_model_version() const { return m_model_proto->model_version(); }
+            const OpsetImports& get_opset_imports() const;
             const std::string& get_producer_version() const
             {
                 return m_model_proto->producer_version();
index f1a9b3e..925379d 100644 (file)
@@ -42,10 +42,10 @@ namespace ngraph
             ONNX_NAMESPACE::ModelProto model_proto;
             const auto& graph = m_attribute_proto->g();
             *(model_proto.mutable_graph()) = graph;
-            // We're creating here a model with unset `opset_import` field. This shouldn't
-            // be a problem, since we add ONNX opset as a default available opset. Moreover
-            // if we encounter a node absent in current available opsets we will try
-            // to add it's domain to available opsets.
+
+            // set opset version and domain from the parent graph
+            *model_proto.mutable_opset_import() = parent_graph.get_opset_imports();
+
             Model model{model_proto};
             return Subgraph{graph, model, parent_graph};
         }
index 0287bbb..7647917 100644 (file)
@@ -296,6 +296,11 @@ namespace ngraph
                 as_node_vector(ng_inputs));
         }
 
+        const OpsetImports& Graph::get_opset_imports() const
+        {
+            return m_model->get_opset_imports();
+        }
+
         Subgraph::Subgraph(const ONNX_NAMESPACE::GraphProto& proto,
                            Model& model,
                            const Graph& parent_graph)
index 7d0df69..4e4e5e4 100644 (file)
@@ -110,6 +110,11 @@ namespace ngraph
             }
         }
 
+        const OpsetImports& Model::get_opset_imports() const
+        {
+            return m_model_proto->opset_import();
+        }
+
     } // namespace onnx_import
 
 } // namespace ngraph
index b039758..80e93c8 100644 (file)
@@ -137,10 +137,10 @@ xfail_issue_38714 = xfail_test(reason="RuntimeError: While validating ONNX node
                                       "While validating node 'v1::<name> (sizes[0]:i64{4},"
                                       "Convert_29306[0]:f32{4}) -> (dynamic?)' with friendly_name '<name>':"
                                       "Argument element types are inconsistent.")
-xfail_issue_41813 = xfail_test(reason="RuntimeError: While validating ONNX node"
-                                      " '<Node(Loop): generic_loop_Loop__69>'"
-                                      "While validating ONNX node '<Node(Resize): Resize__143>':"
-                                      "vector::_M_range_check: __n (which is 2) >= this->size() (which is 2")
+xfail_issue_42297 = xfail_test(reason="RuntimeError: While validating ONNX node '<Node(Conv): Conv__7398>':"
+                                      "Check 'data.get_partial_shape().rank().is_static()'"
+                                      " failed at ngraph/frontend/onnx_import/src/op/conv.cpp:102:"
+                                      "The input data tensor's rank has to be known (static)")
 xfail_issue_41814 = xfail_test(reason="RuntimeError: While validating ONNX node '<Node(Loop):"
                                       " generic_loop_Loop__121>':"
                                       "While validating ONNX node '<Node(TopK):"
index ec2a7ad..9226ce3 100644 (file)
@@ -25,7 +25,7 @@ from tests.test_onnx.utils.model_importer import ModelImportRunner
 
 from tests import (
     xfail_issue_38701,
-    xfail_issue_41813,
+    xfail_issue_42297,
     xfail_issue_41815,
     xfail_issue_41814,
     xfail_issue_36533,
@@ -116,11 +116,11 @@ if len(zoo_models) > 0:
         import_xfail_list = [
             # ONNX Model Zoo
             (xfail_issue_38701, "test_onnx_model_zoo_text_machine_comprehension_bidirectional_attention_flow_model_bidaf_9_bidaf_bidaf_cpu"),
-            (xfail_issue_41813, "test_onnx_model_zoo_vision_object_detection_segmentation_ssd_mobilenetv1_model_ssd_mobilenet_v1_10_ssd_mobilenet_v1_ssd_mobilenet_v1_cpu"),
+            (xfail_issue_42297, "test_onnx_model_zoo_vision_object_detection_segmentation_ssd_mobilenetv1_model_ssd_mobilenet_v1_10_ssd_mobilenet_v1_ssd_mobilenet_v1_cpu"),
             (xfail_issue_38726, "test_onnx_model_zoo_text_machine_comprehension_t5_model_t5_decoder_with_lm_head_12_t5_decoder_with_lm_head_cpu"),
 
             # Model MSFT
-            (xfail_issue_41813, "test_MSFT_opset10_mlperf_ssd_mobilenet_300_ssd_mobilenet_v1_coco_2018_01_28_cpu"),
+            (xfail_issue_42297, "test_MSFT_opset10_mlperf_ssd_mobilenet_300_ssd_mobilenet_v1_coco_2018_01_28_cpu"),
             (xfail_issue_41814, "test_MSFT_opset10_mlperf_ssd_resnet34_1200_ssd_resnet34_mAP_20.2_cpu"),
             (xfail_issue_37957, "test_MSFT_opset10_mask_rcnn_keras_mask_rcnn_keras_cpu"),
             (xfail_issue_36465, "test_MSFT_opset9_LSTM_Seq_lens_unpacked_model_cpu"),
diff --git a/ngraph/test/models/onnx/loop/loop_2d_mul_opset1.prototxt b/ngraph/test/models/onnx/loop/loop_2d_mul_opset1.prototxt
new file mode 100644 (file)
index 0000000..141b240
--- /dev/null
@@ -0,0 +1,158 @@
+ir_version: 6
+producer_name: "nGraph ONNX Importer"
+graph {
+  name: "basic loop"
+  node {
+    input: "trip_count"
+    input: ""
+    input: "a_init"
+    output: "a_final"
+    output: "a_values"
+    op_type: "Loop"
+    attribute {
+      name: "body"
+      g {
+        node {
+          input: "a_in"
+          input: "b"
+          output: "current_a"
+          name: "loop_body_mul"
+          op_type: "Mul"
+        }
+        node {
+          input: "cond_in"
+          output: "cond_out"
+          name: "cond_identity"
+          op_type: "Identity"
+        }
+        node {
+          input: "current_a"
+          output: "a_out"
+          name: "output_accumulator"
+          op_type: "Identity"
+        }
+        name: "simple add"
+        initializer {
+          dims: 1
+          dims: 2
+          data_type: 1
+          float_data: 1
+          float_data: 1
+          name: "b"
+        }
+        input {
+          name: "i"
+          type {
+            tensor_type {
+              elem_type: 7
+              shape {
+              }
+            }
+          }
+        }
+        input {
+          name: "cond_in"
+          type {
+            tensor_type {
+              elem_type: 9
+              shape {
+              }
+            }
+          }
+        }
+        input {
+          name: "a_in"
+          type {
+            tensor_type {
+              elem_type: 1
+              shape {
+                dim {
+                  dim_value: 1
+                }
+                dim {
+                  dim_value: 2
+                }
+              }
+            }
+          }
+        }
+        output {
+          name: "cond_out"
+          type {
+            tensor_type {
+              elem_type: 9
+              shape {
+              }
+            }
+          }
+        }
+        output {
+          name: "current_a"
+          type {
+            tensor_type {
+              elem_type: 1
+              shape {
+              }
+            }
+          }
+        }
+        output {
+          name: "a_out"
+          type {
+            tensor_type {
+              elem_type: 1
+              shape {
+              }
+            }
+          }
+        }
+      }
+      type: GRAPH
+    }
+  }
+  initializer {
+    dims: 1
+    data_type: 7
+    int64_data: 3
+    name: "trip_count"
+  }
+  input {
+    name: "a_init"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 1
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+  output {
+    name: "a_final"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+        }
+      }
+    }
+  }
+  output {
+    name: "a_values"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+        }
+      }
+    }
+  }
+}
+opset_import {
+  version: 1
+}
index 23e4e72..640a7fb 100644 (file)
@@ -16,6 +16,7 @@
 
 #include "gtest/gtest.h"
 #include "ngraph/file_util.hpp"
+#include "ngraph/type.hpp"
 #include "ngraph/type/element_type.hpp"
 #include "onnx_import/default_opset.hpp"
 #include "onnx_import/onnx.hpp"
@@ -219,6 +220,29 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_controlflow_loop_add_value_the_same_node_from_
     test_case.run();
 }
 
+NGRAPH_TEST(${BACKEND_NAME}, onnx_controlflow_loop_the_proper_opset_in_subgraph)
+{
+    const auto function = onnx_import::import_onnx_model(
+        file_util::path_join(SERIALIZED_ZOO, "onnx/loop/loop_2d_mul_opset1.prototxt"));
+
+    const auto parent_ops = function->get_ops();
+    const auto loop_node_it =
+        std::find_if(parent_ops.begin(), parent_ops.end(), [](const std::shared_ptr<Node>& op) {
+            return std::string{op->get_type_name()} == "Loop";
+        });
+    const auto body_ops =
+        ngraph::as_type_ptr<ngraph::opset5::Loop>(*loop_node_it)->get_function()->get_ops();
+    const auto body_mul_node_it =
+        std::find_if(body_ops.begin(), body_ops.end(), [](const std::shared_ptr<Node>& op) {
+            return std::string{op->get_type_name()} == "Multiply";
+        });
+    const auto body_mul_node = ngraph::as_type_ptr<ngraph::opset5::Multiply>(*body_mul_node_it);
+    EXPECT_TRUE(body_mul_node);
+    EXPECT_EQ(
+        body_mul_node->get_autob().m_type,
+        ngraph::op::AutoBroadcastType::NONE); // legacy Mul from ONNX opset1 has NONE broadcasting
+}
+
 // ~~~~~~~~STATIC/DYNAMIC/CONSTANT INPUTS TESTS:~~~~~~~~
 
 NGRAPH_TEST(${BACKEND_NAME}, onnx_controlflow_loop_scalars)