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.

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

[RtInfo/IR] Serialize non RuntimeAttribute rt_info entries of Node and Tensor #27358

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

t-jankowski
Copy link
Contributor

@t-jankowski t-jankowski commented Oct 31, 2024

Details:

  • Adds into pass::Serialize and IR FE support for non RuntimeAttribute entries from rt_info map of ov::Node and ov::Tensor

Tickets:

Signed-off-by: Tomasz Jankowski <[email protected]>
Signed-off-by: Tomasz Jankowski <[email protected]>
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: IR FE OpenVINO IR v10 / v11 FrontEnd labels Oct 31, 2024
@github-actions github-actions bot added the category: MO Model Optimizer label Nov 4, 2024
@t-jankowski t-jankowski marked this pull request as ready for review November 8, 2024 11:46
@t-jankowski t-jankowski requested review from a team as code owners November 8, 2024 11:46
@t-jankowski t-jankowski requested review from itikhono, olpipi, slyalin and praasz and removed request for a team November 8, 2024 11:46
@itikhono
Copy link
Contributor

itikhono commented Nov 8, 2024

What is the purpose of implementing this solution?
What task are we solving, for which models?

@t-jankowski
Copy link
Contributor Author

What is the purpose of implementing this solution? What task are we solving, for which models?

It's related to CVS-105807 and CVS-105845.

@t-jankowski t-jankowski requested a review from a team as a code owner November 19, 2024 14:06
@github-actions github-actions bot added the category: tools OpenVINO C++ / Python tools label Nov 19, 2024
@ilya-lavrenov
Copy link
Contributor

will it affect model caching?
E.g. more attributes affects hash of serialized representation

@t-jankowski
Copy link
Contributor Author

will it affect model caching? E.g. more attributes affects hash of serialized representation

Yes, current solution in this PR affects pass::Hash. These new entries would undergo deterministic mode to not affect hash.

Other case to consider is that there is no black list (rt_map entries not to be serialized), so currently all are serialized.

@ilya-lavrenov @slyalin please advise.

@t-jankowski t-jankowski requested a review from praasz November 29, 2024 10:43
@slyalin
Copy link
Contributor

slyalin commented Dec 6, 2024

@t-jankowski , what kind of advice you need? The purpose of this PR as I understood it, is to provide a reliable tool for developers who are crafting and annotating models in run-time. For those people, the reasonable debug flow is handy -- if you serialize such a model you want to see what you have done with the graph, including rt_info. And then you should be able to read such IR back into run-time without losing the information. Otherwise, this serialize/deserialize flow is broken. The requirement to build serializable attributes with RuntimeAttribute brakes the flow for such external developers without providing any extra value -- it didn't work for Python for example. The purpose of this PR is not to expose more rt_info attributes automatically -- all kind of "regularly" created rt_info should be under the full control and should be analyzed and understood in respect of affecting everything else including hashing etc. It is mostly related to p.p 3-7 from CVS-105807 in terms of improvements. If the current solution in this PR breaks our expectations for hashing (and I don't understand how) or have other impacts then it should be improved.

@mlukasze mlukasze enabled auto-merge December 16, 2024 05:43
Copy link
Contributor

github-actions bot commented Jan 4, 2025

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Jan 4, 2025
@t-jankowski t-jankowski removed the Stale label Jan 7, 2025
@mlukasze mlukasze added the no_stale Do not mark as stale label Jan 10, 2025
@github-actions github-actions bot removed the category: tools OpenVINO C++ / Python tools label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph) category: IR FE OpenVINO IR v10 / v11 FrontEnd category: MO Model Optimizer no_stale Do not mark as stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants