add support for model outputs without names #2850

dtrawins · 2024-11-18T16:40:04Z

🛠 Summary

CVS-159002
When models have outputs without assigned names, add the name in the model initialization. The following format will be applied OUT_

🧪 Checklist

Unit tests added.
The documentation updated.
Change follows security best practices.
``

mzegla · 2024-12-18T09:53:22Z

src/modelinstance.cpp

        try {
            OV_LOGGER("ov::Output<ov::Node> output: {}, output.get_any_name()", reinterpret_cast<const void*>(&output));
+            if (output.get_names().size() == 0) {
+                std::unordered_set<std::string> dummy_name{"OUT_" + std::to_string(outputIndex)};


Shouldn't we use lowercase? Isn't it a standard in input/output naming?

mzegla · 2024-12-18T09:57:24Z

tests/models/no_name_output.py

+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+


Is this script meant to be used only manually by developers?

this is a reference how the test model got created in case some changes should be applied.

mzegla · 2024-12-18T09:58:53Z

tests/models/no_name_output.py

+
+input_data = np.ones((1, 10),dtype=np.int8)*10
+results = compiled_model({"INPUT1": input_data, "INPUT2": input_data})
+


Maybe we could have some assertion here instead of prints?
I suppose the goal of this script is to generate a model and check if it's valid. In such case checking if output looks as expected would be good.

atobiszei · 2024-12-18T10:54:26Z

src/modelinstance.cpp

        try {
            OV_LOGGER("ov::Output<ov::Node> output: {}, output.get_any_name()", reinterpret_cast<const void*>(&output));
+            if (output.get_names().size() == 0) {
+                std::unordered_set<std::string> dummy_name{"OUT_" + std::to_string(outputIndex)};
+                output.add_names(dummy_name);


This is not logic related to "applyLayoutConfiguration". Move those changes outside of this method.

atobiszei · 2024-12-18T10:55:40Z

tests/models/no_name_output.py

@@ -0,0 +1,55 @@
+#
+# Copyright (c) 2020 Intel Corporation


Change year

mzegla

How does it look like for inputs without names?

mzegla · 2025-01-13T16:04:07Z

src/test/test_utils.hpp

@@ -79,6 +79,7 @@ const std::string passthrough_string_model_location = getGenericFullPathForSrcTe
 const std::string dummy_saved_model_location = getGenericFullPathForSrcTest(std::filesystem::current_path().u8string() + "/src/test/dummy_saved_model", false);
 const std::string dummy_tflite_location = getGenericFullPathForSrcTest(std::filesystem::current_path().u8string() + "/src/test/dummy_tflite", false);
 const std::string scalar_model_location = getGenericFullPathForSrcTest(std::filesystem::current_path().u8string() + "/src/test/scalar", false);
+const std::string no_name_output_model_location = std::filesystem::current_path().u8string() + "/src/test/no_name_output";


This will likely not work on windows, see usage of getGenericFullPathForSrcTest above

the tests on windows are passing

That's strange. This new variable seems to have the same purpose as all the above and yet it works without wrapping in getGenericFullPathForSrcTest function? @rasapala can you take a look? Because to me, they all should be constructed in the same manner. Either with or without wrapper function.

mzegla · 2025-01-13T16:05:08Z

src/test/no_name_output/1/model.xml

+		<layer id="1" name="input1" type="Parameter" version="opset1">
+			<data shape="1,10" element_type="i8" />
+			<output>
+				<port id="0" precision="I8" names="input1">
+					<dim>1</dim>
+					<dim>10</dim>
+				</port>
+			</output>
+		</layer>
+		<layer id="0" name="input2" type="Parameter" version="opset1">


Can we use name "input_1" and "input_2" to follow the convention?

yes, but this is barely a test model ;-)

mzegla · 2025-01-13T16:05:47Z

src/modelinstance.cpp

@@ -305,7 +306,7 @@ static Status applyLayoutConfiguration(const ModelConfig& config, std::shared_pt
    }

    OV_LOGGER("ov::Model: {}, model->outputs()", reinterpret_cast<void*>(model.get()));
-    for (const ov::Output<ov::Node>& output : model->outputs()) {
+    for (ov::Output<ov::Node>& output : model->outputs()) {


is this change required?

yes, we are changing the outputs by adding name

but not in this for loop, i dont think we need to remove const in this for loop

atobiszei · 2025-01-15T09:31:00Z

src/modelinstance.cpp

@@ -413,6 +414,29 @@ Status ModelInstance::loadTensors(const ModelConfig& config, bool needsToApplyLa
        SPDLOG_LOGGER_ERROR(modelmanager_logger, "Error during configuration validation against model");
        return status;
    }
+    // add output names if not present in the model


Remove comment and make function
adjustForEmptyOutputNames/addOutputNamesIfNotPresent or sth similar to operate on similar level of abstraction inthis function - see loadInputTensors/applyLayoutConfiguration etc

add supprt for model outputs without names

0c8fc87

dtrawins requested review from atobiszei and dkalinowski November 18, 2024 16:40

dtrawins and others added 9 commits November 18, 2024 17:56

style

d62b6d9

style

69c201c

style

ceaf202

test dummy model

90f5ca8

fix

5fed196

unit tests for no_name model

19c8928

fix unit tests and merge

aea68a9

style

5a84efa

Merge branch 'main' into no-ouput-name

5b9a3df

dtrawins requested a review from mzegla December 18, 2024 09:15

mzegla reviewed Dec 18, 2024

View reviewed changes

atobiszei reviewed Dec 18, 2024

View reviewed changes

Dariusz Trawinski and others added 5 commits January 12, 2025 01:51

review changes

2dae97c

Merge branch 'main' into no-ouput-name

173efc8

fix

698c871

fix

4f02072

header

028a2e4

dtrawins requested review from atobiszei and mzegla January 13, 2025 11:03

mzegla reviewed Jan 13, 2025

View reviewed changes

atobiszei reviewed Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for model outputs without names #2850

add support for model outputs without names #2850

dtrawins commented Nov 18, 2024 •

edited

Loading

mzegla Dec 18, 2024

mzegla Dec 18, 2024

dtrawins Dec 19, 2024

mzegla Dec 18, 2024

atobiszei Dec 18, 2024

atobiszei Dec 18, 2024

mzegla left a comment

mzegla Jan 13, 2025

dtrawins Jan 15, 2025

mzegla Jan 15, 2025

mzegla Jan 13, 2025

dtrawins Jan 15, 2025

mzegla Jan 13, 2025

dtrawins Jan 15, 2025

dkalinowski Jan 15, 2025

atobiszei Jan 15, 2025


		input_data = np.ones((1, 10),dtype=np.int8)*10
		results = compiled_model({"INPUT1": input_data, "INPUT2": input_data})

add support for model outputs without names #2850

Are you sure you want to change the base?

add support for model outputs without names #2850

Conversation

dtrawins commented Nov 18, 2024 • edited Loading

🛠 Summary

🧪 Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzegla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtrawins commented Nov 18, 2024 •

edited

Loading