From 3ec40c490b955324b721d151a7b6149d400f5d94 Mon Sep 17 00:00:00 2001 From: Ivan Tikhonov Date: Fri, 11 Nov 2022 23:31:58 +0400 Subject: [PATCH] Fix copying of tensor names in OVTF TransposeSinking transformation (#13916) * copy tensor names to transpose * enable TransposeSinking, codestyle * fix layer tests * fix segfault * delete transposes in catch block --- src/frontends/tensorflow/src/frontend.cpp | 3 +- .../tensorflow/src/pass/transpose_sinking.cpp | 70 ++++++++++++------- .../tensorflow/tests/transpose_sinking.cpp | 43 ++++++++++++ 3 files changed, 90 insertions(+), 26 deletions(-) diff --git a/src/frontends/tensorflow/src/frontend.cpp b/src/frontends/tensorflow/src/frontend.cpp index ba7d5ce0ff2..031fcfce76c 100644 --- a/src/frontends/tensorflow/src/frontend.cpp +++ b/src/frontends/tensorflow/src/frontend.cpp @@ -434,8 +434,7 @@ void FrontEnd::normalize(const std::shared_ptr& function) const { manager.register_pass(); // TODO: reimplement TransposeSinking that does not corrupt filters for Convolution - // and preserve tensor names in case of sinking - // manager.register_pass(); + manager.register_pass(); manager.run_passes(function); } diff --git a/src/frontends/tensorflow/src/pass/transpose_sinking.cpp b/src/frontends/tensorflow/src/pass/transpose_sinking.cpp index bba56f16efc..e3bdb363cc9 100644 --- a/src/frontends/tensorflow/src/pass/transpose_sinking.cpp +++ b/src/frontends/tensorflow/src/pass/transpose_sinking.cpp @@ -53,9 +53,20 @@ static string describe(shared_ptr node) { stringstream ss; auto transpose = as_type_ptr(node); auto const1 = as_type_ptr(transpose->get_input_node_shared_ptr(1)); - ss << transpose->get_name() << " ( axis order = " << ov::util::vector_to_string(const1->get_axis_vector_val()) - << " , shape = " << ov::util::vector_to_string(transpose->get_shape()) << " ) " - << " , input = " << transpose->input_value(0).get_node()->get_name(); + if (transpose) { + ss << "transpose name: " << transpose->get_name(); + ss << " , input = " << transpose->input_value(0).get_node()->get_name(); + if (transpose->output(0).get_partial_shape().is_static()) { + ss << " , shape = " << ov::util::vector_to_string(transpose->output(0).get_shape()); + } + if (const1) { + ss << " , axis order = " << ov::util::vector_to_string(const1->get_axis_vector_val()); + } else { + ss << " , axis order = (unknown, not constant values)"; + } + } else { + ss << "Node can not be cast to Transpose/Reshape operations."; + } return ss.str(); } @@ -76,7 +87,7 @@ static void write_transposemap(TransposeMap& reorders, static shared_ptr read_transposemap(TransposeMap& reorders, const Output& target) { auto name = target.get_node()->get_name() + "." + to_string(target.get_index()); - auto transpose = reorders[name]; + auto transpose = reorders.at(name); OPENVINO_DEBUG << "Read TransposeMap[" << name << "] -> " << describe(transpose); return transpose; } @@ -86,13 +97,16 @@ static shared_ptr combine_transposes(const shared_ptr& t1, auto t1_const = as_type_ptr(t1->input_value(1).get_node_shared_ptr()); auto t2_const = as_type_ptr(t2->input_value(1).get_node_shared_ptr()); - auto perm_t1 = apply_permutation(default_order, t1_const->get_axis_vector_val()); - auto perm_t2 = apply_permutation(perm_t1, t2_const->get_axis_vector_val()); + if (t1_const && t2_const) { + auto perm_t1 = apply_permutation(default_order, t1_const->get_axis_vector_val()); + auto perm_t2 = apply_permutation(perm_t1, t2_const->get_axis_vector_val()); - auto combined = make_transpose(t2->input_value(0), perm_t2); - OPENVINO_DEBUG << "Combining " << describe(t1) << " and " << describe(t2) << " into " - << describe(combined); - return combined; + auto combined = make_transpose(t2->input_value(0), perm_t2); + OPENVINO_DEBUG << "Combining " << describe(t1) << " and " << describe(t2) << " into " + << describe(combined); + return combined; + } + return {}; } static void insert_transpose(const shared_ptr& target, const shared_ptr& transpose, size_t input_index) { @@ -105,6 +119,10 @@ static void insert_transpose(const shared_ptr& target, const shared_ptrinput(input_index).replace_source_output(new_transpose->output(0)); + if (std::dynamic_pointer_cast(target)) { + new_transpose->output(0).add_names(arg.get_names()); + arg.set_names({}); + } } static void delete_transpose(const shared_ptr& transpose) { @@ -113,10 +131,7 @@ static void delete_transpose(const shared_ptr& transpose) { Output output = transpose->output(0); OPENVINO_DEBUG << "output " << output.get_node_shared_ptr()->get_name(); OPENVINO_DEBUG << "target input size " << output.get_target_inputs().size(); - for (auto input : output.get_target_inputs()) { - OPENVINO_DEBUG << "input " << input.get_node()->get_name(); - input.replace_source_output(transpose->input_value(0)); - } + output.replace(transpose->input_value(0)); } } @@ -203,13 +218,15 @@ static void sink_transpose(const shared_ptr& transpose, auto orig_transpose = read_transposemap(reorders, transpose_in); // combine both transposes auto new_transpose = combine_transposes(orig_transpose, transpose); - // remove original transpose now it's combined with a new one - // should be safe to remove an already detached node - mark_transpose_for_deletion(orig_transpose, transposes_to_delete); - // replace transpose with combined one - replace_node(transpose, new_transpose); - mark_transpose_for_deletion(new_transpose, transposes_to_delete); - write_transposemap(reorders, new_transpose, new_transpose); + if (new_transpose) { + // remove original transpose now it's combined with a new one + // should be safe to remove an already detached node + mark_transpose_for_deletion(orig_transpose, transposes_to_delete); + // replace transpose with combined one + replace_node(transpose, new_transpose); + mark_transpose_for_deletion(new_transpose, transposes_to_delete); + write_transposemap(reorders, new_transpose, new_transpose); + } } static void sink_unary(const shared_ptr& n, @@ -365,6 +382,12 @@ static void sink_prelu(const shared_ptr& prelu, } } +void purge_transposes(const set>& transposes_to_delete) { + for (const auto& r : transposes_to_delete) { + delete_transpose(r); + } +} + // The goal of TransposeSinking is to remove // round-trip transposes(i.e. nhwc->nchw(nchw-only-op)->nhwc) // around nchw-only-op (e.g.Convolution, Batchnorm, Avg/MaxPool) @@ -408,14 +431,13 @@ bool ov::frontend::tensorflow::pass::TransposeSinking::run_on_model(const shared } } catch (...) { OPENVINO_DEBUG << "Caught exception while sinking op"; + purge_transposes(transposes_to_delete); return false; } // STEP 2: purge all the transposes we either sunk or swam. OPENVINO_DEBUG << "Purging transposes "; - for (const auto& r : transposes_to_delete) { - delete_transpose(r); - } + purge_transposes(transposes_to_delete); // STEP 3: fix wrong shape info wholesale OPENVINO_DEBUG << "Fixing wrong shape info for the whole graph"; diff --git a/src/frontends/tensorflow/tests/transpose_sinking.cpp b/src/frontends/tensorflow/tests/transpose_sinking.cpp index 009f6572877..9246c114cec 100644 --- a/src/frontends/tensorflow/tests/transpose_sinking.cpp +++ b/src/frontends/tensorflow/tests/transpose_sinking.cpp @@ -32,6 +32,49 @@ TEST(TransposeSinkingTest, PassProperty) { ASSERT_FALSE(pass->get_property(ov::pass::PassProperty::CHANGE_DYNAMIC_STATE)); } +TEST(TransposeSinkingTest, TensorNames) { + ngraph::Shape shape_nhwc{16, 28, 28, 1}; + auto a = make_shared(ngraph::element::i32, shape_nhwc); + auto ng_order = std::make_shared(ngraph::element::u64, ngraph::Shape{4}, ngraph::Shape{0, 3, 1, 2}); + auto transpose = make_shared(a, ng_order); + auto absn = make_shared(transpose); + auto absn2 = make_shared(absn); + absn2->output(0).set_names({"out_name"}); + auto res = make_shared(absn2); + auto func = make_shared(ngraph::OutputVector{res}, ngraph::ParameterVector{a}); + + ov::pass::Manager pass_manager; + pass_manager.register_pass(); + pass_manager.run_passes(func); + + auto new_transpose = + ngraph::as_type_ptr(func->get_results().at(0)->input_value(0).get_node_shared_ptr()); + ASSERT_TRUE(new_transpose); + EXPECT_EQ(new_transpose->output(0).get_names(), std::unordered_set({"out_name"})); +} + +TEST(TransposeSinkingTest, TensorNamesCombineTransposes) { + ngraph::Shape shape_nhwc{16, 28, 28, 1}; + auto a = make_shared(ngraph::element::i32, shape_nhwc); + auto ng_order = std::make_shared(ngraph::element::u64, ngraph::Shape{4}, ngraph::Shape{0, 3, 1, 2}); + auto transpose_1 = make_shared(a, ng_order); + auto transpose_2 = make_shared(transpose_1, ng_order); + transpose_2->output(0).set_names({"out_name"}); + auto res = make_shared(transpose_2); + auto func = make_shared(ngraph::OutputVector{res}, ngraph::ParameterVector{a}); + + ov::pass::Manager pass_manager; + pass_manager.register_pass(); + pass_manager.run_passes(func); + + auto new_transpose = + ngraph::as_type_ptr(func->get_results().at(0)->input_value(0).get_node_shared_ptr()); + ASSERT_TRUE(new_transpose); + EXPECT_EQ(new_transpose->output(0).get_names(), std::unordered_set({"out_name"})); + size_t transpose_cnt = count_ops_of_type(func); + EXPECT_EQ(transpose_cnt, 1); +} + TEST(TransposeSinkingTest, EdgeSplitting) { // checks if Transpose is pushed through Abs, but stopped by // ReduceSum