-
Notifications
You must be signed in to change notification settings - Fork 56
CVS-178009: Fix provider tests based on code review #880
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
base: ovep-develop
Are you sure you want to change the base?
Changes from all commits
c572ba2
8952ea3
790e552
4cebc5d
bf10bd2
8b81a24
850f17b
71e1247
7514048
ac1e9f3
81ba841
d0916b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,13 @@ struct OnnxToOvNetworkBindings { | |
| [&onnx_name](const auto& ov_parameter_info) { return ov_parameter_info.get_names().contains(onnx_name); }); | ||
| bool matched_names = it != ov_parameters.end(); | ||
|
|
||
| if (it == ov_parameters.end()) { | ||
| LOGS_DEFAULT(WARNING) << log_tag << "The input '" << onnx_name | ||
| << "' is not used due to OpenVINO optimization. " | ||
| "This may cause issues if the input is required."; | ||
| continue; | ||
| } | ||
|
|
||
| // For Stateful Model Compilation, the ONNX model includes KV cache (past/present) tensors. | ||
| // However, these tensors are internally converted to a stateful representation, which removes them. | ||
| // It's also possible that the onnx model does not contain tensors such as beam_idx, whereas our converted | ||
|
|
@@ -110,6 +117,11 @@ struct OnnxToOvNetworkBindings { | |
|
|
||
| info.SetFullyDynamic(has_fully_dynamic); | ||
| info.SetBoundedDynamic(has_bounded_dynamic); | ||
| } else { | ||
| auto shape_size = ov::shape_size(shape.get_shape()); | ||
| if (0 == shape_size) { | ||
| has_dynamic_io_ = true; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate a bit more on this case? The shape came from the OV compiled model, and the shape is reported as static but has a zero dimension? Or maybe no dimension? Does that actually make it dynamic?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another way is we can disable this test case due to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer we don't have specific handling in i/o binding for it a case ov doesn't support, but if we keep it, just comment why it's there so it can be more easily evaluated if we want to change it in the future. |
||
| } | ||
|
|
||
| input_output_map.push_back(std::move(info)); | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer that we only do this if
onnx_input_map.size() > std::distance(ov_parameters.begin(), ov_parameters.end())and we check it after we handle session_context.enable_causallm.