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

Delete check that ReadValue/Assign operations have to be in pair in ov::Model #23051

Conversation

itikhono
Copy link
Contributor

Details:

If ReadValue and Assign are not paired in model topology we have "Model is incorrect. Assign and ReadValue operations must be in pairs on the network."

Check for ReadValue-Assign pairs is useless and it is an obstacle for model transformation debugging. Moreover, with more advanced techniques of state modification there can be other ops (custom ones) that modify state in place.

Tickets:

@itikhono itikhono requested a review from a team as a code owner February 23, 2024 14:02
@itikhono itikhono requested a review from jane-intel February 23, 2024 14:02
@github-actions github-actions bot added the category: Core OpenVINO Core (aka ngraph) label Feb 23, 2024
@itikhono itikhono requested a review from slyalin February 23, 2024 14:15
@itikhono itikhono added this to the 2024.1 milestone Feb 23, 2024
@@ -220,11 +220,6 @@ void ov::Model::prerequirements(bool detect_variables, bool detect_parameters) {
void ov::Model::validate_nodes_and_infer_types() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to add a test with a graph when this check is not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, we need to double check if we have these tests or not but not in this PR

@itikhono itikhono added this pull request to the merge queue Mar 5, 2024
Merged via the queue into openvinotoolkit:master with commit b6b4bda Mar 5, 2024
107 checks passed
@itikhono itikhono deleted the itikhono/memory/delete_assign_readvalue_check branch March 5, 2024 16:47
Pranshu-S pushed a commit to Pranshu-S/openvino that referenced this pull request Mar 7, 2024
…v::Model (openvinotoolkit#23051)

### Details:
If ReadValue and Assign are not paired in model topology we have "Model
is incorrect. Assign and ReadValue operations must be in pairs on the
network."

Check for ReadValue-Assign pairs is useless and it is an obstacle for
model transformation debugging. Moreover, with more advanced techniques
of state modification there can be other ops (custom ones) that modify
state in place.

### Tickets:
 - *CVS-133159*
alvoron pushed a commit to alvoron/openvino that referenced this pull request Apr 29, 2024
…v::Model (openvinotoolkit#23051)

### Details:
If ReadValue and Assign are not paired in model topology we have "Model
is incorrect. Assign and ReadValue operations must be in pairs on the
network."

Check for ReadValue-Assign pairs is useless and it is an obstacle for
model transformation debugging. Moreover, with more advanced techniques
of state modification there can be other ops (custom ones) that modify
state in place.

### Tickets:
 - *CVS-133159*
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants