Fix failing case where input onnx model is used with shared context e… #776
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a bug in the OpenVINO execution provider where ONNX source models fail when compiled with shared context enabled. The issue was identified in Intel's tracking system and involves incorrect handling of EP context models during compilation.
- Added logic to detect EP context models using
CheckForOVEPCtxNodeInGraph() - Modified the conditional check to use
is_epctx_modelinstead of!session_context_.so_context_enable - Added explanatory comments about the OVIR wrapped in epctx corner case
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| graph_body_viewer_0.DomainToVersionMap().at(kOnnxDomain); | ||
|
|
||
| // OVIR wrapped in epctx should be treated as source but this code does not | ||
| // This corner case is not in use and will be addressed in a future commit |
There was a problem hiding this comment.
The comment is incomplete and unclear. It should explain what 'OVIR' stands for and provide a complete explanation of the issue and planned resolution.
Suggested change
| // This corner case is not in use and will be addressed in a future commit | |
| // OVIR (OpenVINO Intermediate Representation) models that are wrapped within an EP (Execution Provider) context (epctx) | |
| // should be treated as source models. However, the current implementation does not handle this case correctly. | |
| // This is a known corner case that is not currently encountered in practice, but it will be properly addressed in a future commit. |
| graph_body_viewer_0.DomainToVersionMap().at(kOnnxDomain); | ||
|
|
||
| // OVIR wrapped in epctx should be treated as source but this code does not | ||
| // This corner case is not in use and will be addressed in a future commit |
There was a problem hiding this comment.
[nitpick] Consider adding a TODO comment or tracking issue reference to make this technical debt more actionable and trackable.
Suggested change
| // This corner case is not in use and will be addressed in a future commit | |
| // TODO: Handle the corner case where OVIR is wrapped in epctx as source. This is not currently in use. Consider tracking in a GitHub issue. |
…r.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
eaaade6 to
1f3aa94
Compare
vthaniel
approved these changes
Aug 13, 2025
vthaniel
left a comment
There was a problem hiding this comment.
Tested the EP Context scenarios on the perf_test app available on ovep-develop
and no issues were found
javier-intel
added a commit
that referenced
this pull request
Aug 14, 2025
#776) * Fix failing case where input onnx model is used with shared context enabled * Update onnxruntime/core/providers/openvino/openvino_execution_provider.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: MayureshV1 <47039074+MayureshV1@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
vthaniel
pushed a commit
that referenced
this pull request
Aug 28, 2025
#776) * Fix failing case where input onnx model is used with shared context enabled * Update onnxruntime/core/providers/openvino/openvino_execution_provider.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: MayureshV1 <47039074+MayureshV1@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for https://hsdes.intel.com/appstore/article-one/#/article/22021483243 that happened when an onnx source model is compiled with shared context enabled without saving model to ep context.