From d1630c9ac145b962760347b2565fb3274f63cddf Mon Sep 17 00:00:00 2001 From: Mateusz Bencer Date: Thu, 3 Mar 2022 11:22:42 +0100 Subject: [PATCH] Fix problem with segfault during using extension feature via Python (#10650) --- .../src/pyopenvino/frontend/frontend.cpp | 8 +- .../tests/test_frontend/test_frontend_onnx.py | 66 +++++++- .../tests/frontend/shared/src/json_config.cpp | 2 +- .../builtin_extensions.cpp | 4 +- .../test_extension.cpp | 7 + .../test_extension.hpp | 2 + .../mo/openvino/tools/mo/utils/cli_parser.py | 14 +- .../unit_tests/mo/extensions_test_actual.py | 148 ++++++++++++++++++ .../mo/unit_tests/mo/frontend_ngraph_test.py | 9 ++ 9 files changed, 248 insertions(+), 12 deletions(-) create mode 100644 tools/mo/unit_tests/mo/extensions_test_actual.py diff --git a/src/bindings/python/src/pyopenvino/frontend/frontend.cpp b/src/bindings/python/src/pyopenvino/frontend/frontend.cpp index d4f8fde9609..8997bb0697b 100644 --- a/src/bindings/python/src/pyopenvino/frontend/frontend.cpp +++ b/src/bindings/python/src/pyopenvino/frontend/frontend.cpp @@ -43,6 +43,7 @@ void regclass_frontend_FrontEnd(py::module m) { fem.def("convert", static_cast (FrontEnd::*)(const InputModel::Ptr&) const>(&FrontEnd::convert), py::arg("model"), + py::keep_alive<0, 1>(), R"( Completely convert and normalize entire function, throws if it is not possible. @@ -67,16 +68,12 @@ void regclass_frontend_FrontEnd(py::module m) { ---------- function : Model Partially converted nGraph function. - - Returns - ---------- - convert : Model - Fully converted nGraph function. )"); fem.def("convert_partially", &FrontEnd::convert_partially, py::arg("model"), + py::keep_alive<0, 1>(), R"( Convert only those parts of the model that can be converted leaving others as-is. Converted parts are not normalized by additional transformations; normalize function or @@ -96,6 +93,7 @@ void regclass_frontend_FrontEnd(py::module m) { fem.def("decode", &FrontEnd::decode, py::arg("model"), + py::keep_alive<0, 1>(), R"( Convert operations with one-to-one mapping with decoding nodes. Each decoding node is an nGraph node representing a single FW operation node with diff --git a/src/bindings/python/tests/test_frontend/test_frontend_onnx.py b/src/bindings/python/tests/test_frontend/test_frontend_onnx.py index 74914ee2aef..5026b9e2423 100644 --- a/src/bindings/python/tests/test_frontend/test_frontend_onnx.py +++ b/src/bindings/python/tests/test_frontend/test_frontend_onnx.py @@ -6,6 +6,8 @@ import onnx import numpy as np from onnx.helper import make_graph, make_model, make_tensor_value_info import pytest +from pathlib import Path +from itertools import chain from openvino.frontend import FrontEndManager from tests.runtime import get_runtime @@ -26,7 +28,19 @@ def create_onnx_model(): ] output_tensors = [make_tensor_value_info("out", onnx.TensorProto.FLOAT, (2, 2))] graph = make_graph([add, const_node, mul], "graph", input_tensors, output_tensors) - return make_model(graph, producer_name="ngraph ONNX Importer") + return make_model(graph, producer_name="ONNX Frontend") + + +def create_onnx_model_2(): + relu = onnx.helper.make_node("Relu", inputs=["in"], outputs=["out"]) + input_tensors = [ + make_tensor_value_info("in", onnx.TensorProto.FLOAT, (1, 2)), + ] + output_tensors = [ + make_tensor_value_info("out", onnx.TensorProto.FLOAT, (1, 2)), + ] + graph = make_graph([relu], "test_graph", input_tensors, output_tensors) + return make_model(graph, producer_name="ONNX Frontend") def create_onnx_model_with_subgraphs(): @@ -52,7 +66,7 @@ def create_onnx_model_with_subgraphs(): res = onnx.helper.make_tensor_value_info("res", onnx.TensorProto.FLOAT, [3]) graph = make_graph([if_node], "graph", [cond, A, B], [res]) - return make_model(graph, producer_name="ngraph ONNX Importer") + return make_model(graph, producer_name="ONNX Frontend") def create_onnx_model_with_custom_attributes(): @@ -88,7 +102,7 @@ def create_onnx_model_with_custom_attributes(): ] output_tensors = [make_tensor_value_info("out", onnx.TensorProto.FLOAT, (2, 2))] graph = make_graph([add, const_node, mul], "graph", input_tensors, output_tensors) - return make_model(graph, producer_name="ngraph ONNX Importer") + return make_model(graph, producer_name="ONNX Frontend") def create_onnx_model_for_op_extension(): @@ -124,7 +138,7 @@ def create_onnx_model_for_op_extension(): output_tensors = [make_tensor_value_info("out", onnx.TensorProto.FLOAT, (3, 3, 32, 32))] graph = make_graph([const_node, elu, avg_pool, floor, concat, mul, cast], "graph", input_tensors, output_tensors) - return make_model(graph, producer_name="ngraph ONNX Importer") + return make_model(graph, producer_name="ONNX Frontend") def run_function(function, *inputs, expected): @@ -140,6 +154,7 @@ def run_function(function, *inputs, expected): # This is because destroy of FrontEndManager will unload all plugins, no objects shall exist after this fem = FrontEndManager() onnx_model_filename = "model.onnx" +onnx_model_2_filename = "model2.onnx" onnx_model_with_custom_attributes_filename = "model_custom_attributes.onnx" onnx_model_with_subgraphs_filename = "model_subgraphs.onnx" onnx_model_for_op_extension_test = "model_op_extension.onnx" @@ -148,6 +163,7 @@ ONNX_FRONTEND_NAME = "onnx" def setup_module(): onnx.save_model(create_onnx_model(), onnx_model_filename) + onnx.save_model(create_onnx_model_2(), onnx_model_2_filename) onnx.save_model(create_onnx_model_with_custom_attributes(), onnx_model_with_custom_attributes_filename) onnx.save_model(create_onnx_model_with_subgraphs(), onnx_model_with_subgraphs_filename) @@ -156,6 +172,7 @@ def setup_module(): def teardown_module(): os.remove(onnx_model_filename) + os.remove(onnx_model_2_filename) os.remove(onnx_model_with_custom_attributes_filename) os.remove(onnx_model_with_subgraphs_filename) os.remove(onnx_model_for_op_extension_test) @@ -593,3 +610,44 @@ def test_op_extension_via_frontend_extension_map_attributes(): model = ie.read_model(onnx_model_for_op_extension_test) assert model + + +def get_builtin_extensions_path(): + win_folder_path = Path(__file__).parent.parent.parent.parent + linux_folder_path = win_folder_path.joinpath("lib") + for lib_path in chain(win_folder_path.glob("*.dll"), linux_folder_path.glob("*.so")): + if "libtest_builtin_extensions_1" in lib_path.name: + return str(lib_path) + return "" + + +@pytest.mark.skipif(len(get_builtin_extensions_path()) == 0, + reason="The extension library path was not found") +def test_so_extension_via_frontend_convert_input_model(): + skip_if_onnx_frontend_is_disabled() + + def load_model(): + fe = fem.load_by_framework(framework=ONNX_FRONTEND_NAME) + fe.add_extension(get_builtin_extensions_path()) + in_model = fe.load(onnx_model_2_filename) + return fe.convert(in_model) + + model = load_model() # model has longer lifetime than frontend + + assert any(op.get_type_name() == "Swish" for op in model.get_ops()) + assert all(op.get_type_name() != "Relu" for op in model.get_ops()) + + +@pytest.mark.skipif(len(get_builtin_extensions_path()) == 0, + reason="The extension library path was not found") +def test_so_extension_via_frontend_decode_input_model(): + skip_if_onnx_frontend_is_disabled() + + def load_decoded_model(): + fe = fem.load_by_framework(framework=ONNX_FRONTEND_NAME) + fe.add_extension(get_builtin_extensions_path()) + in_model = fe.load(onnx_model_2_filename) + return fe.decode(in_model) + + decoded_model = load_decoded_model() # decoded model has longer lifetime than frontend + assert decoded_model diff --git a/src/core/tests/frontend/shared/src/json_config.cpp b/src/core/tests/frontend/shared/src/json_config.cpp index 64f5844e781..6caff415aab 100644 --- a/src/core/tests/frontend/shared/src/json_config.cpp +++ b/src/core/tests/frontend/shared/src/json_config.cpp @@ -143,7 +143,7 @@ TEST_P(FrontEndJsonConfigTest, testAddJsonConfigExtension) { auto target_ext = json_config_ext->get_target_extensions(); // the number of Loaded extensions can be more than the number of actually used ones. - EXPECT_EQ(loaded_ext.size(), 8); + EXPECT_EQ(loaded_ext.size(), 9); EXPECT_EQ(target_ext.size(), 3); for (const auto& target : target_ext) { diff --git a/src/core/tests/frontend/shared/test_builtin_extensions_1/builtin_extensions.cpp b/src/core/tests/frontend/shared/test_builtin_extensions_1/builtin_extensions.cpp index 905cb7d18fe..6fc98b25e29 100644 --- a/src/core/tests/frontend/shared/test_builtin_extensions_1/builtin_extensions.cpp +++ b/src/core/tests/frontend/shared/test_builtin_extensions_1/builtin_extensions.cpp @@ -7,7 +7,9 @@ #ifdef ENABLE_OV_ONNX_FRONTEND # include -# define ONNX_EXT std::make_shared("NewCustomOp_3", CustomTranslatorONNX), +# define ONNX_EXT \ + std::make_shared("NewCustomOp_3", CustomTranslatorONNX), \ + std::make_shared("Relu", ReluToSwishTranslatorONNX), #else # define ONNX_EXT #endif diff --git a/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.cpp b/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.cpp index 363860a9245..9ff0a415097 100644 --- a/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.cpp +++ b/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.cpp @@ -6,6 +6,8 @@ #include +#include "openvino/opsets/opset8.hpp" + bool TestExtension1::transform(const std::shared_ptr& function, const std::string& config) const { function->set_friendly_name("TestFunction"); return true; @@ -29,6 +31,11 @@ ov::OutputVector CustomTranslatorONNX(const ov::frontend::NodeContext& node) { return ov::OutputVector(); } +ov::OutputVector ReluToSwishTranslatorONNX(const ov::frontend::NodeContext& node) { + auto swish = std::make_shared(node.get_input(0)); + return {swish}; +} + std::map CustomTranslatorPaddle(const ov::frontend::NodeContext& node) { return std::map(); } diff --git a/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.hpp b/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.hpp index 3bb978a7e36..7d8b79bbc4f 100644 --- a/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.hpp +++ b/src/core/tests/frontend/shared/test_builtin_extensions_1/test_extension.hpp @@ -23,4 +23,6 @@ ov::OutputVector CustomTranslatorTensorflow(const ov::frontend::NodeContext& nod ov::OutputVector CustomTranslatorONNX(const ov::frontend::NodeContext& node); +ov::OutputVector ReluToSwishTranslatorONNX(const ov::frontend::NodeContext& node); + std::map CustomTranslatorPaddle(const ov::frontend::NodeContext& node); diff --git a/tools/mo/openvino/tools/mo/utils/cli_parser.py b/tools/mo/openvino/tools/mo/utils/cli_parser.py index 29b2ff096dc..a363e1298f3 100644 --- a/tools/mo/openvino/tools/mo/utils/cli_parser.py +++ b/tools/mo/openvino/tools/mo/utils/cli_parser.py @@ -191,6 +191,18 @@ def readable_dirs_or_empty(paths: str): return paths +def readable_dirs_or_files_or_empty(paths: str): + """ + Checks that comma separated list of paths are readable directories, files or a provided path is empty. + :param paths: comma separated list of paths. + :return: comma separated list of paths. + """ + if paths: + paths_list = [readable_file_or_dir(path) for path in paths.split(',')] + return ','.join(paths_list) + return paths + + def readable_dir(path: str): """ Check that specified path is a readable directory. @@ -395,7 +407,7 @@ def get_common_cli_parser(parser: argparse.ArgumentParser = None): "string.", default=import_extensions.default_path(), action=CanonicalizePathCheckExistenceAction, - type=readable_dirs_or_empty) + type=readable_dirs_or_files_or_empty) common_group.add_argument("--batch", "-b", type=check_positive, default=None, diff --git a/tools/mo/unit_tests/mo/extensions_test_actual.py b/tools/mo/unit_tests/mo/extensions_test_actual.py new file mode 100644 index 00000000000..c8f6bf12367 --- /dev/null +++ b/tools/mo/unit_tests/mo/extensions_test_actual.py @@ -0,0 +1,148 @@ +# Copyright (C) 2022 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +import pytest +import unittest +from unittest.mock import Mock +import onnx +from onnx.helper import make_graph, make_model, make_tensor_value_info +import os +from os import path +import json +import argparse +from pathlib import Path +from itertools import chain +from openvino.tools.mo.main import prepare_ir +from openvino.frontend import ( + FrontEndManager, +) # pylint: disable=no-name-in-module,import-error + +try: + import openvino_telemetry as tm +except ImportError: + import openvino.tools.mo.utils.telemetry_stub as tm + + +def base_args_config(): + args = argparse.Namespace() + args.feManager = FrontEndManager() + args.extensions = None + args.use_legacy_frontend = False + args.use_new_frontend = True + args.framework = "onnx" + args.model_name = None + args.input_model = None + args.silent = True + args.transform = [] + args.legacy_ir_generation = False + args.scale = None + args.output = None + args.input = None + args.input_shape = None + args.batch = None + args.mean_values = None + args.scale_values = None + args.output_dir = os.getcwd() + args.freeze_placeholder_with_value = None + args.transformations_config = None + args.disable_fusing = None + args.finegrain_fusing = None + args.disable_gfusing = None + args.disable_resnet_optimization = None + args.enable_concat_optimization = None + args.static_shape = None + args.disable_weights_compression = None + args.reverse_input_channels = None + args.data_type = None + args.layout = None + args.source_layout = None + args.target_layout = None + return args + + +def get_builtin_extensions_path(): + win_folder_path = Path(__file__).parent.parent.parent.parent + linux_folder_path = win_folder_path.joinpath("lib") + for lib_path in chain( + win_folder_path.glob("*.dll"), linux_folder_path.glob("*.so") + ): + if "libtest_builtin_extensions_1" in lib_path.name: + return str(lib_path) + return "" + + +class TestMoFallback(unittest.TestCase): + def setUp(self): + tm.Telemetry.__init__ = Mock(return_value=None) + tm.Telemetry.send_event = Mock() + + self.models = {} + relu = onnx.helper.make_node("Relu", inputs=["in"], outputs=["out"]) + input_tensors = [ + make_tensor_value_info("in", onnx.TensorProto.FLOAT, (1, 2)), + ] + output_tensors = [ + make_tensor_value_info("out", onnx.TensorProto.FLOAT, (1, 2)), + ] + graph = make_graph([relu], "test_graph", input_tensors, output_tensors) + model = make_model( + graph, + producer_name="MO tests", + opset_imports=[onnx.helper.make_opsetid("", 13)], + ) + self.models["test_model.onnx"] = model + + self.test_config_files = {} + self.test_config_files[ + "test_config.json" + ] = """[ + { + "custom_attributes": { + "test_attribute": true + }, + "id": "buildin_extensions_1::TestExtension1", + "library": "library_path", + "match_kind": "scope" + } + ]""".replace( + "library_path", get_builtin_extensions_path() + ) + + for name, model in self.models.items(): + onnx.save(model, name) + for name, config in self.test_config_files.items(): + with open(name, "w") as f: + f.write(config) + + def tearDown(self): + for name in self.models.keys(): + os.remove(name) + for name in self.test_config_files.keys(): + os.remove(name) + + @pytest.mark.skipif( + len(get_builtin_extensions_path()) == 0, + reason="The extension library path was not found", + ) + def test_conersion_if_extensions_is_used(self): + args = base_args_config() + args.input_model = "test_model.onnx" + args.extensions = get_builtin_extensions_path() + + graph, model = prepare_ir(args) + + assert any(op.get_type_name() == "Swish" for op in model.get_ops()) + assert all(op.get_type_name() != "Relu" for op in model.get_ops()) + + @pytest.mark.skipif( + len(get_builtin_extensions_path()) == 0, + reason="The extension library path was not found", + ) + def test_conersion_if_transformations_config_is_used(self): + args = base_args_config() + args.input_model = "test_model.onnx" + args.transformations_config = "test_config.json" + + graph, model = prepare_ir(args) + + assert model.get_friendly_name() == "TestFunction" diff --git a/tools/mo/unit_tests/mo/frontend_ngraph_test.py b/tools/mo/unit_tests/mo/frontend_ngraph_test.py index fa99ed8c228..24cc8e215f9 100644 --- a/tools/mo/unit_tests/mo/frontend_ngraph_test.py +++ b/tools/mo/unit_tests/mo/frontend_ngraph_test.py @@ -61,6 +61,15 @@ def test_main_test(): assert not status.returncode +def test_mo_extensions_test(): + setup_env() + args = [sys.executable, '-m', 'pytest', + os.path.join(os.path.dirname(__file__), 'extensions_test_actual.py'), '-s'] + + status = subprocess.run(args, env=os.environ) + assert not status.returncode + + def test_mo_fallback_test(): setup_env() args = [sys.executable, '-m', 'pytest',