From 45bdf627cb95862de59b18b378e6d0ffc6ea0690 Mon Sep 17 00:00:00 2001 From: Mikhail Nosov Date: Sat, 11 Dec 2021 03:42:46 +0300 Subject: [PATCH] [OV20] Preprocessing: User friendly error message for convert_layout (#9077) * Fix incomprehensible error message during layout conversion when layout rank doesn't match with shape rank * clang-format fix, Removed debug print * Updated unit test according to review comments * Moved apply_permutation and find_permutation to src/layout_utils.hpp --- src/core/include/openvino/core/layout.hpp | 15 +----- src/core/src/layout.cpp | 46 ++++++++++++++++--- src/core/src/layout_utils.hpp | 22 +++++++++ src/core/src/preprocess/pre_post_process.cpp | 3 +- .../src/preprocess/preprocess_steps_impl.cpp | 9 ++-- src/core/tests/preprocess.cpp | 25 ++++++++++ 6 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 src/core/src/layout_utils.hpp diff --git a/src/core/include/openvino/core/layout.hpp b/src/core/include/openvino/core/layout.hpp index ef55e966a6e..1bfc56a6669 100644 --- a/src/core/include/openvino/core/layout.hpp +++ b/src/core/include/openvino/core/layout.hpp @@ -15,15 +15,6 @@ namespace ov { -class Layout; - -namespace layout { - -std::vector find_permutation(const Layout& src_layout, const Rank& src_shape_rank, const Layout& dst_layout); -Layout apply_permutation(const Layout& src_layout, const std::vector& dims); - -} // namespace layout - class OPENVINO_API Layout { public: /// \brief Constructs a dynamic Layout with no layout information. @@ -85,11 +76,7 @@ private: int64_t m_left_size = 0; int64_t m_right_size = 0; - 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 Layout& dst_layout); + friend class LayoutUtils; }; namespace layout { diff --git a/src/core/src/layout.cpp b/src/core/src/layout.cpp index afeafa46551..244f8f2d7ef 100644 --- a/src/core/src/layout.cpp +++ b/src/core/src/layout.cpp @@ -7,6 +7,7 @@ #include #include +#include "layout_utils.hpp" #include "ngraph/except.hpp" #include "ngraph/util.hpp" @@ -238,9 +239,15 @@ std::string Layout::to_string() const { return res.str(); } -namespace layout { +class LayoutUtils { +public: + static Layout apply_permutation(const Layout& src_layout, const std::vector& dims); + static 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) { +Layout LayoutUtils::apply_permutation(const Layout& src_layout, const std::vector& dims) { { // Validate dims std::vector used(dims.size(), false); for (size_t i = 0; i < dims.size(); i++) { @@ -274,7 +281,10 @@ 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 LayoutUtils::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,21 +336,32 @@ 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 {}; } for (size_t i = 0; i < inverted.size(); i++) { - res[inverted[i]] = i; + res[inverted[i]] = static_cast(i); } return check_trivial(res); } std::vector mapped(src_static.m_left_size, false); // Fill known names (??c? -> nc??) will produce res=[-1,2,-1,-1], mapped=[false,false,true,false] - for (auto src_item : src_static.m_index_map) { + for (const auto& src_item : src_static.m_index_map) { OPENVINO_ASSERT(dst.has_name(src_item.second), "Dimension name '", src_item.second, @@ -372,6 +393,19 @@ std::vector find_permutation(const Layout& src_layout, const Rank& rank return check_trivial(res); } +namespace layout { +namespace utils { +Layout apply_permutation(const Layout& src_layout, const std::vector& dims) { + return LayoutUtils::apply_permutation(src_layout, dims); +} + +std::vector find_permutation(const Layout& src_layout, + const PartialShape& src_shape, + const Layout& dst_layout) { + return LayoutUtils::find_permutation(src_layout, src_shape, dst_layout); +} +} // namespace utils + // Helper functions bool has_batch(const Layout& layout) { return layout.has_name(BATCH); diff --git a/src/core/src/layout_utils.hpp b/src/core/src/layout_utils.hpp new file mode 100644 index 00000000000..c6d9e8e32da --- /dev/null +++ b/src/core/src/layout_utils.hpp @@ -0,0 +1,22 @@ +// Copyright (C) 2018-2021 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 +// + +#pragma once + +#include + +#include "openvino/core/partial_shape.hpp" + +namespace ov { +namespace layout { +namespace utils { + +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 utils +} // namespace layout +} // namespace ov diff --git a/src/core/src/preprocess/pre_post_process.cpp b/src/core/src/preprocess/pre_post_process.cpp index 85d80f0fa5a..00b8b809109 100644 --- a/src/core/src/preprocess/pre_post_process.cpp +++ b/src/core/src/preprocess/pre_post_process.cpp @@ -6,6 +6,7 @@ #include "color_utils.hpp" #include "function_guard.hpp" +#include "layout_utils.hpp" #include "ngraph/opsets/opset1.hpp" #include "openvino/core/model.hpp" #include "preprocess_steps_impl.hpp" @@ -399,7 +400,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::utils::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 2a7e1d1a83d..948586744b3 100644 --- a/src/core/src/preprocess/preprocess_steps_impl.cpp +++ b/src/core/src/preprocess/preprocess_steps_impl.cpp @@ -5,6 +5,7 @@ #include "preprocess_steps_impl.hpp" #include "color_utils.hpp" +#include "layout_utils.hpp" #include "openvino/core/node.hpp" #include "openvino/core/shape.hpp" #include "openvino/op/nv12_to_bgr.hpp" @@ -174,7 +175,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::utils::find_permutation(context.layout(), nodes[0].get_partial_shape(), dst_layout); if (permutation.empty()) { // No transpose is needed, just update layout if (!layout.empty()) { @@ -203,7 +204,7 @@ void PreStepsList::add_convert_layout_impl(const std::vector& dims) { OPENVINO_ASSERT(nodes.size() == 1, "Can't convert layout for multi-plane input. Suggesting to convert current image to " "RGB/BGR color format using 'convert_color'"); - auto new_layout = layout::apply_permutation(context.layout(), dims); + auto new_layout = layout::utils::apply_permutation(context.layout(), dims); auto perm_constant = op::v0::Constant::create(element::u64, Shape{dims.size()}, dims); auto transpose = std::make_shared(nodes[0], perm_constant); context.layout() = std::move(new_layout); // Update context's current layout @@ -430,7 +431,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::utils::find_permutation(context.layout(), node.get_partial_shape(), dst_layout); if (permutation.empty()) { // No transpose is needed, just update layout if (!layout.empty()) { @@ -451,7 +452,7 @@ void PostStepsList::add_convert_layout_impl(const std::vector& dims) { } m_actions.emplace_back([dims](const Output& node, PostprocessingContext& context) { auto perm_constant = op::v0::Constant::create(element::u64, Shape{dims.size()}, dims); - auto new_layout = layout::apply_permutation(context.layout(), dims); + auto new_layout = layout::utils::apply_permutation(context.layout(), dims); auto transpose = std::make_shared(node, perm_constant); auto res = std::make_tuple(Output(transpose), true); context.layout() = std::move(new_layout); // Update context's current layout diff --git a/src/core/tests/preprocess.cpp b/src/core/tests/preprocess.cpp index efc08e27955..a2673daa5c7 100644 --- a/src/core/tests/preprocess.cpp +++ b/src/core/tests/preprocess.cpp @@ -534,6 +534,31 @@ 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{1, 2}; + std::stringstream shape_str; + shape_str << shape; + auto f = create_simple_function(element::f32, shape); + Layout from{"NHWC"}; + Layout to{"NCHW"}; + // 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) { + // 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"; + } +} + 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??");