From 37064741b28e0539838456009bdde82d24ad6698 Mon Sep 17 00:00:00 2001 From: Mikhail Nosov Date: Wed, 8 Dec 2021 14:55:36 +0300 Subject: [PATCH] Fix incomprehensible error message during layout conversion when layout rank doesn't match with shape rank --- src/core/include/openvino/core/layout.hpp | 4 +-- src/core/src/layout.cpp | 16 +++++++++-- src/core/src/preprocess/pre_post_process.cpp | 2 +- .../src/preprocess/preprocess_steps_impl.cpp | 4 +-- src/core/tests/preprocess.cpp | 27 +++++++++++++++++++ 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/core/include/openvino/core/layout.hpp b/src/core/include/openvino/core/layout.hpp index ef55e966a6e..b2f405de522 100644 --- a/src/core/include/openvino/core/layout.hpp +++ b/src/core/include/openvino/core/layout.hpp @@ -19,7 +19,7 @@ class Layout; namespace layout { -std::vector find_permutation(const Layout& src_layout, const Rank& src_shape_rank, const Layout& dst_layout); +std::vector find_permutation(const Layout& src_layout, const PartialShape& src_shape, const Layout& dst_layout); Layout apply_permutation(const Layout& src_layout, const std::vector& dims); } // namespace layout @@ -88,7 +88,7 @@ private: friend Layout layout::apply_permutation(const Layout& src_layout, const std::vector& dims); friend std::vector layout::find_permutation(const Layout& src_layout, - const Rank& src_shape_rank, + const PartialShape& src_shape, const Layout& dst_layout); }; diff --git a/src/core/src/layout.cpp b/src/core/src/layout.cpp index afeafa46551..2b374f42ba8 100644 --- a/src/core/src/layout.cpp +++ b/src/core/src/layout.cpp @@ -274,7 +274,8 @@ Layout apply_permutation(const Layout& src_layout, const std::vector& return res; } -std::vector find_permutation(const Layout& src_layout, const Rank& rank, const Layout& dst) { +std::vector find_permutation(const Layout& src_layout, const PartialShape& src_shape, const Layout& dst) { + auto rank = src_shape.rank(); auto check_trivial = [](std::vector& res) -> std::vector& { size_t i = 0; while (i < res.size() && res[i] == i) { @@ -326,10 +327,21 @@ std::vector find_permutation(const Layout& src_layout, const Rank& rank auto dst_static = to_static(dst, rank); OPENVINO_ASSERT(src_static.m_left_size == dst_static.m_left_size, "Conversion is not supported for layouts with different sizes"); + OPENVINO_ASSERT(rank.is_dynamic() || src_static.m_left_size == rank.get_length(), + "Conversion layout ", + src_layout.to_string(), + " <-> ", + dst.to_string(), + " failure. Layout is not consistent with input shape ", + src_shape, + ". Layout length ", + src_static.m_left_size, + " shall match with input shape rank ", + rank.get_length()); std::vector res(src_static.m_left_size, -1); if (src_static.m_names.size() > dst_static.m_names.size()) { // find inverted permutation from least specified layout to most one - auto inverted = find_permutation(dst_static, rank, src_static); + auto inverted = find_permutation(dst_static, src_shape, src_static); if (inverted.empty()) { return {}; } diff --git a/src/core/src/preprocess/pre_post_process.cpp b/src/core/src/preprocess/pre_post_process.cpp index 644d393f36f..9aeef224446 100644 --- a/src/core/src/preprocess/pre_post_process.cpp +++ b/src/core/src/preprocess/pre_post_process.cpp @@ -399,7 +399,7 @@ std::shared_ptr PrePostProcessor::build() { param->get_layout() != input->get_tensor_data()->get_layout()) { // Find transpose between model and tensor layouts and update tensor shape auto net_to_tensor = - layout::find_permutation(param->get_layout(), net_shape.rank(), input->get_tensor_data()->get_layout()); + layout::find_permutation(param->get_layout(), net_shape, input->get_tensor_data()->get_layout()); if (!net_to_tensor.empty()) { std::vector dims(new_param_shape.size()); std::transform(net_to_tensor.begin(), net_to_tensor.end(), dims.begin(), [&](int64_t v) { diff --git a/src/core/src/preprocess/preprocess_steps_impl.cpp b/src/core/src/preprocess/preprocess_steps_impl.cpp index af402095b12..47bdd3469a2 100644 --- a/src/core/src/preprocess/preprocess_steps_impl.cpp +++ b/src/core/src/preprocess/preprocess_steps_impl.cpp @@ -174,7 +174,7 @@ void PreStepsList::add_convert_layout_impl(const Layout& layout) { "Can't convert layout for multi-plane input. Suggesting to convert current image to " "RGB/BGR color format using 'convert_color'"); Layout dst_layout = layout.empty() ? context.target_layout() : layout; - auto permutation = layout::find_permutation(context.layout(), nodes[0].get_partial_shape().rank(), dst_layout); + auto permutation = layout::find_permutation(context.layout(), nodes[0].get_partial_shape(), dst_layout); if (permutation.empty()) { // No transpose is needed, just update layout if (!layout.empty()) { @@ -430,7 +430,7 @@ void PostStepsList::add_convert_impl(const element::Type& type) { void PostStepsList::add_convert_layout_impl(const Layout& layout) { m_actions.emplace_back([layout](const Output& node, PostprocessingContext& context) { Layout dst_layout = layout.empty() ? context.target_layout() : layout; - auto permutation = layout::find_permutation(context.layout(), node.get_partial_shape().rank(), dst_layout); + auto permutation = layout::find_permutation(context.layout(), node.get_partial_shape(), dst_layout); if (permutation.empty()) { // No transpose is needed, just update layout if (!layout.empty()) { diff --git a/src/core/tests/preprocess.cpp b/src/core/tests/preprocess.cpp index fd496fd77f7..f34c978929a 100644 --- a/src/core/tests/preprocess.cpp +++ b/src/core/tests/preprocess.cpp @@ -534,6 +534,33 @@ TEST(pre_post_process, reuse_model_layout_no_tensor_info) { EXPECT_EQ(f->get_parameters().front()->get_layout(), "NC??"); } +TEST(pre_post_process, set_layout_out_of_bounds) { + auto shape = PartialShape{Dimension::dynamic(), 3, 2, 1}; + std::stringstream shape_str; + shape_str << shape; + auto f = create_simple_function(element::f32, shape); + Layout from {"N???C"}; + Layout to {"NC???"}; + // TODO: replace with EXPECT_THAT after upgrade gtest to v1.11 + try { + auto p = PrePostProcessor(f); + p.input().tensor().set_layout(from); + p.input().model().set_layout(to); + f = p.build(); + FAIL() << "Layout conversion shall throw"; + } catch (const ov::Exception& err) { + std::cout << err.what() << "---\n"; + // Verify that error message contains tensor and network layout + EXPECT_TRUE(std::string(err.what()).find(from.to_string()) != std::string::npos) << err.what(); + EXPECT_TRUE(std::string(err.what()).find(to.to_string()) != std::string::npos) << err.what(); + // Verify that error message contains 'shape' word + EXPECT_TRUE(std::string(err.what()).find(shape_str.str()) != std::string::npos) << err.what(); + } catch (...) { + FAIL() << "Expected ov::Exception"; + } +// EXPECT_EQ(f->get_parameters().front()->get_layout(), "NC??"); +} + TEST(pre_post_process, reuse_model_layout_tensor_info) { auto f = create_simple_function(element::u8, PartialShape{Dimension::dynamic(), 3, 2, 1}); f->get_parameters().front()->set_layout("NC??");