Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align friendly names uniqueization #22729

Merged
merged 10 commits into from
Mar 25, 2024
Merged

Align friendly names uniqueization #22729

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4f54cc5
Align friendly names uniqueization
itikhono Feb 8, 2024
fb50318
fix unit test
itikhono Feb 8, 2024
ed8c2c5
Merge branch 'master' into itikhono/align_friendly_names_uniqueization
itikhono Feb 8, 2024
0ac182a
Make friendly names unique in MOC
itikhono Feb 9, 2024
b389f75
Merge branch 'itikhono/align_friendly_names_uniqueization' of https:/…
itikhono Feb 9, 2024
f4a5652
Merge branch 'master' into itikhono/align_friendly_names_uniqueization
itikhono Feb 12, 2024
a2d3a11
Merge branch 'master' into itikhono/align_friendly_names_uniqueization
itikhono Feb 13, 2024
935f8a6
Merge branch 'master' into itikhono/align_friendly_names_uniqueization
jane-intel Feb 15, 2024
45dd77c
do not change not autogenerated friendly names from IR
itikhono Feb 15, 2024
1d8a3c3
Merge branch 'master' into itikhono/align_friendly_names_uniqueization
andrei-kochin Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/common/transformations/src/transformations/common_optimizations/moc_transformations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ bool ov::pass::MOCTransformations::run_on_model(const std::shared_ptr<ov::Model>
REGISTER_PASS(manager, AlignEltwiseInputRanks)
REGISTER_PASS(manager, SharedOpOptimization)
REGISTER_PASS(manager, ConstantFolding)
REGISTER_PASS(manager, ResolveNameCollisions)
manager.register_pass<ResolveNameCollisions>(true);
manager.run_passes(f);

if (!m_use_shapes) {
Expand Down
25 changes: 1 addition & 24 deletions src/core/src/pass/serialize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,28 +773,6 @@ std::string escape_delim(const std::string& name, const char delim = ',') {
return result_name;
}

std::string generate_unique_name(const std::unordered_set<std::string>& unique_names,
const std::string& base_name,
int suffix) {
std::string new_name = base_name + std::to_string(suffix);
if (unique_names.find(new_name) == unique_names.end()) {
return new_name;
} else {
suffix++;
return generate_unique_name(unique_names, base_name, suffix);
}
}

// TODO: remove when CNNNetwork will be supporting not-unique names
std::string get_node_unique_name(std::unordered_set<std::string>& unique_names, const ov::Node* n) {
std::string name = n->get_friendly_name();
if (unique_names.find(name) != unique_names.end()) {
name = generate_unique_name(unique_names, name, 0);
}
unique_names.insert(name);
return name;
}

void visit_exec_graph_node(pugi::xml_node& layer, const ov::Node* n) {
auto data = layer.child("data");
for (const auto& param : n->get_rt_info()) {
Expand Down Expand Up @@ -926,7 +904,6 @@ void ngfunction_2_ir(pugi::xml_node& netXml,
pugi::xml_node layers = netXml.append_child("layers");

const std::unordered_map<ov::Node*, int> layer_ids = create_layer_ids(model);
std::unordered_set<std::string> unique_names;

const bool exec_graph = is_exec_graph(model);

Expand Down Expand Up @@ -966,7 +943,7 @@ void ngfunction_2_ir(pugi::xml_node& netXml,
// If determinism is not required, include auto-generated names into xml
// layer name is not critical for hash computing
if (!deterministic) {
layer.append_attribute("name").set_value(get_node_unique_name(unique_names, node).c_str());
layer.append_attribute("name").set_value(node->get_friendly_name().c_str());
}
layer.append_attribute("type").set_value(translate_type_name(node_type_name).c_str());
if (!exec_graph) {
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/ir/src/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ std::string FrontEnd::get_name() const {

void FrontEnd::normalize(const std::shared_ptr<ov::Model>& model) const {
ov::pass::Manager manager;
manager.register_pass<pass::ResolveNameCollisions>();
manager.register_pass<pass::ResolveNameCollisions>(true);
manager.run_passes(model);
}

Expand Down
4 changes: 0 additions & 4 deletions src/frontends/ir/src/ir_deserializer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,17 +457,13 @@ std::shared_ptr<ov::Model> ov::XmlDeserializer::parse_function(const pugi::xml_n
std::map<size_t /*layer-id*/, NodeParams> params;

std::vector<size_t /*layer-id*/> outputs;
std::unordered_set<std::string> opName;

std::vector<size_t> order;
std::set<size_t> dfs_used_nodes;
std::map<size_t /*to-layer-id*/, std::vector<Edge>> edges;
// Read all layers and store their parameters in params map
FOREACH_CHILD (node, root.child("layers"), "layer") {
auto node_param = parse_generic_params(node);
if (opName.find(node_param.name) != opName.end() && node_param.type != "Result")
OPENVINO_THROW("Invalid IR! ", node_param.name, " name is not unique!");
opName.insert(node_param.name);
params[node_param.layerId] = {node, node_param};
if (node_param.type == "Result" || node_param.type == "Assign") {
outputs.push_back(node_param.layerId);
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/ir/tests/frontend_test_basic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ TEST_F(IRFrontendTests, name_is_not_unique) {
createTemporalModelFile(xmlModel, buffer);
std::shared_ptr<ov::Model> model;

ASSERT_THROW(core.read_model(xmlFileName, binFileName), ov::Exception);
EXPECT_NO_THROW(core.read_model(xmlFileName, binFileName));
}

TEST_F(IRFrontendTests, edge_has_wrong_port_id) {
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/onnx/frontend/src/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void FrontEnd::normalize(const std::shared_ptr<ov::Model>& model) const {
// Here, you can register transformations as a second step of importing process
// In particular, you can operate on not supported ops (it allows to N:N ONNX->OV mapping).
ov::pass::Manager manager;
manager.register_pass<pass::ResolveNameCollisions>();
manager.register_pass<pass::ResolveNameCollisions>(true);
manager.run_passes(model);
}

Expand Down
2 changes: 1 addition & 1 deletion src/frontends/paddle/src/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ void FrontEnd::add_extension(const std::shared_ptr<ov::Extension>& extension) {

void FrontEnd::normalize(const std::shared_ptr<ov::Model>& model) const {
ov::pass::Manager manager;
manager.register_pass<ov::pass::ResolveNameCollisions>();
manager.register_pass<ov::pass::ResolveNameCollisions>(true);
manager.run_passes(model);
}

Expand Down
2 changes: 1 addition & 1 deletion src/frontends/tensorflow/src/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void FrontEnd::normalize(const std::shared_ptr<ov::Model>& model) const {
manager.register_pass<ov::pass::RemoveConcatZeroDimInput>();
manager.register_pass<ov::pass::TransposeSinkingGeneral>();
manager.register_pass<ov::pass::ReverseShapeAndTypeInfer>();
manager.register_pass<ov::pass::ResolveNameCollisions>();
manager.register_pass<ov::pass::ResolveNameCollisions>(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please clarify, why do we need to resolve uniqueness for non-autogenerated names. TF should provide unique names for nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TF Frontend can create additional friendly names during conversion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it creates additional friendly names, it means they are auto-generated. For auto-generated names, this transformation already works. Please correct me if I am wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no guarantee this won't create duplicates if someone adds code manually setting friendly names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe somehow it's possible to create duplicates in TF, some workaround or trick

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can some test be added to simulate the "trick" that name will be duplicated?

manager.run_passes(model);
}

Expand Down
2 changes: 1 addition & 1 deletion src/frontends/tensorflow_lite/src/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ void FrontEnd::normalize(const std::shared_ptr<ov::Model>& function) const {
manager.register_pass<ov::frontend::tensorflow_lite::pass::Rfft2dSimplifier>();
manager.register_pass<ov::pass::TransposeSinking>();
manager.register_pass<ov::pass::TransposeSinkingGeneral>();
manager.register_pass<ov::pass::ResolveNameCollisions>();
manager.register_pass<ov::pass::ResolveNameCollisions>(true);
manager.run_passes(function);
}

Expand Down
Loading