From 7ce40996e56985d0f7c171ffcc6f13b27830f561 Mon Sep 17 00:00:00 2001 From: Oleg Pipikin Date: Fri, 14 Apr 2023 20:12:18 +0200 Subject: [PATCH] Fix copy constructor and assignment for ov::Any (#16757) * Fix copy constructor and assignment for ov::Any * Fix1 * Apply comments * Add test * Fix code style * Fix2 * Fix3 --- src/core/include/openvino/core/any.hpp | 8 ++--- src/core/include/openvino/core/model.hpp | 17 +++++++++-- src/core/src/any.cpp | 13 +++++++++ src/core/src/model.cpp | 18 ++++++++++-- src/core/tests/any.cpp | 29 +++++++++++++++++-- src/inference/src/dev/core_impl.cpp | 13 ++------- src/plugins/hetero/plugin.cpp | 11 +------ .../tests/deprecated/unit/CMakeLists.txt | 4 +-- 8 files changed, 77 insertions(+), 36 deletions(-) diff --git a/src/core/include/openvino/core/any.hpp b/src/core/include/openvino/core/any.hpp index 00df6f531be..d312d347ccb 100644 --- a/src/core/include/openvino/core/any.hpp +++ b/src/core/include/openvino/core/any.hpp @@ -675,14 +675,14 @@ public: /// @brief Default constructor Any() = default; - /// @brief Default copy constructor + /// @brief Сopy constructor /// @param other other Any object - Any(const Any& other) = default; + Any(const Any& other); - /// @brief Default copy assignment operator + /// @brief Сopy assignment operator /// @param other other Any object /// @return reference to the current object - Any& operator=(const Any& other) = default; + Any& operator=(const Any& other); /// @brief Default move constructor /// @param other other Any object diff --git a/src/core/include/openvino/core/model.hpp b/src/core/include/openvino/core/model.hpp index 3ad2aa7c055..303c1488739 100644 --- a/src/core/include/openvino/core/model.hpp +++ b/src/core/include/openvino/core/model.hpp @@ -231,7 +231,15 @@ public: "This method is deprecated and will be removed soon. Please use evaluate with ov::Tensor instead.") bool evaluate(const ov::HostTensorVector& output_tensors, const ov::HostTensorVector& input_tensors, - ov::EvaluationContext evaluation_context = ov::EvaluationContext()) const; + ov::EvaluationContext& evaluation_context) const; + + /// \deprecated Use evaluate with ov::Tensor instead + /// \brief Evaluate the model on inputs, putting results in outputs. + /// \param output_tensors Tensors for the outputs to compute. One for each result + /// \param input_tensors Tensors for the inputs. One for each inputs. + OPENVINO_DEPRECATED( + "This method is deprecated and will be removed soon. Please use evaluate with ov::Tensor instead.") + bool evaluate(const ov::HostTensorVector& output_tensors, const ov::HostTensorVector& input_tensors) const; /// \brief Evaluate the model on inputs, putting results in outputs. /// \param output_tensors Tensors for the outputs to compute. One for each result @@ -240,7 +248,12 @@ public: /// when evaluating the model. This additional information can be shared across nodes. bool evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors, - ov::EvaluationContext evaluation_context = ov::EvaluationContext()) const; + ov::EvaluationContext& evaluation_context) const; + + /// \brief Evaluate the model on inputs, putting results in outputs. + /// \param output_tensors Tensors for the outputs to compute. One for each result + /// \param input_tensors Tensors for the inputs. One for each inputs. + bool evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors) const; /// \brief Return a list of model's sinks. const ov::SinkVector& get_sinks() const { diff --git a/src/core/src/any.cpp b/src/core/src/any.cpp index be1018dc8ea..38772c74d59 100644 --- a/src/core/src/any.cpp +++ b/src/core/src/any.cpp @@ -73,6 +73,19 @@ Any::~Any() { _impl = {}; } +Any::Any(const Any& other) { + *this = other; +}; + +Any& Any::operator=(const Any& other) { + if (other._temp) + _temp = other._temp->copy(); + if (other._impl) + _impl = other._impl->copy(); + _so = other._so; + return *this; +}; + Any::Any(const Any& other, const std::vector>& so) : _so{so}, _impl{other._impl} {} Any::Any(const char* str) : Any(std::string{str}) {} diff --git a/src/core/src/model.cpp b/src/core/src/model.cpp index d0e1d162db1..ff1c7e0cf90 100644 --- a/src/core/src/model.cpp +++ b/src/core/src/model.cpp @@ -495,21 +495,33 @@ int64_t ov::Model::get_result_index(const Output& value) const { return -1; } +bool ov::Model::evaluate(const HostTensorVector& output_tensors, const HostTensorVector& input_tensors) const { + ov::EvaluationContext evaluation_context; + OPENVINO_SUPPRESS_DEPRECATED_START + return evaluate(output_tensors, input_tensors, evaluation_context); + OPENVINO_SUPPRESS_DEPRECATED_END +} + bool ov::Model::evaluate(const HostTensorVector& output_tensors, const HostTensorVector& input_tensors, - EvaluationContext evaluation_context) const { + EvaluationContext& evaluation_context) const { OPENVINO_SUPPRESS_DEPRECATED_START auto outputs = ov::util::wrap_tensors(output_tensors); auto inputs = ov::util::wrap_tensors(input_tensors); - bool sts = evaluate(outputs, inputs, std::move(evaluation_context)); + bool sts = evaluate(outputs, inputs, evaluation_context); ov::util::update_output_host_tensors(output_tensors, outputs); OPENVINO_SUPPRESS_DEPRECATED_END return sts; } +bool ov::Model::evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors) const { + ov::EvaluationContext evaluation_context; + return evaluate(output_tensors, input_tensors, evaluation_context); +} + bool ov::Model::evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors, - ov::EvaluationContext evaluation_context) const { + ov::EvaluationContext& evaluation_context) const { evaluation_context.emplace("VariableContext", ov::op::util::VariableContext()); std::map value_map; for (size_t i = 0; i < m_parameters.size(); ++i) { diff --git a/src/core/tests/any.cpp b/src/core/tests/any.cpp index f66ae9720f0..97f902c319a 100644 --- a/src/core/tests/any.cpp +++ b/src/core/tests/any.cpp @@ -326,7 +326,7 @@ TEST_F(AnyTests, AnyDoesNotShareValues) { } } -TEST_F(AnyTests, DISABLED_AnyMapSharesValues) { +TEST_F(AnyTests, AnyMapSharesValues) { AnyMap map{ {"1", 1}, {"2", 2}, @@ -338,8 +338,31 @@ TEST_F(AnyTests, DISABLED_AnyMapSharesValues) { ASSERT_EQ(1, copy_map["1"].as()); ASSERT_EQ(2, copy_map["2"].as()); - map["1"].as() = 110; // change map - EXPECT_EQ(1, copy_map["1"].as()); // TODO: why value is changed here? + // change map + map["1"].as() = 110; + + // check copied state + EXPECT_EQ(110, map["1"].as()); + EXPECT_EQ(1, copy_map["1"].as()); +} + +TEST_F(AnyTests, AnyMapSharesComplexValues) { + const std::string string_props = "{map1:{subprop_map:{prop:value}},prop1:1,prop2:2.0}"; + ov::Any any(string_props); + ov::AnyMap map; + ASSERT_NO_THROW(map = any.as()); + + AnyMap copy_map = map; + + // check initial state + ASSERT_EQ(1, copy_map["prop1"].as()); + + // change map + map["prop1"].as() = "110"; + + // check original and copied state + EXPECT_EQ("110", map["prop1"].as()); + EXPECT_EQ("1", copy_map["prop1"].as()); } TEST_F(AnyTests, AnyNotEmpty) { diff --git a/src/inference/src/dev/core_impl.cpp b/src/inference/src/dev/core_impl.cpp index 86f404f2ee3..1d884f40cf7 100644 --- a/src/inference/src/dev/core_impl.cpp +++ b/src/inference/src/dev/core_impl.cpp @@ -65,15 +65,6 @@ bool is_virtual_device(const std::string& device_name) { device_name.find("HETERO") != std::string::npos || device_name.find("BATCH") != std::string::npos); }; -ov::AnyMap clone_map(const ov::AnyMap& m) { - ov::AnyMap rm; - for (auto&& kvp : m) { - rm[kvp.first] = kvp.second.is() ? ov::Any(clone_map(kvp.second.as())) : kvp.second; - } - - return rm; -} - /** * @brief Converts / flattens ov::device::properties from * @code @@ -94,7 +85,7 @@ ov::AnyMap clone_map(const ov::AnyMap& m) { * @return ov::AnyMap Flattened ov::AnyMap with properties */ ov::AnyMap flatten_sub_properties(const std::string& user_device_name, const ov::AnyMap& user_properties) { - ov::AnyMap result_properties = clone_map(user_properties); + ov::AnyMap result_properties = user_properties; // puts sub-property to result_properties if it's not there yet auto update_result_properties = [&result_properties](const ov::AnyMap& sub_properties) -> void { @@ -748,7 +739,7 @@ ov::AnyMap ov::CoreImpl::get_supported_property(const std::string& full_device_n // ov::device::priority cannot be shared, because it's specific for current virtual // plugin. So, we need to remove ov::device::priorities from the list, because it's // supposed to be set for current virtual plugin and cannot be propogated down - ov::AnyMap return_properties = clone_map(user_properties); + ov::AnyMap return_properties = user_properties; auto device_priorities_it = return_properties.find(ov::device::priorities.name()); if (device_priorities_it != return_properties.end()) { return_properties.erase(device_priorities_it); diff --git a/src/plugins/hetero/plugin.cpp b/src/plugins/hetero/plugin.cpp index d46cd823e62..fe75e760597 100644 --- a/src/plugins/hetero/plugin.cpp +++ b/src/plugins/hetero/plugin.cpp @@ -65,15 +65,6 @@ Configs any_copy(const ov::AnyMap& params) { return result; } -ov::AnyMap clone_map(const ov::AnyMap& m) { - ov::AnyMap rm; - for (auto&& kvp : m) { - rm[kvp.first] = kvp.second.is() ? ov::Any(clone_map(kvp.second.as())) : kvp.second; - } - - return rm; -} - } // namespace Engine::Engine() { @@ -83,7 +74,7 @@ Engine::Engine() { } ParsedConfig Engine::MergeConfigs(const ov::AnyMap& user_config) const { - auto device_config = clone_map(user_config); + auto device_config = user_config; auto hetero_config = _config; // after API 1.0 removal, replace with the loop over getHeteroSupportedConfigKeys() diff --git a/src/plugins/intel_gna/tests/deprecated/unit/CMakeLists.txt b/src/plugins/intel_gna/tests/deprecated/unit/CMakeLists.txt index deb0c0d98e9..3a4b3fc173c 100644 --- a/src/plugins/intel_gna/tests/deprecated/unit/CMakeLists.txt +++ b/src/plugins/intel_gna/tests/deprecated/unit/CMakeLists.txt @@ -66,9 +66,7 @@ target_include_directories(${TARGET_NAME} PRIVATE set_target_properties(${TARGET_NAME} PROPERTIES COMPILE_PDB_NAME ${TARGET_NAME}) # because IE unit tests use plugin and IE object files compiled with LTO -if(CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.0) - set_target_properties(${TARGET_NAME} PROPERTIES INTERPROCEDURAL_OPTIMIZATION_RELEASE ${ENABLE_LTO}) -endif() +set_target_properties(${TARGET_NAME} PROPERTIES INTERPROCEDURAL_OPTIMIZATION_RELEASE ${ENABLE_LTO}) ## Mock macros doesn't use "override" specificator target_compile_options(${TARGET_NAME} PRIVATE $<$: -Wno-error=inconsistent-missing-override >)