From 3c0101c9a907e2582443898e48cb2218bd4d06a4 Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Tue, 6 Jul 2021 19:47:56 +0300 Subject: [PATCH 1/8] [MO] Add support to moc_frontend of ":" as delimiter for --input Additions: Changed default logic for 'Place::get_in(out)put_port' to return nullptr Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr --- model-optimizer/mo/front/extractor.py | 17 +- model-optimizer/mo/moc_frontend/extractor.py | 63 ++++- .../unit_tests/mo/frontend_ngraph_test.py | 9 + .../moc_frontend/moc_extractor_test_actual.py | 239 ++++++++++++++++++ .../mock_mo_frontend.hpp | 63 ++++- .../mock_mo_python_api/mock_mo_python_api.cpp | 2 + .../include/frontend_manager/input_model.hpp | 17 +- .../include/frontend_manager/place.hpp | 16 +- .../frontend_manager/src/frontend_manager.cpp | 26 +- .../mock_py_frontend.hpp | 2 +- ngraph/test/frontend/frontend_manager.cpp | 24 +- 11 files changed, 415 insertions(+), 63 deletions(-) create mode 100644 model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py diff --git a/model-optimizer/mo/front/extractor.py b/model-optimizer/mo/front/extractor.py index a0376a6328ce9b..44173dde07dcd5 100644 --- a/model-optimizer/mo/front/extractor.py +++ b/model-optimizer/mo/front/extractor.py @@ -451,6 +451,16 @@ def extract_node_attrs(graph: Graph, extractor: callable): return graph +def raise_no_node(node_name: str): + raise Error('No node with name {}'.format(node_name)) + +def raise_node_name_collision(node_name: str, found_nodes: list): + raise Error('Name collision was found, there are several nodes for mask "{}": {}. ' + 'If your intention was to specify port for node, please instead specify node names connected to ' + 'this port. If your intention was to specify the node name, please add port to the node ' + 'name'.format(node_name, found_nodes)) + + def get_node_id_with_ports(graph: Graph, node_name: str, skip_if_no_port=True): """ Extracts port and node ID out of user provided name @@ -483,12 +493,9 @@ def get_node_id_with_ports(graph: Graph, node_name: str, skip_if_no_port=True): found_names.append((in_port, out_port, name)) if len(found_names) == 0: - raise Error('No node with name {}'.format(node_name)) + raise_no_node(node_name) if len(found_names) > 1: - raise Error('Name collision was found, there are several nodes for mask "{}": {}. ' - 'If your intention was to specify port for node, please instead specify node names connected to ' - 'this port. If your intention was to specify the node name, please add port to the node ' - 'name'.format(node_name, [name for _, _, name in found_names])) + raise_node_name_collision(node_name, [name for _, _, name in found_names]) in_port, out_port, name = found_names[0] node_id = graph.get_node_id_by_name(name) if in_port is not None: diff --git a/model-optimizer/mo/moc_frontend/extractor.py b/model-optimizer/mo/moc_frontend/extractor.py index 95f463bb8fb04f..4462ea804eb6db 100644 --- a/model-optimizer/mo/moc_frontend/extractor.py +++ b/model-optimizer/mo/moc_frontend/extractor.py @@ -1,16 +1,14 @@ # Copyright (C) 2018-2021 Intel Corporation # SPDX-License-Identifier: Apache-2.0 -import logging as log import re -from collections import defaultdict -from copy import copy - -import numpy as np +from mo.front.extractor import raise_no_node, raise_node_name_collision from mo.utils.error import Error -from ngraph.frontend import InputModel # pylint: disable=no-name-in-module,import-error +from ngraph.frontend import InputModel, Place # pylint: disable=no-name-in-module,import-error + +import numpy as np def decode_name_with_port(input_model: InputModel, node_name: str): @@ -21,15 +19,60 @@ def decode_name_with_port(input_model: InputModel, node_name: str): :param node_name: user provided node name :return: decoded place in the graph """ - # Check exact match with one of the names in the graph first + found_nodes = [] + found_node_names = [] + + """ + Helper function to add Place node to list of found nodes. Adds only if node is not None + :param found_nodes List[Place]: list of found nodes + :param node Place: Place node to add to list + """ + def add_found_node(found_nodes: list, node: Place): + found_nodes.append(node) if node else None + node = input_model.get_place_by_tensor_name(node_name) - if node: - return node + found_node_names.append('Tensor:' + node_name) + add_found_node(found_nodes, node) + + node = input_model.get_place_by_operation_name(node_name) + add_found_node(found_nodes, node) + found_node_names.append('Operation:' + node_name) + + regexp_post = r'(.+):(\d+)' + match_post = re.search(regexp_post, node_name) + if match_post: + found_node_names.append(match_post.group(1)) + node_post = input_model.get_place_by_operation_name(match_post.group(1)) + add_found_node(found_nodes, node_post.get_output_port( + outputPortIndex=int(match_post.group(2)))) if node_post else None + + regexp_pre = r'(\d+):(.+)' + match_pre = re.search(regexp_pre, node_name) + if match_pre: + found_node_names.append(match_pre.group(2)) + node_pre = input_model.get_place_by_operation_name(match_pre.group(2)) + add_found_node(found_nodes, node_pre.get_input_port( + inputPortIndex=int(match_pre.group(1)))) if node_pre else None + + if len(found_nodes) == 0: + raise_no_node(node_name) + + # Check that there is no collision, all found places shall point to same data + if len(found_nodes) > 1: + all_equal = all([n.is_equal_data(found_nodes[0]) for n in found_nodes]) + if not all_equal: + raise_node_name_collision(node_name, found_node_names) + + # TODO: ONNX specific (59408) + # To comply with legacy behavior, for ONNX-only there shall be considered additional 2 possibilities + # 1) "abc:1" - get_place_by_tensor_name("abc").get_producing_operation().get_output_port(1) + # 2) "1:abc" - get_place_by_tensor_name("abc").get_producing_operation().get_input_port(1) + # This logic is not going to work with other frontends # TODO: Add support for input/output group name and port index here (58562) # Legacy frontends use format "number:name:number" to specify input and output port indices # For new frontends this logic shall be extended to additionally support input and output group names - raise Error('There is no node with name {}'.format(node_name)) + return found_nodes[0] def fe_input_user_data_repack(input_model: InputModel, input_user_shapes: [None, list, dict, np.ndarray], diff --git a/model-optimizer/unit_tests/mo/frontend_ngraph_test.py b/model-optimizer/unit_tests/mo/frontend_ngraph_test.py index af747b4132868b..d0b8d13274d5a5 100644 --- a/model-optimizer/unit_tests/mo/frontend_ngraph_test.py +++ b/model-optimizer/unit_tests/mo/frontend_ngraph_test.py @@ -34,6 +34,15 @@ def test_frontends(): assert not status.returncode +def test_moc_extractor(): + setup_env() + args = [sys.executable, '-m', 'pytest', + './moc_frontend/moc_extractor_test_actual.py', '-s'] + + status = subprocess.run(args, env=os.environ) + assert not status.returncode + + def test_main_test(): setup_env() args = [sys.executable, '-m', 'pytest', diff --git a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py new file mode 100644 index 00000000000000..491eda3935e26d --- /dev/null +++ b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py @@ -0,0 +1,239 @@ +# Copyright (C) 2018-2021 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +import unittest + +from mo.moc_frontend.extractor import decode_name_with_port +from mo.utils.error import Error + +import pytest + + +mock_available = True +try: + # pylint: disable=no-name-in-module,import-error + from mock_mo_python_api import get_model_statistic, get_place_statistic, \ + clear_frontend_statistic, clear_model_statistic, clear_place_statistic + + # pylint: disable=no-name-in-module,import-error + from ngraph.frontend import FrontEndManager + +except Exception: + print("No mock frontend API available," + "ensure to use -DENABLE_TESTS=ON option when running these tests") + mock_available = False + +# FrontEndManager shall be initialized and destroyed after all tests finished +# This is because destroy of FrontEndManager will unload all plugins, +# no objects shall exist after this +if mock_available: + fem = FrontEndManager() + +mock_needed = pytest.mark.skipif(not mock_available, + reason="mock MO fe is not available") + + +class TestMainFrontend(unittest.TestCase): + def setUp(self): + clear_frontend_statistic() + clear_model_statistic() + clear_place_statistic() + self.fe = fem.load_by_framework('mock_mo_ngraph_frontend') + self.model = self.fe.load_from_file('abc.bin') + + # Mock model has 'tensor' tensor place + @mock_needed + def test_decode_name_with_port_tensor(self): + node = decode_name_with_port(self.model, "tensor") + model_stat = get_model_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 1 + assert node + + # Mock model has 'operation' operation place + @mock_needed + def test_decode_name_with_port_op(self): + node = decode_name_with_port(self.model, "operation") + model_stat = get_model_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 1 + assert node + + # pylint: disable=wrong-spelling-in-comment + # Mock model doesn't have 'mocknoname' place + @mock_needed + def test_decode_name_with_port_noname(self): + with self.assertRaisesRegex(Error, 'No\\ node\\ with\\ name.*mocknoname*'): + decode_name_with_port(self.model, 'mocknoname') + model_stat = get_model_statistic() + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 1 + + # Mock model has both tensor and operation with same name and non-equal data + # Collision is expected + @mock_needed + def test_decode_name_with_port_collision_op_tensor(self): + with self.assertRaisesRegex(Error, 'Name\\ collision.*tensorAndOp*'): + decode_name_with_port(self.model, 'tensorAndOp') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 1 + assert place_stat.is_equal_data > 0 + + # Mock model has 'operation' and output port up to 10 + @mock_needed + def test_decode_name_with_port_delim_op_out(self): + node = decode_name_with_port(self.model, 'operation:7') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_output_port == 1 + assert place_stat.lastArgInt == 7 + assert node + + # Mock model has 'operation' and input port up to 10 + @mock_needed + def test_decode_name_with_port_delim_op_in(self): + node = decode_name_with_port(self.model, '7:operation') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_input_port == 1 + assert place_stat.lastArgInt == 7 + assert node + + # Mock model has 'operation' and 'operation:0' op places, collision is expected + @mock_needed + def test_decode_name_with_port_delim_op_collision_out(self): + with self.assertRaisesRegex(Error, 'Name\\ collision.*operation\\:0*'): + decode_name_with_port(self.model, 'operation:0') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.is_equal_data > 0 + assert place_stat.get_output_port == 1 + assert place_stat.lastArgInt == 0 + + # Mock model has 'operation' and '0:operation' op places, collision is expected + @mock_needed + def test_decode_name_with_port_delim_op_collision_in(self): + with self.assertRaisesRegex(Error, 'Name\\ collision.*0\\:operation*'): + decode_name_with_port(self.model, '0:operation') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.is_equal_data > 0 + assert place_stat.get_input_port == 1 + assert place_stat.lastArgInt == 0 + + # Mock model has 'tensor' and 'tensor:0' tensor places, no collision is expected + @mock_needed + def test_decode_name_with_port_delim_tensor_no_collision_out(self): + node = decode_name_with_port(self.model, 'tensor:0') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_output_port == 0 + assert node + + # Mock model has 'tensor' and '0:tensor' tensor places, no collision is expected + @mock_needed + def test_decode_name_with_port_delim_tensor_no_collision_in(self): + node = decode_name_with_port(self.model, '0:tensor') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_input_port == 0 + assert node + + # Mock model doesn't have such '1234:operation' or output port=1234 for 'operation' + @mock_needed + def test_decode_name_with_port_delim_no_port_out(self): + with self.assertRaisesRegex(Error, 'No\\ node\\ with\\ name.*operation\\:1234*'): + decode_name_with_port(self.model, 'operation:1234') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_output_port == 1 + assert place_stat.lastArgInt == 1234 + + # Mock model doesn't have such '1234:operation' or input port=1234 for 'operation' + @mock_needed + def test_decode_name_with_port_delim_no_port_in(self): + with self.assertRaisesRegex(Error, 'No\\ node\\ with\\ name.*1234\\:operation*'): + decode_name_with_port(self.model, '1234:operation') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_input_port == 1 + assert place_stat.lastArgInt == 1234 + + # Mock model has tensor with name 'conv2d:0' and operation 'conv2d' with output port = 1 + # It is setup to return 'is_equal_data=True' for these tensor and port + # So no collision is expected + @mock_needed + def test_decode_name_with_port_delim_equal_data_out(self): + node = decode_name_with_port(self.model, 'conv2d:0') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_output_port == 1 + assert place_stat.is_equal_data > 0 + assert node + + # Mock model has tensor with name '0:conv2d' and operation 'conv2d' with input port = 1 + # It is setup to return 'is_equal_data=True' for these tensor and port + # So no collision is expected + @mock_needed + def test_decode_name_with_port_delim_equal_data_in(self): + node = decode_name_with_port(self.model, '0:conv2d') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 2 + assert place_stat.get_input_port == 1 + assert place_stat.is_equal_data > 0 + assert node + + # Stress case: Mock model has: + # Tensor '8:9' + # Operation '8:9' + # Operation '8' with output port = 9 + # Operation '9' with input port = 8 + # All places point to same data - no collision is expected + @mock_needed + def test_decode_name_with_port_delim_all_same_data(self): + node = decode_name_with_port(self.model, '8:9') + model_stat = get_model_statistic() + place_stat = get_place_statistic() + + assert model_stat.get_place_by_tensor_name == 1 + assert model_stat.get_place_by_operation_name == 3 + assert place_stat.get_input_port == 1 + assert place_stat.get_output_port == 1 + # At least 3 comparisons of places are expected + assert place_stat.is_equal_data > 2 + assert node diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp index d22980794c3a23..8327cc178c2c60 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp @@ -33,6 +33,7 @@ struct MOCK_API PlaceStat int m_is_input = 0; int m_is_output = 0; int m_is_equal = 0; + int m_is_equal_data = 0; // Arguments tracking std::string m_lastArgString; @@ -46,6 +47,7 @@ struct MOCK_API PlaceStat int is_input() const { return m_is_input; } int is_output() const { return m_is_output; } int is_equal() const { return m_is_equal; } + int is_equal_data() const { return m_is_equal_data; } // Arguments getters std::string get_lastArgString() const { return m_lastArgString; } @@ -60,10 +62,12 @@ class MOCK_API PlaceMockPy : public Place { static PlaceStat m_stat; std::string m_name; + bool m_is_op = false; + int m_portIndex = -1; public: - PlaceMockPy(const std::string& name = {}) - : m_name(name) + PlaceMockPy(const std::string& name = {}, bool is_op = false, int portIndex = -1) + : m_name(name), m_is_op(is_op), m_portIndex(portIndex) { } @@ -84,7 +88,10 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_input_port++; m_stat.m_lastArgInt = inputPortIndex; - return std::make_shared(); + if (inputPortIndex < 10) { + return std::make_shared(m_name, false, inputPortIndex); + } + return nullptr; } Place::Ptr get_input_port(const std::string& inputName) const override @@ -114,7 +121,10 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_output_port++; m_stat.m_lastArgInt = outputPortIndex; - return std::make_shared(); + if (outputPortIndex < 10) { + return std::make_shared(m_name, false, outputPortIndex); + } + return nullptr; } Place::Ptr get_output_port(const std::string& outputName) const override @@ -149,7 +159,24 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_is_equal++; m_stat.m_lastArgPlace = another; - return m_name == another->get_names().at(0); + std::shared_ptr mock = std::dynamic_pointer_cast(another); + return m_name == mock->m_name && m_is_op == mock->m_is_op && m_portIndex == mock->m_portIndex; + } + + bool is_equal_data(Ptr another) const override + { + m_stat.m_is_equal_data++; + m_stat.m_lastArgPlace = another; + std::shared_ptr mock = std::dynamic_pointer_cast(another); + if (mock->m_name.find("conv2d") != std::string::npos && + m_name.find("conv2d") != std::string::npos) { + return true; + } + if ((mock->m_name.find("8") != std::string::npos || mock->m_name.find("9") != std::string::npos) && + (m_name.find("8") != std::string::npos || m_name.find("9") != std::string::npos)) { + return true; + } + return mock->m_is_op == m_is_op; } //---------------Stat-------------------- @@ -167,6 +194,7 @@ struct MOCK_API ModelStat int m_get_inputs = 0; int m_get_outputs = 0; int m_get_place_by_tensor_name = 0; + int m_get_place_by_operation_name = 0; int m_set_partial_shape = 0; int m_get_partial_shape = 0; int m_set_element_type = 0; @@ -190,6 +218,7 @@ struct MOCK_API ModelStat int extract_subgraph() const { return m_extract_subgraph; } int override_all_inputs() const { return m_override_all_inputs; } int override_all_outputs() const { return m_override_all_outputs; } + int get_place_by_operation_name() const { return m_get_place_by_operation_name; } int get_place_by_tensor_name() const { return m_get_place_by_tensor_name; } int set_partial_shape() const { return m_set_partial_shape; } int get_partial_shape() const { return m_get_partial_shape; } @@ -208,12 +237,19 @@ struct MOCK_API ModelStat /// \brief Mock implementation of InputModel /// Every call increments appropriate counters in statistic and stores argument values to statistics /// as well -/// ("mock_output1", "mock_output2") class MOCK_API InputModelMockPy : public InputModel { static ModelStat m_stat; static PartialShape m_returnShape; + std::set m_operations = { + "8", "9", "8:9", "operation", "operation:0", "0:operation", "tensorAndOp", "conv2d" + }; + std::set m_tensors = { + "8:9", "tensor", "tensor:0", "0:tensor", "tensorAndOp", "conv2d:0", "0:conv2d", + "mock_input1", "mock_input2", "newInput1", "newIn1", "newIn2", + "mock_output1", "mock_output2", "new_output2", "newOut1", "newOut2" + }; public: std::vector get_inputs() const override { @@ -229,11 +265,24 @@ class MOCK_API InputModelMockPy : public InputModel std::make_shared("mock_output2")}; } + Place::Ptr get_place_by_operation_name(const std::string& opName) const override + { + m_stat.m_get_place_by_operation_name++; + m_stat.m_lastArgString = opName; + if (m_operations.count(opName)) { + return std::make_shared(opName, true); + } + return nullptr; + } + Place::Ptr get_place_by_tensor_name(const std::string& tensorName) const override { m_stat.m_get_place_by_tensor_name++; m_stat.m_lastArgString = tensorName; - return std::make_shared(tensorName); + if (m_tensors.count(tensorName)) { + return std::make_shared(tensorName); + } + return nullptr; } void override_all_outputs(const std::vector& outputs) override diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp index d9bbe52ab69b5f..2c515e92c32e6d 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp @@ -31,6 +31,7 @@ static void register_mock_model_stat(py::module m) py::class_ mdlStat(m, "ModelStat", py::dynamic_attr()); mdlStat.def_property_readonly("get_inputs", &ModelStat::get_inputs); mdlStat.def_property_readonly("get_outputs", &ModelStat::get_outputs); + mdlStat.def_property_readonly("get_place_by_operation_name", &ModelStat::get_place_by_operation_name); mdlStat.def_property_readonly("get_place_by_tensor_name", &ModelStat::get_place_by_tensor_name); mdlStat.def_property_readonly("set_partial_shape", &ModelStat::set_partial_shape); @@ -67,6 +68,7 @@ static void register_mock_place_stat(py::module m) placeStat.def_property_readonly("is_input", &PlaceStat::is_input); placeStat.def_property_readonly("is_output", &PlaceStat::is_output); placeStat.def_property_readonly("is_equal", &PlaceStat::is_equal); + placeStat.def_property_readonly("is_equal_data", &PlaceStat::is_equal_data); } PYBIND11_MODULE(mock_mo_python_api, m) diff --git a/ngraph/frontend/frontend_manager/include/frontend_manager/input_model.hpp b/ngraph/frontend/frontend_manager/include/frontend_manager/input_model.hpp index 6761e1dda383f2..7b42135f0da410 100644 --- a/ngraph/frontend/frontend_manager/include/frontend_manager/input_model.hpp +++ b/ngraph/frontend/frontend_manager/include/frontend_manager/input_model.hpp @@ -69,26 +69,29 @@ namespace ngraph /// \brief Returns a tensor place by a tensor name following framework conventions, or /// nullptr if a tensor with this name doesn't exist. /// \param tensorName Name of tensor - /// \return Tensor place corresponding to specifed tensor name + /// \return Tensor place corresponding to specified tensor name or nullptr if not exists virtual Place::Ptr get_place_by_tensor_name(const std::string& tensorName) const; /// \brief Returns an operation place by an operation name following framework - /// conventions, or nullptr if an operation with this name doesn't exist. \param - /// operationName Name of operation \return Place representing operation - virtual Place::Ptr get_place_by_operation_name(const std::string& operationName); + /// conventions, or nullptr if an operation with this name doesn't exist. + /// + /// \param operationName Name of operation + /// + /// \return Place representing operation or nullptr if not exists + virtual Place::Ptr get_place_by_operation_name(const std::string& operationName) const; /// \brief Returns an input port place by operation name and appropriate port index /// \param operationName Name of operation /// \param outputPortIndex Index of input port for this operation - /// \return Place representing input port of operation + /// \return Place representing input port of operation or nullptr if not exists virtual Place::Ptr get_place_by_operation_name_and_input_port(const std::string& operationName, int inputPortIndex); /// \brief Returns an output port place by operation name and appropriate port index - /// \param operationNameNname of operation + /// \param operationName Name of operation /// \param outputPortIndex Index of output port for this operation - /// \return Place representing output port of operation + /// \return Place representing output port of operation or nullptr if not exists virtual Place::Ptr get_place_by_operation_name_and_output_port(const std::string& operationName, int outputPortIndex); diff --git a/ngraph/frontend/frontend_manager/include/frontend_manager/place.hpp b/ngraph/frontend/frontend_manager/include/frontend_manager/place.hpp index 5df561fa0d5e4d..80d8bba04e6bb3 100644 --- a/ngraph/frontend/frontend_manager/include/frontend_manager/place.hpp +++ b/ngraph/frontend/frontend_manager/include/frontend_manager/place.hpp @@ -143,14 +143,14 @@ namespace ngraph /// \brief For operation node returns reference to an input port; applicable if /// operation node has only one input port /// - /// \return Input port place + /// \return Input port place or nullptr if not exists virtual Ptr get_input_port() const; /// \brief For operation node returns reference to an input port with specified index /// /// \param inputPortIndex Input port index /// - /// \return Appropriate input port place + /// \return Appropriate input port place or nullptr if not exists virtual Ptr get_input_port(int inputPortIndex) const; /// \brief For operation node returns reference to an input port with specified name; @@ -158,7 +158,7 @@ namespace ngraph /// /// \param inputName Name of port group /// - /// \return Appropriate input port place + /// \return Appropriate input port place or nullptr if not exists virtual Ptr get_input_port(const std::string& inputName) const; /// \brief For operation node returns reference to an input port with specified name and @@ -168,20 +168,20 @@ namespace ngraph /// /// \param inputPortIndex Input port index in a group /// - /// \return Appropriate input port place + /// \return Appropriate input port place or nullptr if not exists virtual Ptr get_input_port(const std::string& inputName, int inputPortIndex) const; /// \brief For operation node returns reference to an output port; applicable for /// operations with only one output port /// - /// \return Appropriate output port place + /// \return Appropriate output port place or nullptr if not exists virtual Ptr get_output_port() const; /// \brief For operation node returns reference to an output port with specified index /// /// \param outputPortIndex Output port index /// - /// \return Appropriate output port place + /// \return Appropriate output port place or nullptr if not exists virtual Ptr get_output_port(int outputPortIndex) const; /// \brief For operation node returns reference to an output port with specified name; @@ -189,7 +189,7 @@ namespace ngraph /// /// \param outputName Name of output port group /// - /// \return Appropriate output port place + /// \return Appropriate output port place or nullptr if not exists virtual Ptr get_output_port(const std::string& outputName) const; /// \brief For operation node returns reference to an output port with specified name @@ -199,7 +199,7 @@ namespace ngraph /// /// \param outputPortIndex Output port index /// - /// \return Appropriate output port place + /// \return Appropriate output port place or nullptr if not exists virtual Ptr get_output_port(const std::string& outputName, int outputPortIndex) const; /// \brief Returns all input ports that consume data flows through this place diff --git a/ngraph/frontend/frontend_manager/src/frontend_manager.cpp b/ngraph/frontend/frontend_manager/src/frontend_manager.cpp index 037a2522523dc4..97919a29b5b27e 100644 --- a/ngraph/frontend/frontend_manager/src/frontend_manager.cpp +++ b/ngraph/frontend/frontend_manager/src/frontend_manager.cpp @@ -191,24 +191,24 @@ std::vector InputModel::get_outputs() const Place::Ptr InputModel::get_place_by_tensor_name(const std::string& tensorName) const { - FRONT_END_NOT_IMPLEMENTED(get_place_by_tensor_name); + return nullptr; } -Place::Ptr InputModel::get_place_by_operation_name(const std::string& operationName) +Place::Ptr InputModel::get_place_by_operation_name(const std::string& operationName) const { - FRONT_END_NOT_IMPLEMENTED(get_place_by_operation_name); + return nullptr; } Place::Ptr InputModel::get_place_by_operation_name_and_input_port(const std::string& operationName, int inputPortIndex) { - FRONT_END_NOT_IMPLEMENTED(get_place_by_operation_name_and_input_port); + return nullptr; } Place::Ptr InputModel::get_place_by_operation_name_and_output_port(const std::string& operationName, int outputPortIndex) { - FRONT_END_NOT_IMPLEMENTED(get_place_by_operation_name_and_output_port); + return nullptr; } void InputModel::set_name_for_tensor(Place::Ptr tensor, const std::string& newName) @@ -350,42 +350,42 @@ Place::Ptr Place::get_producing_port() const Place::Ptr Place::get_input_port() const { - FRONT_END_NOT_IMPLEMENTED(get_input_port); + return nullptr; } Place::Ptr Place::get_input_port(int inputPortIndex) const { - FRONT_END_NOT_IMPLEMENTED(get_input_port); + return nullptr; } Place::Ptr Place::get_input_port(const std::string& inputName) const { - FRONT_END_NOT_IMPLEMENTED(get_input_port); + return nullptr; } Place::Ptr Place::get_input_port(const std::string& inputName, int inputPortIndex) const { - FRONT_END_NOT_IMPLEMENTED(get_input_port); + return nullptr; } Place::Ptr Place::get_output_port() const { - FRONT_END_NOT_IMPLEMENTED(get_output_port); + return nullptr; } Place::Ptr Place::get_output_port(int outputPortIndex) const { - FRONT_END_NOT_IMPLEMENTED(get_output_port); + return nullptr; } Place::Ptr Place::get_output_port(const std::string& outputName) const { - FRONT_END_NOT_IMPLEMENTED(get_output_port); + return nullptr; } Place::Ptr Place::get_output_port(const std::string& outputName, int outputPortIndex) const { - FRONT_END_NOT_IMPLEMENTED(get_output_port); + return nullptr; } std::vector Place::get_consuming_ports() const diff --git a/ngraph/python/tests/mock/mock_py_ngraph_frontend/mock_py_frontend.hpp b/ngraph/python/tests/mock/mock_py_ngraph_frontend/mock_py_frontend.hpp index 651e9e53809683..521714430d1696 100644 --- a/ngraph/python/tests/mock/mock_py_ngraph_frontend/mock_py_frontend.hpp +++ b/ngraph/python/tests/mock/mock_py_ngraph_frontend/mock_py_frontend.hpp @@ -334,7 +334,7 @@ class MOCK_API InputModelMockPy : public InputModel return std::make_shared(); } - Place::Ptr get_place_by_operation_name(const std::string& operationName) override + Place::Ptr get_place_by_operation_name(const std::string& operationName) const override { m_stat.m_get_place_by_operation_name++; m_stat.m_lastArgString = operationName; diff --git a/ngraph/test/frontend/frontend_manager.cpp b/ngraph/test/frontend/frontend_manager.cpp index af70885d237901..508f334ddaec38 100644 --- a/ngraph/test/frontend/frontend_manager.cpp +++ b/ngraph/test/frontend/frontend_manager.cpp @@ -113,10 +113,10 @@ TEST(FrontEndManagerTest, testDefaultInputModel) ASSERT_ANY_THROW(im->override_all_inputs({nullptr})); ASSERT_ANY_THROW(im->override_all_outputs({nullptr})); ASSERT_ANY_THROW(im->extract_subgraph({nullptr}, {nullptr})); - ASSERT_ANY_THROW(im->get_place_by_tensor_name("")); - ASSERT_ANY_THROW(im->get_place_by_operation_name("")); - ASSERT_ANY_THROW(im->get_place_by_operation_name_and_input_port("", 0)); - ASSERT_ANY_THROW(im->get_place_by_operation_name_and_output_port("", 0)); + ASSERT_EQ(im->get_place_by_tensor_name(""), nullptr); + ASSERT_EQ(im->get_place_by_operation_name(""), nullptr); + ASSERT_EQ(im->get_place_by_operation_name_and_input_port("", 0), nullptr); + ASSERT_EQ(im->get_place_by_operation_name_and_output_port("", 0), nullptr); ASSERT_ANY_THROW(im->set_name_for_tensor(nullptr, "")); ASSERT_ANY_THROW(im->add_name_for_tensor(nullptr, "")); ASSERT_ANY_THROW(im->set_name_for_operation(nullptr, "")); @@ -148,14 +148,14 @@ TEST(FrontEndManagerTest, testDefaultPlace) ASSERT_ANY_THROW(place->get_producing_operation()); ASSERT_ANY_THROW(place->get_producing_operation(0)); ASSERT_ANY_THROW(place->get_producing_port()); - ASSERT_ANY_THROW(place->get_input_port()); - ASSERT_ANY_THROW(place->get_input_port(0)); - ASSERT_ANY_THROW(place->get_input_port("")); - ASSERT_ANY_THROW(place->get_input_port("", 0)); - ASSERT_ANY_THROW(place->get_output_port()); - ASSERT_ANY_THROW(place->get_output_port(0)); - ASSERT_ANY_THROW(place->get_output_port("")); - ASSERT_ANY_THROW(place->get_output_port("", 0)); + ASSERT_EQ(place->get_input_port(), nullptr); + ASSERT_EQ(place->get_input_port(0), nullptr); + ASSERT_EQ(place->get_input_port(""), nullptr); + ASSERT_EQ(place->get_input_port("", 0), nullptr); + ASSERT_EQ(place->get_output_port(), nullptr); + ASSERT_EQ(place->get_output_port(0), nullptr); + ASSERT_EQ(place->get_output_port(""), nullptr); + ASSERT_EQ(place->get_output_port("", 0), nullptr); ASSERT_ANY_THROW(place->get_consuming_ports()); ASSERT_ANY_THROW(place->is_input()); ASSERT_ANY_THROW(place->is_output()); From 3f1393fca166dd3c4be7a1109977c61e36269029 Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Tue, 6 Jul 2021 20:42:01 +0300 Subject: [PATCH 2/8] Corrected comments in code --- model-optimizer/mo/moc_frontend/extractor.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/model-optimizer/mo/moc_frontend/extractor.py b/model-optimizer/mo/moc_frontend/extractor.py index 4462ea804eb6db..1e4d44e8564382 100644 --- a/model-optimizer/mo/moc_frontend/extractor.py +++ b/model-optimizer/mo/moc_frontend/extractor.py @@ -14,7 +14,7 @@ def decode_name_with_port(input_model: InputModel, node_name: str): """ Decode name with optional port specification w/o traversing all the nodes in the graph - TODO: in future node_name can specify input/output port groups and indices (58562) + TODO: in future node_name can specify input/output port groups as well as indices (58562) :param input_model: Input Model :param node_name: user provided node name :return: decoded place in the graph @@ -22,12 +22,12 @@ def decode_name_with_port(input_model: InputModel, node_name: str): found_nodes = [] found_node_names = [] - """ - Helper function to add Place node to list of found nodes. Adds only if node is not None - :param found_nodes List[Place]: list of found nodes - :param node Place: Place node to add to list - """ def add_found_node(found_nodes: list, node: Place): + """ + Helper function to add Place node to list of found nodes. Adds only if node is not None + :param found_nodes List[Place]: list of found nodes + :param node Place: Place node to add to list + """ found_nodes.append(node) if node else None node = input_model.get_place_by_tensor_name(node_name) @@ -70,8 +70,7 @@ def add_found_node(found_nodes: list, node: Place): # This logic is not going to work with other frontends # TODO: Add support for input/output group name and port index here (58562) - # Legacy frontends use format "number:name:number" to specify input and output port indices - # For new frontends this logic shall be extended to additionally support input and output group names + # For new frontends logic shall be extended to additionally support input and output group names return found_nodes[0] From 8126c28008f1035441b9f67a3b74bea8712fc734 Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Tue, 6 Jul 2021 20:47:45 +0300 Subject: [PATCH 3/8] Missing empty line --- model-optimizer/mo/front/extractor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/model-optimizer/mo/front/extractor.py b/model-optimizer/mo/front/extractor.py index 44173dde07dcd5..139dc6503eb680 100644 --- a/model-optimizer/mo/front/extractor.py +++ b/model-optimizer/mo/front/extractor.py @@ -454,6 +454,7 @@ def extract_node_attrs(graph: Graph, extractor: callable): def raise_no_node(node_name: str): raise Error('No node with name {}'.format(node_name)) + def raise_node_name_collision(node_name: str, found_nodes: list): raise Error('Name collision was found, there are several nodes for mask "{}": {}. ' 'If your intention was to specify port for node, please instead specify node names connected to ' From 39fc549eeb6877c9231b1200c603a0a1688dad9e Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Tue, 6 Jul 2021 20:49:48 +0300 Subject: [PATCH 4/8] Clang format fixes --- .../mock_mo_frontend.hpp | 54 +++++++++++++------ .../mock_mo_python_api/mock_mo_python_api.cpp | 3 +- .../test/frontend/paddlepaddle/basic_api.cpp | 10 ++-- .../paddlepaddle/cut_specific_model.cpp | 6 +-- .../test/frontend/paddlepaddle/load_from.cpp | 6 +-- .../frontend/paddlepaddle/partial_shape.cpp | 20 +++---- .../paddlepaddle/set_element_type.cpp | 6 +-- 7 files changed, 64 insertions(+), 41 deletions(-) diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp index 8327cc178c2c60..4336a48815cc0b 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp @@ -67,7 +67,9 @@ class MOCK_API PlaceMockPy : public Place public: PlaceMockPy(const std::string& name = {}, bool is_op = false, int portIndex = -1) - : m_name(name), m_is_op(is_op), m_portIndex(portIndex) + : m_name(name) + , m_is_op(is_op) + , m_portIndex(portIndex) { } @@ -88,7 +90,8 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_input_port++; m_stat.m_lastArgInt = inputPortIndex; - if (inputPortIndex < 10) { + if (inputPortIndex < 10) + { return std::make_shared(m_name, false, inputPortIndex); } return nullptr; @@ -121,7 +124,8 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_output_port++; m_stat.m_lastArgInt = outputPortIndex; - if (outputPortIndex < 10) { + if (outputPortIndex < 10) + { return std::make_shared(m_name, false, outputPortIndex); } return nullptr; @@ -160,7 +164,8 @@ class MOCK_API PlaceMockPy : public Place m_stat.m_is_equal++; m_stat.m_lastArgPlace = another; std::shared_ptr mock = std::dynamic_pointer_cast(another); - return m_name == mock->m_name && m_is_op == mock->m_is_op && m_portIndex == mock->m_portIndex; + return m_name == mock->m_name && m_is_op == mock->m_is_op && + m_portIndex == mock->m_portIndex; } bool is_equal_data(Ptr another) const override @@ -169,11 +174,14 @@ class MOCK_API PlaceMockPy : public Place m_stat.m_lastArgPlace = another; std::shared_ptr mock = std::dynamic_pointer_cast(another); if (mock->m_name.find("conv2d") != std::string::npos && - m_name.find("conv2d") != std::string::npos) { + m_name.find("conv2d") != std::string::npos) + { return true; } - if ((mock->m_name.find("8") != std::string::npos || mock->m_name.find("9") != std::string::npos) && - (m_name.find("8") != std::string::npos || m_name.find("9") != std::string::npos)) { + if ((mock->m_name.find("8") != std::string::npos || + mock->m_name.find("9") != std::string::npos) && + (m_name.find("8") != std::string::npos || m_name.find("9") != std::string::npos)) + { return true; } return mock->m_is_op == m_is_op; @@ -243,13 +251,25 @@ class MOCK_API InputModelMockPy : public InputModel static PartialShape m_returnShape; std::set m_operations = { - "8", "9", "8:9", "operation", "operation:0", "0:operation", "tensorAndOp", "conv2d" - }; - std::set m_tensors = { - "8:9", "tensor", "tensor:0", "0:tensor", "tensorAndOp", "conv2d:0", "0:conv2d", - "mock_input1", "mock_input2", "newInput1", "newIn1", "newIn2", - "mock_output1", "mock_output2", "new_output2", "newOut1", "newOut2" - }; + "8", "9", "8:9", "operation", "operation:0", "0:operation", "tensorAndOp", "conv2d"}; + std::set m_tensors = {"8:9", + "tensor", + "tensor:0", + "0:tensor", + "tensorAndOp", + "conv2d:0", + "0:conv2d", + "mock_input1", + "mock_input2", + "newInput1", + "newIn1", + "newIn2", + "mock_output1", + "mock_output2", + "new_output2", + "newOut1", + "newOut2"}; + public: std::vector get_inputs() const override { @@ -269,7 +289,8 @@ class MOCK_API InputModelMockPy : public InputModel { m_stat.m_get_place_by_operation_name++; m_stat.m_lastArgString = opName; - if (m_operations.count(opName)) { + if (m_operations.count(opName)) + { return std::make_shared(opName, true); } return nullptr; @@ -279,7 +300,8 @@ class MOCK_API InputModelMockPy : public InputModel { m_stat.m_get_place_by_tensor_name++; m_stat.m_lastArgString = tensorName; - if (m_tensors.count(tensorName)) { + if (m_tensors.count(tensorName)) + { return std::make_shared(tensorName); } return nullptr; diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp index 2c515e92c32e6d..5660f4fa58aaa6 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp @@ -31,7 +31,8 @@ static void register_mock_model_stat(py::module m) py::class_ mdlStat(m, "ModelStat", py::dynamic_attr()); mdlStat.def_property_readonly("get_inputs", &ModelStat::get_inputs); mdlStat.def_property_readonly("get_outputs", &ModelStat::get_outputs); - mdlStat.def_property_readonly("get_place_by_operation_name", &ModelStat::get_place_by_operation_name); + mdlStat.def_property_readonly("get_place_by_operation_name", + &ModelStat::get_place_by_operation_name); mdlStat.def_property_readonly("get_place_by_tensor_name", &ModelStat::get_place_by_tensor_name); mdlStat.def_property_readonly("set_partial_shape", &ModelStat::set_partial_shape); diff --git a/ngraph/test/frontend/paddlepaddle/basic_api.cpp b/ngraph/test/frontend/paddlepaddle/basic_api.cpp index 633e8edbcf4ef4..d191e4fccda3af 100644 --- a/ngraph/test/frontend/paddlepaddle/basic_api.cpp +++ b/ngraph/test/frontend/paddlepaddle/basic_api.cpp @@ -21,8 +21,8 @@ static const std::vector models{ }; INSTANTIATE_TEST_SUITE_P(PDPDBasicTest, - FrontEndBasicTest, - ::testing::Combine(::testing::Values(PDPD), - ::testing::Values(std::string(TEST_PDPD_MODELS)), - ::testing::ValuesIn(models)), - FrontEndBasicTest::getTestCaseName); + FrontEndBasicTest, + ::testing::Combine(::testing::Values(PDPD), + ::testing::Values(std::string(TEST_PDPD_MODELS)), + ::testing::ValuesIn(models)), + FrontEndBasicTest::getTestCaseName); diff --git a/ngraph/test/frontend/paddlepaddle/cut_specific_model.cpp b/ngraph/test/frontend/paddlepaddle/cut_specific_model.cpp index c4f00198b26d0d..3251762b6f9421 100644 --- a/ngraph/test/frontend/paddlepaddle/cut_specific_model.cpp +++ b/ngraph/test/frontend/paddlepaddle/cut_specific_model.cpp @@ -28,6 +28,6 @@ static CutModelParam getTestData_2in_2out() } INSTANTIATE_TEST_SUITE_P(PDPDCutTest, - FrontEndCutModelTest, - ::testing::Values(getTestData_2in_2out()), - FrontEndCutModelTest::getTestCaseName); \ No newline at end of file + FrontEndCutModelTest, + ::testing::Values(getTestData_2in_2out()), + FrontEndCutModelTest::getTestCaseName); \ No newline at end of file diff --git a/ngraph/test/frontend/paddlepaddle/load_from.cpp b/ngraph/test/frontend/paddlepaddle/load_from.cpp index 0f3256fc2bcac6..2950c3d271f4f7 100644 --- a/ngraph/test/frontend/paddlepaddle/load_from.cpp +++ b/ngraph/test/frontend/paddlepaddle/load_from.cpp @@ -24,6 +24,6 @@ static LoadFromFEParam getTestData() } INSTANTIATE_TEST_SUITE_P(PDPDCutTest, - FrontEndLoadFromTest, - ::testing::Values(getTestData()), - FrontEndLoadFromTest::getTestCaseName); \ No newline at end of file + FrontEndLoadFromTest, + ::testing::Values(getTestData()), + FrontEndLoadFromTest::getTestCaseName); \ No newline at end of file diff --git a/ngraph/test/frontend/paddlepaddle/partial_shape.cpp b/ngraph/test/frontend/paddlepaddle/partial_shape.cpp index 0cef8886760e2f..ddb7213f9ec75f 100644 --- a/ngraph/test/frontend/paddlepaddle/partial_shape.cpp +++ b/ngraph/test/frontend/paddlepaddle/partial_shape.cpp @@ -62,13 +62,13 @@ static PartShape getTestShape_conv2d_relu() } INSTANTIATE_TEST_SUITE_P(PDPDPartialShapeTest, - FrontEndPartialShapeTest, - ::testing::Combine(::testing::Values(BaseFEParam{ - PDPD, std::string(TEST_PDPD_MODELS)}), - ::testing::ValuesIn(std::vector{ - getTestShape_2in_2out(), - getTestShape_conv2d_relu(), - getTestShape_conv2d(), - getTestShape_conv2d_setDynamicBatch(), - getTestShape_2in_2out_dynbatch()})), - FrontEndPartialShapeTest::getTestCaseName); \ No newline at end of file + FrontEndPartialShapeTest, + ::testing::Combine(::testing::Values(BaseFEParam{ + PDPD, std::string(TEST_PDPD_MODELS)}), + ::testing::ValuesIn(std::vector{ + getTestShape_2in_2out(), + getTestShape_conv2d_relu(), + getTestShape_conv2d(), + getTestShape_conv2d_setDynamicBatch(), + getTestShape_2in_2out_dynbatch()})), + FrontEndPartialShapeTest::getTestCaseName); \ No newline at end of file diff --git a/ngraph/test/frontend/paddlepaddle/set_element_type.cpp b/ngraph/test/frontend/paddlepaddle/set_element_type.cpp index c14ce0c8b6f9c3..e53ea790ac869f 100644 --- a/ngraph/test/frontend/paddlepaddle/set_element_type.cpp +++ b/ngraph/test/frontend/paddlepaddle/set_element_type.cpp @@ -21,6 +21,6 @@ static SetTypeFEParam getTestData_relu() } INSTANTIATE_TEST_SUITE_P(PDPDCutTest, - FrontEndElementTypeTest, - ::testing::Values(getTestData_relu()), - FrontEndElementTypeTest::getTestCaseName); \ No newline at end of file + FrontEndElementTypeTest, + ::testing::Values(getTestData_relu()), + FrontEndElementTypeTest::getTestCaseName); \ No newline at end of file From 5f6d234a85ad7ba65bc8ba95724fd7ce4b548166 Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Mon, 19 Jul 2021 16:17:57 +0300 Subject: [PATCH 5/8] Fix review comments --- model-optimizer/mo/moc_frontend/extractor.py | 37 ++++++++------------ 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/model-optimizer/mo/moc_frontend/extractor.py b/model-optimizer/mo/moc_frontend/extractor.py index 1e4d44e8564382..901ae73c4bed68 100644 --- a/model-optimizer/mo/moc_frontend/extractor.py +++ b/model-optimizer/mo/moc_frontend/extractor.py @@ -22,46 +22,39 @@ def decode_name_with_port(input_model: InputModel, node_name: str): found_nodes = [] found_node_names = [] - def add_found_node(found_nodes: list, node: Place): - """ - Helper function to add Place node to list of found nodes. Adds only if node is not None - :param found_nodes List[Place]: list of found nodes - :param node Place: Place node to add to list - """ - found_nodes.append(node) if node else None - node = input_model.get_place_by_tensor_name(node_name) - found_node_names.append('Tensor:' + node_name) - add_found_node(found_nodes, node) + found_node_names.append('Tensor:' + node_name) if node else None + found_nodes.append(node) if node else None node = input_model.get_place_by_operation_name(node_name) - add_found_node(found_nodes, node) - found_node_names.append('Operation:' + node_name) + found_node_names.append('Operation:' + node_name) if node else None + found_nodes.append(node) if node else None regexp_post = r'(.+):(\d+)' match_post = re.search(regexp_post, node_name) if match_post: - found_node_names.append(match_post.group(1)) node_post = input_model.get_place_by_operation_name(match_post.group(1)) - add_found_node(found_nodes, node_post.get_output_port( - outputPortIndex=int(match_post.group(2)))) if node_post else None + node_post = node_post.get_output_port( + outputPortIndex=int(match_post.group(2))) if node_post else None + found_node_names.append(match_post.group(1)) if node_post else None + found_nodes.append(node_post) if node_post else None regexp_pre = r'(\d+):(.+)' match_pre = re.search(regexp_pre, node_name) if match_pre: - found_node_names.append(match_pre.group(2)) node_pre = input_model.get_place_by_operation_name(match_pre.group(2)) - add_found_node(found_nodes, node_pre.get_input_port( - inputPortIndex=int(match_pre.group(1)))) if node_pre else None + node_pre = node_pre.get_input_port( + inputPortIndex=int(match_pre.group(1))) if node_pre else None + found_node_names.append(match_pre.group(2)) if node_pre else None + found_nodes.append(node_pre) if node_pre else None if len(found_nodes) == 0: raise_no_node(node_name) # Check that there is no collision, all found places shall point to same data - if len(found_nodes) > 1: - all_equal = all([n.is_equal_data(found_nodes[0]) for n in found_nodes]) - if not all_equal: - raise_node_name_collision(node_name, found_node_names) + all_equal = all([n.is_equal_data(found_nodes[0]) for n in found_nodes]) + if not all_equal: + raise_node_name_collision(node_name, found_node_names) # TODO: ONNX specific (59408) # To comply with legacy behavior, for ONNX-only there shall be considered additional 2 possibilities From 3f58ff7f524c23c5f0a759da14480cefdae5a25a Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Mon, 19 Jul 2021 17:45:55 +0300 Subject: [PATCH 6/8] Updated test to verify review comments fixes --- .../unit_tests/mo/moc_frontend/moc_extractor_test_actual.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py index 491eda3935e26d..93ee6f048dc01b 100644 --- a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py +++ b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py @@ -113,7 +113,7 @@ def test_decode_name_with_port_delim_op_in(self): # Mock model has 'operation' and 'operation:0' op places, collision is expected @mock_needed def test_decode_name_with_port_delim_op_collision_out(self): - with self.assertRaisesRegex(Error, 'Name\\ collision.*operation\\:0*'): + with self.assertRaisesRegex(Error, 'Name\\ collision(?!.*Tensor.*).*operation\\:0*'): decode_name_with_port(self.model, 'operation:0') model_stat = get_model_statistic() place_stat = get_place_statistic() @@ -127,7 +127,7 @@ def test_decode_name_with_port_delim_op_collision_out(self): # Mock model has 'operation' and '0:operation' op places, collision is expected @mock_needed def test_decode_name_with_port_delim_op_collision_in(self): - with self.assertRaisesRegex(Error, 'Name\\ collision.*0\\:operation*'): + with self.assertRaisesRegex(Error, 'Name\\ collision(?!.*Tensor.*).*0\\:operation*'): decode_name_with_port(self.model, '0:operation') model_stat = get_model_statistic() place_stat = get_place_statistic() From 9488d343c2ed7c2947bc401c3e087e8d8a06da7b Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Mon, 26 Jul 2021 16:56:37 +0300 Subject: [PATCH 7/8] Update unit tests after rebase --- model-optimizer/unit_tests/mo/frontend_ngraph_test.py | 2 +- .../unit_tests/mo/moc_frontend/moc_extractor_test_actual.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/model-optimizer/unit_tests/mo/frontend_ngraph_test.py b/model-optimizer/unit_tests/mo/frontend_ngraph_test.py index ab165f7c25695a..f440284b8f926a 100644 --- a/model-optimizer/unit_tests/mo/frontend_ngraph_test.py +++ b/model-optimizer/unit_tests/mo/frontend_ngraph_test.py @@ -37,7 +37,7 @@ def test_frontends(): def test_moc_extractor(): setup_env() args = [sys.executable, '-m', 'pytest', - './moc_frontend/moc_extractor_test_actual.py', '-s'] + os.path.join(os.path.dirname(__file__), 'moc_frontend/moc_extractor_test_actual.py'), '-s'] status = subprocess.run(args, env=os.environ) assert not status.returncode diff --git a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py index 93ee6f048dc01b..a7ee8605dae3c8 100644 --- a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py +++ b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py @@ -39,7 +39,7 @@ def setUp(self): clear_model_statistic() clear_place_statistic() self.fe = fem.load_by_framework('mock_mo_ngraph_frontend') - self.model = self.fe.load_from_file('abc.bin') + self.model = self.fe.load('abc.bin') # Mock model has 'tensor' tensor place @mock_needed From e809cd6e506002ba0fa80174ec02dbc89e8f30af Mon Sep 17 00:00:00 2001 From: Michael Nosov Date: Wed, 28 Jul 2021 21:44:18 +0300 Subject: [PATCH 8/8] Apply review comments --- model-optimizer/mo/moc_frontend/extractor.py | 35 +++++++------ .../moc_frontend/moc_extractor_test_actual.py | 9 +++- .../mock_mo_frontend.cpp | 5 ++ .../mock_mo_frontend.hpp | 52 ++++++++++++++----- .../mock_mo_python_api/mock_mo_python_api.cpp | 8 +++ 5 files changed, 81 insertions(+), 28 deletions(-) diff --git a/model-optimizer/mo/moc_frontend/extractor.py b/model-optimizer/mo/moc_frontend/extractor.py index 901ae73c4bed68..b5ddd83b1fdef9 100644 --- a/model-optimizer/mo/moc_frontend/extractor.py +++ b/model-optimizer/mo/moc_frontend/extractor.py @@ -6,7 +6,7 @@ from mo.front.extractor import raise_no_node, raise_node_name_collision from mo.utils.error import Error -from ngraph.frontend import InputModel, Place # pylint: disable=no-name-in-module,import-error +from ngraph.frontend import InputModel # pylint: disable=no-name-in-module,import-error import numpy as np @@ -23,37 +23,42 @@ def decode_name_with_port(input_model: InputModel, node_name: str): found_node_names = [] node = input_model.get_place_by_tensor_name(node_name) - found_node_names.append('Tensor:' + node_name) if node else None - found_nodes.append(node) if node else None + if node: + found_node_names.append('Tensor:' + node_name) + found_nodes.append(node) node = input_model.get_place_by_operation_name(node_name) - found_node_names.append('Operation:' + node_name) if node else None - found_nodes.append(node) if node else None + if node: + found_node_names.append('Operation:' + node_name) + found_nodes.append(node) regexp_post = r'(.+):(\d+)' match_post = re.search(regexp_post, node_name) if match_post: node_post = input_model.get_place_by_operation_name(match_post.group(1)) - node_post = node_post.get_output_port( - outputPortIndex=int(match_post.group(2))) if node_post else None - found_node_names.append(match_post.group(1)) if node_post else None - found_nodes.append(node_post) if node_post else None + if node_post: + node_post = node_post.get_output_port( + outputPortIndex=int(match_post.group(2))) + if node_post: + found_node_names.append(match_post.group(1)) + found_nodes.append(node_post) regexp_pre = r'(\d+):(.+)' match_pre = re.search(regexp_pre, node_name) if match_pre: node_pre = input_model.get_place_by_operation_name(match_pre.group(2)) - node_pre = node_pre.get_input_port( - inputPortIndex=int(match_pre.group(1))) if node_pre else None - found_node_names.append(match_pre.group(2)) if node_pre else None - found_nodes.append(node_pre) if node_pre else None + if node_pre: + node_pre = node_pre.get_input_port( + inputPortIndex=int(match_pre.group(1))) + if node_pre: + found_node_names.append(match_pre.group(2)) + found_nodes.append(node_pre) if len(found_nodes) == 0: raise_no_node(node_name) # Check that there is no collision, all found places shall point to same data - all_equal = all([n.is_equal_data(found_nodes[0]) for n in found_nodes]) - if not all_equal: + if not all([n.is_equal_data(found_nodes[0]) for n in found_nodes]): raise_node_name_collision(node_name, found_node_names) # TODO: ONNX specific (59408) diff --git a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py index a7ee8605dae3c8..17ac755c4d17cd 100644 --- a/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py +++ b/model-optimizer/unit_tests/mo/moc_frontend/moc_extractor_test_actual.py @@ -10,10 +10,12 @@ mock_available = True + try: # pylint: disable=no-name-in-module,import-error from mock_mo_python_api import get_model_statistic, get_place_statistic, \ - clear_frontend_statistic, clear_model_statistic, clear_place_statistic + clear_frontend_statistic, clear_model_statistic, clear_place_statistic, \ + clear_setup, set_equal_data, set_max_port_counts # pylint: disable=no-name-in-module,import-error from ngraph.frontend import FrontEndManager @@ -38,6 +40,8 @@ def setUp(self): clear_frontend_statistic() clear_model_statistic() clear_place_statistic() + clear_setup() + set_max_port_counts(10, 10) self.fe = fem.load_by_framework('mock_mo_ngraph_frontend') self.model = self.fe.load('abc.bin') @@ -193,6 +197,7 @@ def test_decode_name_with_port_delim_no_port_in(self): # So no collision is expected @mock_needed def test_decode_name_with_port_delim_equal_data_out(self): + set_equal_data('conv2d', 'conv2d') node = decode_name_with_port(self.model, 'conv2d:0') model_stat = get_model_statistic() place_stat = get_place_statistic() @@ -208,6 +213,7 @@ def test_decode_name_with_port_delim_equal_data_out(self): # So no collision is expected @mock_needed def test_decode_name_with_port_delim_equal_data_in(self): + set_equal_data('conv2d', 'conv2d') node = decode_name_with_port(self.model, '0:conv2d') model_stat = get_model_statistic() place_stat = get_place_statistic() @@ -226,6 +232,7 @@ def test_decode_name_with_port_delim_equal_data_in(self): # All places point to same data - no collision is expected @mock_needed def test_decode_name_with_port_delim_all_same_data(self): + set_equal_data('8', '9') node = decode_name_with_port(self.model, '8:9') model_stat = get_model_statistic() place_stat = get_place_statistic() diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.cpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.cpp index 9ddbba38040770..17c647e35b5801 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.cpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.cpp @@ -14,6 +14,11 @@ FeStat FrontEndMockPy::m_stat = {}; ModelStat InputModelMockPy::m_stat = {}; PlaceStat PlaceMockPy::m_stat = {}; +std::string MockSetup::m_equal_data_node1 = {}; +std::string MockSetup::m_equal_data_node2 = {}; +int MockSetup::m_max_input_port_index = 0; +int MockSetup::m_max_output_port_index = 0; + PartialShape InputModelMockPy::m_returnShape = {}; extern "C" MOCK_API FrontEndVersion GetAPIVersion() diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp index 51927d10d3c9a7..eb8182132f95c1 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp @@ -21,6 +21,35 @@ using namespace ngraph; using namespace ngraph::frontend; //////////////////////////////// +/// \brief This structure holds number static setup values +/// It will be used by Python unit tests to setup particular mock behavior +struct MOCK_API MockSetup +{ + static std::string m_equal_data_node1; + static std::string m_equal_data_node2; + static int m_max_input_port_index; + static int m_max_output_port_index; + + static void clear_setup() + { + m_equal_data_node1 = {}; + m_equal_data_node2 = {}; + m_max_input_port_index = 0; + m_max_output_port_index = 0; + } + + static void set_equal_data(const std::string& node1, const std::string& node2) + { + m_equal_data_node1 = node1; + m_equal_data_node2 = node2; + } + + static void set_max_port_counts(int max_input, int max_output) + { + m_max_input_port_index = max_input; + m_max_output_port_index = max_output; + } +}; /// \brief This structure holds number of calls of particular methods of Place objects /// It will be used by Python unit tests to verify that appropriate API @@ -90,7 +119,7 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_input_port++; m_stat.m_lastArgInt = inputPortIndex; - if (inputPortIndex < 10) + if (inputPortIndex < MockSetup::m_max_input_port_index) { return std::make_shared(m_name, false, inputPortIndex); } @@ -124,7 +153,7 @@ class MOCK_API PlaceMockPy : public Place { m_stat.m_get_output_port++; m_stat.m_lastArgInt = outputPortIndex; - if (outputPortIndex < 10) + if (outputPortIndex < MockSetup::m_max_output_port_index) { return std::make_shared(m_name, false, outputPortIndex); } @@ -173,16 +202,15 @@ class MOCK_API PlaceMockPy : public Place m_stat.m_is_equal_data++; m_stat.m_lastArgPlace = another; std::shared_ptr mock = std::dynamic_pointer_cast(another); - if (mock->m_name.find("conv2d") != std::string::npos && - m_name.find("conv2d") != std::string::npos) - { - return true; - } - if ((mock->m_name.find("8") != std::string::npos || - mock->m_name.find("9") != std::string::npos) && - (m_name.find("8") != std::string::npos || m_name.find("9") != std::string::npos)) + if (!MockSetup::m_equal_data_node1.empty() && !MockSetup::m_equal_data_node2.empty()) { - return true; + if ((mock->m_name.find(MockSetup::m_equal_data_node1) != std::string::npos || + mock->m_name.find(MockSetup::m_equal_data_node2) != std::string::npos) && + (m_name.find(MockSetup::m_equal_data_node1) != std::string::npos || + m_name.find(MockSetup::m_equal_data_node2) != std::string::npos)) + { + return true; + } } return mock->m_is_op == m_is_op; } @@ -390,7 +418,7 @@ class MOCK_API FrontEndMockPy : public FrontEnd static void clear_stat() { m_stat = {}; } -protected: +private: InputModel::Ptr load_impl(const std::vector>& params) const override { if (params.size() > 0 && is_type>(params[0])) diff --git a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp index aa61dbdd5c2447..d2d17042cfc5bd 100644 --- a/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp +++ b/model-optimizer/unit_tests/mock_mo_frontend/mock_mo_python_api/mock_mo_python_api.cpp @@ -21,6 +21,13 @@ static void register_mock_frontend_stat(py::module m) feStat.def_property_readonly("convert_model", &FeStat::convert_model); } +static void register_mock_setup(py::module m) +{ + m.def("clear_setup", &MockSetup::clear_setup); + m.def("set_equal_data", &MockSetup::set_equal_data); + m.def("set_max_port_counts", &MockSetup::set_max_port_counts); +} + static void register_mock_model_stat(py::module m) { m.def("get_model_statistic", &InputModelMockPy::get_stat); @@ -75,6 +82,7 @@ PYBIND11_MODULE(mock_mo_python_api, m) { m.doc() = "Mock frontend call counters for testing Pyngraph frontend bindings"; register_mock_frontend_stat(m); + register_mock_setup(m); register_mock_model_stat(m); register_mock_place_stat(m); }