Improve logging of the matching process #28587

CuriousPanCake · 2025-01-21T13:35:46Z

Improve logging of the matching process to make more human-friendly

Logging before:

Several examples of the improved logging:

Signed-off-by: Andrii Staikov [email protected]

Ticket:

CVS-151312

praasz · 2025-02-20T12:08:49Z

src/common/util/include/openvino/util/env_util.hpp

+/// \param str String to split
+/// \param delimiter Delimiter to use for splitting
+/// \return Returns an unordered set of split strings
+std::unordered_set<std::string> split_by_delimiter(std::string str, char delimiter);


Could you look at ov::util::join and do opposite to return vector of strings which are split using delimiter?
Can be this function be in same place as join?

praasz · 2025-02-20T12:10:45Z

src/core/dev_api/openvino/core/log_util.hpp

+    std::string level_str;
+};
+
+OPENVINO_API std::string node_version_type_str(const std::shared_ptr<ov::Node>& node);


Suggested change

OPENVINO_API std::string node_version_type_str(const std::shared_ptr<ov::Node>& node);

OPENVINO_API std::string node_version_type_str(const ov::Node& node);

why pass shared pointer not just node do need do something with ownership in this function?

Agreed, fixed

src/common/transformations/include/transformations/utils/gen_pattern.hpp

src/core/src/log_util.cpp

praasz · 2025-02-20T12:23:03Z

src/core/src/log_util.cpp

+    for (const auto& arg : node->input_values()) {
+        if (verbose)
+            stream << sep << arg;
+        else
+            stream << sep << arg.get_node_shared_ptr()->get_type_name();
+        sep = ", ";
+    }


Suggested change

for (const auto& arg : node->input_values()) {

if (verbose)

stream << sep << arg;

else

stream << sep << arg.get_node_shared_ptr()->get_type_name();

sep = ", ";

}

if (verbose){

verbose_loop()

} else{

not_werbose_loop()

}

looks like util:join here

Moved arguments printing to a designated function.

It looks like not.

suggestion

