Skip to content

Commit

Permalink
[core]/fix hash inconsistency (#22208)
Browse files Browse the repository at this point in the history
* fix the hash inconsistency

Signed-off-by: fishbell <[email protected]>

* format

Signed-off-by: fishbell <[email protected]>

* apply review comments

Signed-off-by: fishbell <[email protected]>

* remove unncess lock

Signed-off-by: fishbell <[email protected]>

* remove commented code

Signed-off-by: fishbell <[email protected]>

---------

Signed-off-by: fishbell <[email protected]>
Co-authored-by: Chen Peter <[email protected]>
  • Loading branch information
songbell and peterchen-intel authored Jan 29, 2024
1 parent a16bc97 commit 4816508
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,7 @@ TEST_P(LLT2Sequence, RNNLowLatency_v2) {
auto H = make_shared<Parameter>(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size});
auto C = make_shared<Parameter>(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);
Expand All @@ -930,9 +931,9 @@ TEST_P(LLT2Sequence, RNNLowLatency_v2) {
auto H = make_shared<Parameter>(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size});
auto C = make_shared<Parameter>(element::f32, Shape{attrs.batch, attrs.num_dir, attrs.hidden_size});
auto variable_h = make_shared<ov::op::util::Variable>(
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::Variable>(
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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions src/core/include/openvino/core/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ class OPENVINO_API Node : public std::enable_shared_from_this<Node> {
std::vector<std::shared_ptr<Node>> 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<size_t> m_next_instance_id;
Expand Down
7 changes: 6 additions & 1 deletion src/core/src/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 5 additions & 5 deletions src/core/src/pass/serialize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -770,9 +770,8 @@ std::string generate_unique_name(const std::unordered_set<std::string>& unique_n
}
}

template <typename T>
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
Expand Down Expand Up @@ -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<long long>(version));
Expand Down
28 changes: 28 additions & 0 deletions src/inference/tests/unit/compilation_context_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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, {}));
}

0 comments on commit 4816508

Please sign in to comment.