Skip to content

Commit

Permalink
[ONNX Importer] Resolved problem with incorrect opset version set in …
Browse files Browse the repository at this point in the history
…subgraph (openvinotoolkit#2959)
  • Loading branch information
Mateusz Bencer authored Nov 10, 2020
1 parent 0a71ada commit 0baad06
Show file tree
Hide file tree
Showing 9 changed files with 209 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions ngraph/frontend/onnx_import/src/core/attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
Expand Down
5 changes: 5 additions & 0 deletions ngraph/frontend/onnx_import/src/core/graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions ngraph/frontend/onnx_import/src/core/model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ namespace ngraph
}
}

const OpsetImports& Model::get_opset_imports() const
{
return m_model_proto->opset_import();
}

} // namespace onnx_import

} // namespace ngraph
8 changes: 4 additions & 4 deletions ngraph/python/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ def xfail_test(reason="Mark the test as expected to fail", strict=True):
"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):"
Expand Down
6 changes: 3 additions & 3 deletions ngraph/python/tests/test_onnx/test_zoo_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

from tests import (
xfail_issue_38701,
xfail_issue_41813,
xfail_issue_42297,
xfail_issue_41815,
xfail_issue_41814,
xfail_issue_36533,
Expand Down Expand Up @@ -116,11 +116,11 @@
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"),
Expand Down
158 changes: 158 additions & 0 deletions ngraph/test/models/onnx/loop/loop_2d_mul_opset1.prototxt
Original file line number Diff line number Diff line change
@@ -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
}
24 changes: 24 additions & 0 deletions ngraph/test/onnx/onnx_import_controlflow.in.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 0baad06

Please sign in to comment.