if (const auto& args = node->input_values(); verbose){ stream << ov::util::join(args, ", "); } else { //custom join for (const auto& arg : args){ ... } }

src/common/util/include/openvino/util/log.hpp

src/core/dev_api/openvino/core/log_util.hpp

src/common/util/src/env_util.cpp

src/core/dev_api/openvino/core/log_util.hpp

praasz · 2025-02-21T06:17:03Z

src/common/transformations/include/transformations/utils/gen_pattern.hpp

            _VERBOSE_LOG(level, "X OP type mismatch: ", m_signature, " vs ", graph_value);
+            OPENVINO_LOG_GENPATTERN2(matcher, pattern_value, graph_value);


Just topic for future improvements, maybe is possible to merge these two log macros into one?

src/core/src/log_util.cpp

praasz · 2025-02-21T06:25:16Z

src/core/src/log_util.cpp

+namespace ov {
+namespace util {


Suggested change

namespace ov {

namespace util {

namespace ov::util {

Detail

praasz · 2025-02-21T06:27:11Z

src/core/dev_api/openvino/core/log_util.hpp

+#include "openvino/core/core_visibility.hpp"
+#include "openvino/core/node.hpp"
+
+#ifdef ENABLE_OPENVINO_DEBUG


Move it above includes to not add code if file is will be empty.

praasz · 2025-02-21T06:27:39Z

src/core/dev_api/openvino/core/log_util.hpp

+namespace ov {
+namespace util {


Suggested change

namespace ov {

namespace util {

namespace ov::util {

praasz · 2025-02-21T06:28:45Z

src/tests/test_utils/common_test_utils/tests/utils_tests.cpp

@@ -51,3 +52,53 @@ TEST(UtilsTests, filter_lines_by_prefix) {
    res = filter_lines_by_prefix(lines, "ab");
    ASSERT_EQ(res, "");
 }
+
+TEST(UtilsTests, split_by_delimiter) {
+    const auto line = "EliminateSplitConcat,MarkDequantization,StatefulSDPAFusion";


Add test string "name,name,name"
Will it return three entries?

praasz · 2025-02-21T06:32:36Z

src/common/util/include/openvino/util/env_util.hpp

+/// \param str String to split
+/// \param delimiter Delimiter to use for splitting
+/// \return Returns an unordered set of split strings
+std::unordered_set<std::string> split_by_delimiter(const std::string& str, char delimiter);


Move it to "common_util.hpp"

Suggested change

std::unordered_set<std::string> split_by_delimiter(const std::string& str, char delimiter);

std::vector<std::string> split(const std::string& str, const std::string& delimiter = ",");

Return value can remove some split strings, it can be always created from vector.
And use string delimiter as it can provide more complex option for split the text

praasz · 2025-02-21T06:45:55Z

src/core/src/log_util.cpp

+    for (const auto& arg : node->input_values()) {
+        if (verbose)
+            stream << sep << arg;
+        else
+            stream << sep << arg.get_node_shared_ptr()->get_type_name();
+        sep = ", ";
+    }


It looks like not.

suggestion

if (const auto& args = node->input_values(); verbose){ stream << ov::util::join(args, ", "); } else { //custom join for (const auto& arg : args){ ... } }

praasz · 2025-02-21T06:48:24Z

src/core/src/log_util.cpp

+            stream << sep << arg;
+        else
+            stream << sep << arg.get_node_shared_ptr()->get_type_name();
+        sep = ", ";


Why it has to be set every iteration, check ov::util::join how separator is added.

Co-authored-by: Pawel Raasz <[email protected]>

itikhono · 2025-02-21T07:14:45Z

we decided to merge this PR and address the remaining comments within CVS-162907 ticket.

CuriousPanCake added 6 commits January 8, 2025 11:14

wip

37532da

some git fix

c11593f

wip2

a25fa61

wip3

21daf79

wip4: very working prototype

2b6ebe6

working version

2e050e6

github-actions bot added category: Core OpenVINO Core (aka ngraph) category: transformations OpenVINO Runtime library - Transformations category: CPP API OpenVINO CPP API bindings labels Jan 21, 2025

CuriousPanCake added 5 commits January 21, 2025 14:38

fix namespace

0f08cbc

clear code

d3c03db

more cleaning

738578b

more clean up

7331229

Merge branch 'master' into CVS-151312_exp

77e1c75

CuriousPanCake marked this pull request as ready for review January 21, 2025 14:14

CuriousPanCake requested review from a team as code owners January 21, 2025 14:14

CuriousPanCake requested review from itikhono and removed request for a team January 21, 2025 14:14

CuriousPanCake added 7 commits January 21, 2025 15:16

cleaning

dd54af5

supported GenericPattern, many other small improvements

1fec8fc

fix exception throwing

4029bf9

add verbose option

14f34d9

added switching on/off using env variable

1b660bf

added colors

ec2d547

almost done

b4de07d

CuriousPanCake requested review from a team as code owners February 13, 2025 09:36

CuriousPanCake removed the request for review from a team February 13, 2025 09:36

praasz reviewed Feb 20, 2025

View reviewed changes

itikhono reviewed Feb 20, 2025

View reviewed changes

src/core/dev_api/openvino/core/log_util.hpp Outdated Show resolved Hide resolved

fixed multiple review concerns

614958d

CuriousPanCake requested review from a team as code owners February 20, 2025 15:08

add images

169262f

github-actions bot added the category: IE Tests OpenVINO Test: plugins and common label Feb 20, 2025

CuriousPanCake requested review from itikhono and praasz and removed request for itikhono February 20, 2025 15:10

remove images

310f6d7

CuriousPanCake requested a review from itikhono February 20, 2025 15:17

CuriousPanCake added 2 commits February 20, 2025 16:18

clear code

164be9b

code style

688ae40

itikhono approved these changes Feb 20, 2025

View reviewed changes

praasz reviewed Feb 21, 2025

View reviewed changes

src/core/src/log_util.cpp Outdated Show resolved Hide resolved

praasz reviewed Feb 21, 2025

View reviewed changes

Using the friendly name instead of the autogenerated

4e55038

Co-authored-by: Pawel Raasz <[email protected]>

itikhono enabled auto-merge February 21, 2025 07:20

itikhono added this pull request to the merge queue Feb 21, 2025

p-durandin removed this pull request from the merge queue due to a manual request Feb 21, 2025

praasz approved these changes Feb 21, 2025

View reviewed changes

Merge branch 'master' into CVS-151312_exp

2c525f3

itikhono enabled auto-merge February 21, 2025 12:08

itikhono added this pull request to the merge queue Feb 21, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 21, 2025

Merge branch 'master' into CVS-151312_exp

695db12

dorloff enabled auto-merge February 21, 2025 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve logging of the matching process #28587

Improve logging of the matching process #28587

CuriousPanCake commented Jan 21, 2025 •

edited

Loading

praasz Feb 20, 2025

praasz Feb 20, 2025

CuriousPanCake Feb 20, 2025

praasz Feb 20, 2025

CuriousPanCake Feb 20, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

praasz Feb 21, 2025

itikhono commented Feb 21, 2025 •

edited

Loading

	OPENVINO_API std::string node_version_type_str(const std::shared_ptr<ov::Node>& node);
	OPENVINO_API std::string node_version_type_str(const ov::Node& node);

		_VERBOSE_LOG(level, "X OP type mismatch: ", m_signature, " vs ", graph_value);
		OPENVINO_LOG_GENPATTERN2(matcher, pattern_value, graph_value);

	std::unordered_set<std::string> split_by_delimiter(const std::string& str, char delimiter);
	std::vector<std::string> split(const std::string& str, const std::string& delimiter = ",");

Improve logging of the matching process #28587

Are you sure you want to change the base?

Improve logging of the matching process #28587

Conversation

CuriousPanCake commented Jan 21, 2025 • edited Loading

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

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

itikhono commented Feb 21, 2025 • edited Loading

CuriousPanCake commented Jan 21, 2025 •

edited

Loading

itikhono commented Feb 21, 2025 •

edited

Loading