From 6ea4aec3c86f0635c9d49b965ad0fccc7d8df8f3 Mon Sep 17 00:00:00 2001 From: Tikhonov Ivan Date: Fri, 20 Sep 2024 12:48:33 +0400 Subject: [PATCH 1/4] fix Validate pass behaviour --- .../tests/utils/convert_precision.cpp | 57 ++++-- src/core/include/openvino/pass/manager.hpp | 2 +- src/core/src/pass/manager.cpp | 14 +- src/core/tests/pass_manager.cpp | 175 ++++++++++++++++++ 4 files changed, 226 insertions(+), 22 deletions(-) diff --git a/src/common/transformations/tests/utils/convert_precision.cpp b/src/common/transformations/tests/utils/convert_precision.cpp index 03095b28d96a74..c88fd0dde4f7f6 100644 --- a/src/common/transformations/tests/utils/convert_precision.cpp +++ b/src/common/transformations/tests/utils/convert_precision.cpp @@ -22,6 +22,7 @@ #include "openvino/opsets/opset9.hpp" #include "openvino/pass/manager.hpp" #include "openvino/pass/visualize_tree.hpp" +#include "openvino/runtime/core.hpp" #include "ov_ops/type_relaxed.hpp" #include "transformations/common_optimizations/disable_shapeof_constant_folding.hpp" #include "transformations/rt_info/disable_fp16_compression.hpp" @@ -1009,27 +1010,49 @@ TEST(TransformationTests, ConvertPrecision_TypeRelaxed) { } TEST(TransformationTests, ConvertPrecision_Variables) { - std::shared_ptr f(nullptr); - { - Shape shape{1, 10, 2}; - auto inp = std::make_shared(element::f16, shape); - auto m_i = std::make_shared(element::f16, shape, 1); - auto m_r = std::make_shared(m_i, "ID"); - auto sum = std::make_shared(inp, m_r); - auto m_w = std::make_shared(sum, "ID"); - auto mul = std::make_shared(inp, sum); + ov::Core core; + Shape shape{1}; + element::Type data_type = element::boolean; + using c_type = bool; - mul->add_control_dependency(m_w); + auto inp = std::make_shared(data_type, shape); + auto m_i = std::make_shared(data_type, Shape{1}, 1); - f = std::make_shared(NodeVector{mul}, ParameterVector{inp}); + std::string variable_id = "ID"; + ov::op::util::VariableInfo var_info{Shape{1}, data_type, variable_id}; + auto variable = make_shared(var_info); - pass::Manager manager; - manager.register_pass(); - manager.register_pass(precisions_map{{element::f16, element::f32}}); - manager.run_passes(f); + auto m_r = std::make_shared(m_i, variable); + auto sum = std::make_shared(inp, m_r); + auto assign = std::make_shared(m_r, variable); + auto res = std::make_shared(sum); + + auto f = std::make_shared(ResultVector{res}, SinkVector{assign}, ParameterVector{inp}); + + auto compiled = core.compile_model(f, "GPU"); + auto infer = compiled.create_infer_request(); + + ov::Tensor in(data_type, shape); + auto in_data = in.data(); + for (size_t i = 0; i < ov::shape_size(shape); ++i) { + *in_data++ = 1; } - OV_ASSERT_NO_THROW(check_rt_info(f)); - ASSERT_FALSE(has_type(f)); + + infer.set_input_tensor(in); + infer.infer(); + + std::cout << "out after 1nd infer" << std::endl; + auto out = infer.get_output_tensor(); + auto out_values = out.data(); + std::cout << out_values[0] << std::endl; + + ov::Tensor tmp(data_type, shape); + auto tmp_data = tmp.data(); + *tmp_data = 0; + infer.query_state()[0].set_state(tmp); + infer.set_input_tensor(in); + + infer.infer(); } TEST(TransformationTests, ConvertPrecision_skip_precision_sensitive) { diff --git a/src/core/include/openvino/pass/manager.hpp b/src/core/include/openvino/pass/manager.hpp index a026957697f2db..ead5d99ee3f0f4 100644 --- a/src/core/include/openvino/pass/manager.hpp +++ b/src/core/include/openvino/pass/manager.hpp @@ -101,7 +101,7 @@ class OPENVINO_API Manager { std::string m_name = "UnnamedManager"; private: - bool run_pass(const std::shared_ptr& pass, const std::shared_ptr& model, bool needs_validate); + bool run_pass(const std::shared_ptr& pass, const std::shared_ptr& model, bool& needs_validate); }; } // namespace pass } // namespace ov diff --git a/src/core/src/pass/manager.cpp b/src/core/src/pass/manager.cpp index 86c2e57e03599a..27fea5db29e5fe 100644 --- a/src/core/src/pass/manager.cpp +++ b/src/core/src/pass/manager.cpp @@ -336,16 +336,18 @@ bool ov::pass::Manager::run_passes(const shared_ptr& model) { bool model_changed = false; bool pass_changed_model = false; + bool needs_validate = false; profiler.start_timer(m_name); for (const auto& pass : m_pass_list) { const auto& pass_name = pass->get_name(); profiler.start_timer(pass_name); - pass_changed_model = run_pass(pass, model, pass_changed_model); + pass_changed_model = run_pass(pass, model, needs_validate); profiler.stop_timer(pass_name, pass_changed_model); model_changed = model_changed || pass_changed_model; + needs_validate = needs_validate || pass_changed_model; profiler.visualize(model, pass_name); profiler.serialize(model, pass_name); @@ -357,7 +359,7 @@ bool ov::pass::Manager::run_passes(const shared_ptr& model) { bool ov::pass::Manager::run_pass(const std::shared_ptr& pass, const std::shared_ptr& model, - bool needs_validate) { + bool& needs_validate) { if (m_pass_config->is_disabled(pass->get_type_info())) { OPENVINO_DEBUG("Pass ", pass->get_name(), " is disabled."); return false; @@ -379,9 +381,13 @@ bool ov::pass::Manager::run_pass(const std::shared_ptr& pass, // GraphRewrite is a temporary container for MatcherPass to make execution on entire ov::Model return GraphRewrite(matcher_pass).run_on_model(model); } else if (auto model_pass = dynamic_pointer_cast(pass)) { - if (dynamic_pointer_cast(model_pass) && !needs_validate) { - return false; + if (dynamic_pointer_cast(model_pass)) { + if (!needs_validate) { + return false; + } + needs_validate = false; } + return model_pass->run_on_model(model); } return false; diff --git a/src/core/tests/pass_manager.cpp b/src/core/tests/pass_manager.cpp index fe3a0e1232c814..7a78e48fabbf0e 100644 --- a/src/core/tests/pass_manager.cpp +++ b/src/core/tests/pass_manager.cpp @@ -16,8 +16,10 @@ #include "openvino/op/matmul.hpp" #include "openvino/op/multiply.hpp" #include "openvino/op/parameter.hpp" +#include "openvino/pass/graph_rewrite.hpp" #include "openvino/pass/manager.hpp" #include "openvino/pass/pass.hpp" +#include "openvino/pass/validate.hpp" using namespace ov; using namespace std; @@ -77,6 +79,100 @@ bool validate_list(const std::vector>& nodes) { } // namespace +class TestMatcherPassTrue : public ov::pass::MatcherPass { +public: + OPENVINO_RTTI("TestMatcherPassTrue"); + TestMatcherPassTrue() : MatcherPass() { + auto any_input = ov::pass::pattern::any_input(); + ov::matcher_pass_callback callback = [](ov::pass::pattern::Matcher& m) { + return true; + }; + + auto m = std::make_shared(any_input, "TestMatcherPassTrue"); + this->register_matcher(m, callback); + } +}; + +class TestMatcherPassFalse : public ov::pass::MatcherPass { +public: + OPENVINO_RTTI("TestMatcherPassFalse"); + TestMatcherPassFalse() : MatcherPass() { + auto any_input = ov::pass::pattern::any_input(); + ov::matcher_pass_callback callback = [](ov::pass::pattern::Matcher& m) { + return false; + }; + + auto m = std::make_shared(any_input, "TestMatcherPassFalse"); + this->register_matcher(m, callback); + } +}; + +class TestModelPassTrue : public pass::ModelPass { +public: + OPENVINO_RTTI("TestModelPassTrue"); + + bool run_on_model(const std::shared_ptr& f) override { + return true; + } +}; + +class TestModelPassFalse : public pass::ModelPass { +public: + OPENVINO_RTTI("TestModelPassFalse"); + + bool run_on_model(const std::shared_ptr& f) override { + return false; + } +}; + +class TestValidate : public pass::Validate { +public: + OPENVINO_RTTI("TestValidate"); + + bool run_on_model(const std::shared_ptr& f) override { + m_applied = true; + return pass::Validate::run_on_model(f); + } + + bool is_applied() const { + return m_applied; + } + +private: + bool m_applied = false; +}; + +class TestValidate2 : public TestValidate {}; + +class TestManager : public pass::Manager { +public: + bool is_validation_applied() { + bool applied = false; + bool is_init = true; + for (const auto& pass : m_pass_list) { + auto validate_2 = std::dynamic_pointer_cast(pass); + auto validate = std::dynamic_pointer_cast(pass); + if (validate && !validate_2) { + if (is_init) { + is_init = false; + applied = validate->is_applied(); + } + applied = applied && validate->is_applied(); + } + } + return applied; + } + + bool is_2nd_validation_applied() { + for (const auto& pass : m_pass_list) { + if (auto validate = std::dynamic_pointer_cast(pass)) { + return validate->is_applied(); + } + } + return false; + } +}; + TEST(pass_manager, add) { pass::Manager pass_manager; @@ -90,3 +186,82 @@ TEST(pass_manager, add) { EXPECT_EQ(node_count, sorted.size()); EXPECT_TRUE(validate_list(sorted)); } + +TEST(pass_manager, passes_not_applied) { + TestManager pass_manager; + pass_manager.set_per_pass_validation(false); + + auto graph = make_test_graph(); + + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + const auto res = pass_manager.run_passes(graph); + + EXPECT_FALSE(res); + EXPECT_FALSE(pass_manager.is_validation_applied()); +} + +TEST(pass_manager, model_pass_applied) { + TestManager pass_manager; + pass_manager.set_per_pass_validation(false); + + auto graph = make_test_graph(); + + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + const auto res = pass_manager.run_passes(graph); + + EXPECT_TRUE(res); + EXPECT_TRUE(pass_manager.is_validation_applied()); +} + +TEST(pass_manager, matcher_pass_applied) { + TestManager pass_manager; + pass_manager.set_per_pass_validation(false); + + auto graph = make_test_graph(); + + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + const auto res = pass_manager.run_passes(graph); + + EXPECT_TRUE(res); + EXPECT_TRUE(pass_manager.is_validation_applied()); +} + +TEST(pass_manager, two_validations) { + TestManager pass_manager; + pass_manager.set_per_pass_validation(false); + + auto graph = make_test_graph(); + + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + pass_manager.register_pass(); + const auto res = pass_manager.run_passes(graph); + + EXPECT_TRUE(res); + EXPECT_TRUE(pass_manager.is_validation_applied()); + EXPECT_FALSE(pass_manager.is_2nd_validation_applied()); +} \ No newline at end of file From c8837be78320eaeb824bff3807957ab6a362af55 Mon Sep 17 00:00:00 2001 From: Tikhonov Ivan Date: Fri, 20 Sep 2024 12:49:49 +0400 Subject: [PATCH 2/4] delete fixme --- .../common_optimizations/simplify_shape_of_sub_graph.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/common/transformations/src/transformations/common_optimizations/simplify_shape_of_sub_graph.cpp b/src/common/transformations/src/transformations/common_optimizations/simplify_shape_of_sub_graph.cpp index a225f0655f98ee..3032c4b7d56c7a 100644 --- a/src/common/transformations/src/transformations/common_optimizations/simplify_shape_of_sub_graph.cpp +++ b/src/common/transformations/src/transformations/common_optimizations/simplify_shape_of_sub_graph.cpp @@ -358,8 +358,6 @@ bool pass::SimplifyShapeOfSubGraph::run_on_model(const std::shared_ptr& f REGISTER_PASS(manager, PrepareShapeOpsForEliminationAroundBE) REGISTER_PASS(manager, AbsSinking) - // FIXME: manager runs Validate based on the last pass, when fixed the following line must be deleted - REGISTER_PASS(manager, Validate) REGISTER_PASS(manager, SharedOpOptimization) REGISTER_PASS(manager, EliminateGatherUnsqueeze) // should run after SharedOpOptimization REGISTER_PASS(manager, NopElimination, m_use_shapes) From 1544c56c74b06ceeebd3e1d8d81f8c8b9ce03a25 Mon Sep 17 00:00:00 2001 From: Tikhonov Ivan Date: Fri, 20 Sep 2024 12:53:11 +0400 Subject: [PATCH 3/4] revert debug changes --- .../tests/utils/convert_precision.cpp | 57 ++++++------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/src/common/transformations/tests/utils/convert_precision.cpp b/src/common/transformations/tests/utils/convert_precision.cpp index c88fd0dde4f7f6..03095b28d96a74 100644 --- a/src/common/transformations/tests/utils/convert_precision.cpp +++ b/src/common/transformations/tests/utils/convert_precision.cpp @@ -22,7 +22,6 @@ #include "openvino/opsets/opset9.hpp" #include "openvino/pass/manager.hpp" #include "openvino/pass/visualize_tree.hpp" -#include "openvino/runtime/core.hpp" #include "ov_ops/type_relaxed.hpp" #include "transformations/common_optimizations/disable_shapeof_constant_folding.hpp" #include "transformations/rt_info/disable_fp16_compression.hpp" @@ -1010,49 +1009,27 @@ TEST(TransformationTests, ConvertPrecision_TypeRelaxed) { } TEST(TransformationTests, ConvertPrecision_Variables) { - ov::Core core; - Shape shape{1}; - element::Type data_type = element::boolean; - using c_type = bool; - - auto inp = std::make_shared(data_type, shape); - auto m_i = std::make_shared(data_type, Shape{1}, 1); - - std::string variable_id = "ID"; - ov::op::util::VariableInfo var_info{Shape{1}, data_type, variable_id}; - auto variable = make_shared(var_info); - - auto m_r = std::make_shared(m_i, variable); - auto sum = std::make_shared(inp, m_r); - auto assign = std::make_shared(m_r, variable); - auto res = std::make_shared(sum); + std::shared_ptr f(nullptr); + { + Shape shape{1, 10, 2}; + auto inp = std::make_shared(element::f16, shape); + auto m_i = std::make_shared(element::f16, shape, 1); + auto m_r = std::make_shared(m_i, "ID"); + auto sum = std::make_shared(inp, m_r); + auto m_w = std::make_shared(sum, "ID"); + auto mul = std::make_shared(inp, sum); - auto f = std::make_shared(ResultVector{res}, SinkVector{assign}, ParameterVector{inp}); + mul->add_control_dependency(m_w); - auto compiled = core.compile_model(f, "GPU"); - auto infer = compiled.create_infer_request(); + f = std::make_shared(NodeVector{mul}, ParameterVector{inp}); - ov::Tensor in(data_type, shape); - auto in_data = in.data(); - for (size_t i = 0; i < ov::shape_size(shape); ++i) { - *in_data++ = 1; + pass::Manager manager; + manager.register_pass(); + manager.register_pass(precisions_map{{element::f16, element::f32}}); + manager.run_passes(f); } - - infer.set_input_tensor(in); - infer.infer(); - - std::cout << "out after 1nd infer" << std::endl; - auto out = infer.get_output_tensor(); - auto out_values = out.data(); - std::cout << out_values[0] << std::endl; - - ov::Tensor tmp(data_type, shape); - auto tmp_data = tmp.data(); - *tmp_data = 0; - infer.query_state()[0].set_state(tmp); - infer.set_input_tensor(in); - - infer.infer(); + OV_ASSERT_NO_THROW(check_rt_info(f)); + ASSERT_FALSE(has_type(f)); } TEST(TransformationTests, ConvertPrecision_skip_precision_sensitive) { From 91e8729ad6fa647574915297ced19d9201443655 Mon Sep 17 00:00:00 2001 From: Tikhonov Ivan Date: Mon, 23 Sep 2024 16:12:01 +0400 Subject: [PATCH 4/4] fix build --- src/core/src/pass/manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/pass/manager.cpp b/src/core/src/pass/manager.cpp index 8138b5ed44dff7..03bb4fa07c0b65 100644 --- a/src/core/src/pass/manager.cpp +++ b/src/core/src/pass/manager.cpp @@ -379,8 +379,8 @@ bool ov::pass::Manager::run_pass(const std::shared_ptr& pass, if (auto matcher_pass = std::dynamic_pointer_cast(pass)) { // GraphRewrite is a temporary container for MatcherPass to make execution on entire ov::Model return GraphRewrite(matcher_pass).run_on_model(model); - } else if (auto model_pass = dynamic_pointer_cast(pass)) { - if (dynamic_pointer_cast(model_pass)) { + } else if (auto model_pass = std::dynamic_pointer_cast(pass)) { + if (std::dynamic_pointer_cast(model_pass)) { if (!needs_validate) { return false; }