From 48165087bf18b072ee1dac011b3b2418d383ee3e Mon Sep 17 00:00:00 2001 From: yanlan song Date: Mon, 29 Jan 2024 09:29:21 +0800 Subject: [PATCH] [core]/fix hash inconsistency (#22208) * fix the hash inconsistency Signed-off-by: fishbell * format Signed-off-by: fishbell * apply review comments Signed-off-by: fishbell * remove unncess lock Signed-off-by: fishbell * remove commented code Signed-off-by: fishbell --------- Signed-off-by: fishbell Co-authored-by: Chen Peter --- .../low_latency_v2_test.cpp | 5 ++-- .../tests/resolve_names_collisions.cpp | 1 + src/core/include/openvino/core/node.hpp | 1 + src/core/src/node.cpp | 7 ++++- src/core/src/pass/serialize.cpp | 10 +++---- .../tests/unit/compilation_context_test.cpp | 28 +++++++++++++++++++ 6 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/common/transformations/tests/common_optimizations/low_latency_v2_test.cpp b/src/common/transformations/tests/common_optimizations/low_latency_v2_test.cpp index 702b4cbdfeeb5b..f56c34227a8be4 100644 --- a/src/common/transformations/tests/common_optimizations/low_latency_v2_test.cpp +++ b/src/common/transformations/tests/common_optimizations/low_latency_v2_test.cpp @@ -910,6 +910,7 @@ TEST_P(LLT2Sequence, RNNLowLatency_v2) { auto H = make_shared(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size}); auto C = make_shared(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size}); auto outputs = create_sequence(p.rnn_type, attrs, X, H, C); + outputs[0].get_node()->set_friendly_name("lstm_node"); ParameterVector params{X, H}; if (p.rnn_type == RNNType::LSTM) { params.push_back(C); @@ -930,9 +931,9 @@ TEST_P(LLT2Sequence, RNNLowLatency_v2) { auto H = make_shared(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size}); auto C = make_shared(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size}); auto variable_h = make_shared( - ov::op::util::VariableInfo{H->get_shape(), H->get_element_type(), "node_28/variable_0"}); + ov::op::util::VariableInfo{H->get_shape(), H->get_element_type(), "lstm_node/variable_0"}); auto variable_c = make_shared( - ov::op::util::VariableInfo{C->get_shape(), C->get_element_type(), "node_28/variable_1"}); + ov::op::util::VariableInfo{C->get_shape(), C->get_element_type(), "lstm_node/variable_1"}); auto read_val_H = create_read_value(H, variable_h); auto read_val_C = create_read_value(C, variable_c); diff --git a/src/common/transformations/tests/resolve_names_collisions.cpp b/src/common/transformations/tests/resolve_names_collisions.cpp index a67ce5ba44ea3d..a4986e51119800 100644 --- a/src/common/transformations/tests/resolve_names_collisions.cpp +++ b/src/common/transformations/tests/resolve_names_collisions.cpp @@ -19,6 +19,7 @@ TEST(ResolveNameCollisionsTest, FixGeneratedNames) { EXPECT_NE(std::string::npos, gen_friendly_name.find("Parameter_")); unsigned long long index = std::stoull(gen_friendly_name.substr(name.length())); name += std::to_string(++index); + name += "_autogenerated"; arg0->set_friendly_name(name); diff --git a/src/core/include/openvino/core/node.hpp b/src/core/include/openvino/core/node.hpp index ff2190ae9ab59a..393dc2be32d226 100644 --- a/src/core/include/openvino/core/node.hpp +++ b/src/core/include/openvino/core/node.hpp @@ -434,6 +434,7 @@ class OPENVINO_API Node : public std::enable_shared_from_this { std::vector> m_control_dependencies; size_t m_instance_id{m_next_instance_id.fetch_add(1)}; std::string m_friendly_name; + mutable std::string m_auto_generated_friendly_name; mutable std::string m_unique_name; mutable std::atomic_bool m_name_changing{false}; static std::atomic m_next_instance_id; diff --git a/src/core/src/node.cpp b/src/core/src/node.cpp index 66a026cf6e396c..054955e241748e 100644 --- a/src/core/src/node.cpp +++ b/src/core/src/node.cpp @@ -282,8 +282,13 @@ std::string ov::Node::description() const { } const std::string& ov::Node::get_friendly_name() const { + const auto& name = get_name(); + AtomicGuard lock(m_name_changing); if (m_friendly_name.empty()) { - return get_name(); + if (m_auto_generated_friendly_name.empty()) { + m_auto_generated_friendly_name = name + "_" + "autogenerated"; + } + return m_auto_generated_friendly_name; } return m_friendly_name; } diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 96b400cb813f95..0ed69cad73dfa6 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -770,9 +770,8 @@ std::string generate_unique_name(const std::unordered_set& unique_n } } -template -bool is_name_auto_generated(const T& n) { - return n.get_friendly_name() == n.get_name(); +bool is_name_auto_generated(const ov::Node& n) { + return n.get_friendly_name().find("autogenerated") != std::string::npos; } // TODO: remove when CNNNetwork will be supporting not-unique names @@ -908,8 +907,9 @@ void ngfunction_2_ir(pugi::xml_node& netXml, ConstantWriter& constant_node_write_handler, int64_t version, bool deterministic) { - // If determinism is not required, include auto-generated names into xml - if (!deterministic || !is_name_auto_generated(model)) { + // If determinism is not required, do not include names into xml + // model name is not critial for hash computing + if (!deterministic) { netXml.append_attribute("name").set_value(model.get_friendly_name().c_str()); } netXml.append_attribute("version").set_value(static_cast(version)); diff --git a/src/inference/tests/unit/compilation_context_test.cpp b/src/inference/tests/unit/compilation_context_test.cpp index 2070f2302e29ef..d8f8bbf953d975 100644 --- a/src/inference/tests/unit/compilation_context_test.cpp +++ b/src/inference/tests/unit/compilation_context_test.cpp @@ -14,6 +14,7 @@ #include "common_test_utils/common_utils.hpp" #include "common_test_utils/test_constants.hpp" #include "cpp/ie_cnn_network.h" +#include "openvino/core/preprocess/pre_post_process.hpp" #include "openvino/op/add.hpp" #include "openvino/op/constant.hpp" #include "openvino/op/multiply.hpp" @@ -357,3 +358,30 @@ TEST(NetworkContext_ModelName, HashOfExistingFile) { ASSERT_EQ(ModelCache::compute_hash(file1, {{"key", "value"}}), ModelCache::compute_hash(file2, {{"key", "value"}})); } + +TEST(NetworkContext, HashOfSameModelWithClone) { + auto model1 = create_simple_model(); + // test model with friendly name + model1->set_friendly_name("test model"); + auto model1_clone = model1->clone(); + ASSERT_EQ(ModelCache::compute_hash(model1, {}), ModelCache::compute_hash(model1_clone, {})); + auto model2 = create_simple_model(); // model without friendly name + auto preproc = ov::preprocess::PrePostProcessor(model2); + const auto output_precision = ov::element::f16; + // SET INPUT PRECISION + const auto& inputs = model2->inputs(); + for (size_t i = 0; i < inputs.size(); i++) { + const auto& item = inputs[i]; + auto& in = preproc.input(item.get_any_name()); + in.tensor().set_element_type(output_precision); + } + // SET OUTPUT PRECISION + const auto& outs = model2->outputs(); + for (size_t i = 0; i < outs.size(); i++) { + preproc.output(i).tensor().set_element_type(output_precision); + } + model2 = preproc.build(); + + auto model2_clone = model2->clone(); + ASSERT_EQ(ModelCache::compute_hash(model2, {}), ModelCache::compute_hash(model2_clone, {})); +} \ No newline at end of file