CVS-178009: Fix provider tests based on code review #880

sgbihu · 2025-12-09T07:36:02Z

Description

Microsoft forbade setting the integer type's error.
Using new API to enable/disable the tests.

Motivation and Context

https://jira.devtools.intel.com/browse/CVS-178009

yuslepukhin · 2025-12-09T21:07:12Z

Looks good to me. I will comment in the mainline about the tests.

MayureshV1

Looks good.
Dmitri from ORT team has also reviews the change.

MayureshV1 · 2025-12-10T00:41:50Z

This implementation would fail the test case in case single ops fallback to running on default CPU EP. needs more time to rework. Converting to draft.

For now, we will revert these test change. PR #883

FYI. @yuslepukhin

onnxruntime/test/providers/cpu/nn/conv_transpose_op_test.cc

onnxruntime/test/providers/cpu/tensor/quantize_linear_test.cc

onnxruntime/test/providers/cpu/tensor/resize_op_test.cc

Copilot

Pull request overview

This PR addresses provider test issues identified during code review by updating OpenVINO execution provider test exclusions and support configurations. The changes primarily involve migrating from deprecated test APIs to newer configuration methods and adding OpenVINO-specific test variants with adjusted expected outputs.

Key changes:

Migrated test exclusion patterns from deprecated test.Run() parameter to ConfigExcludeEps() and RunWithConfig() API
Added OpenVINO-specific test cases with adapted expected values due to different quantization rounding behaviors
Updated OpenVINO capability definitions to enable previously disabled operations

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
slice_op.test.cc	Added OpenVINO to excluded providers for tests with ONNX shape disagreement
resize_op_test.cc	Migrated to new exclusion API and added OVEP-specific test variants for uint8 resize operations
quantize_linear_test.cc	Migrated to new exclusion API and added OVEP-specific quantization tests with adjusted expected outputs
cast_op_test.cc	Added exclusion for OpenVINO when input size is zero
loop_test.cc	Added OpenVINO to excluded providers for iteration count test
quantize_ops_test.cc	Migrated contrib op tests to new API and added OVEP-specific test variants
data_ops.cc	Removed operations from model-only support list and added UINT32 type support

Comments suppressed due to low confidence (1)

onnxruntime/test/contrib_ops/quantize_ops_test.cc:1

The expected output differs from the original test (line 535 expects Int4x2(-1, 1) while this OVEP variant expects Int4x2(0, 1) for the second pair). Add a comment explaining why OpenVINO's Int4 quantization produces a different value for the -3.0f input.

// Copyright (c) Microsoft Corporation. All rights reserved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

onnxruntime/core/providers/openvino/ov_versions/data_ops.cc

onnxruntime/test/providers/cpu/tensor/resize_op_test.cc

onnxruntime/test/providers/cpu/tensor/quantize_linear_test.cc

onnxruntime/test/providers/cpu/tensor/resize_op_test.cc

Co-authored-by: Copilot <[email protected]>

This reverts commit 8b81a24.

ericcraw · 2025-12-16T19:15:57Z

onnxruntime/core/providers/openvino/backends/basic_backend.h

                               [&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()) continue;


If we can't match onnx io names to ov io names we shouldn't just skip it. We should fail.

In this case, the OV has optimized out this input because it is unused. How to handle this case?

Hmm, in that case, perhaps we only allow skipping if there's fewer ov i/o parameters than onnx. We should also log a warning that we couldn't find the onnx i/o name in the resulting ov compiled model and that it may have been optimized out.

Thoughts?

Can we add a mechanism in OV ONNX FE to avoid the parameter be optimized out? This need more investigation to research how to keep the unused Parameter

I mean sure, ideally, OV doesn't remove I/O but in the time between now and a potential future that it doesn't, I'd prefer that we don't silently skip binding I/O.

As an aside, do we know the FE is removing the input? Or are you proposing that there be ONNX FE setting that prevents later passes from removing model I/O?

I have tried to remove the code in OV ONNX FE, the remove_dangling_parameters will delete the floating parameters in the model. Because the unused parameter may 0-sized buffer, which will have an issue when allocating the tensors. How about just output some warnings?

std::shared_ptr<ov::Model> Graph::convert() { convert_to_ov_nodes(); remove_dangling_parameters(); auto function = create_model(); set_metadata(function); return function; }

@sgbihu , If we need an OV change we will not intercept this for ORT1.24 and can plan those test changes later.
Any changes we make to the test, Can we ensure functionality with OV 2025.4.

ericcraw · 2025-12-16T19:36:55Z

onnxruntime/core/providers/openvino/backends/basic_backend.h

+          auto shape_size = ov::shape_size(shape.get_shape());
+          if (0 == shape_size) {
+            has_dynamic_io_ = true;
+          }


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?

The reason is the result is 0 size tensor. The OV can't create 0 size buffer. We need set this as dynamic to avoid allocate the buffer.

Another way is we can disable this test case due to 0 size buffer.

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.

ericcraw · 2025-12-18T17:22:18Z

onnxruntime/core/providers/openvino/backends/basic_backend.h

                               [&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()) {


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.

ericcraw · 2025-12-19T17:46:35Z

onnxruntime/core/providers/openvino/backends/basic_backend.h

+          auto shape_size = ov::shape_size(shape.get_shape());
+          if (0 == shape_size) {
+            has_dynamic_io_ = true;
+          }


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.

ericcraw

I'm going to be out for the holidays, as long as we address [this]. Consider it an approval for me. (#880 (comment))

MayureshV1 approved these changes Dec 9, 2025

View reviewed changes

MayureshV1 marked this pull request as draft December 10, 2025 00:23

MayureshV1 changed the title ~~Fix provider tests based on code review~~ CVS-178009: Fix provider tests based on code review Dec 10, 2025

sgbihu force-pushed the fix_provider_tests branch from 8ec98b6 to 6ae5d7e Compare December 11, 2025 03:17

sgbihu added 4 commits December 16, 2025 07:30

change the code based on review inputs

c572ba2

Update the unit test based on OV test results

8952ea3

Disable loop cases

790e552

The OV doesn't include the fix, need to disable again

4cebc5d

sgbihu force-pushed the fix_provider_tests branch from 0649789 to 4cebc5d Compare December 15, 2025 23:30

MayureshV1 marked this pull request as ready for review December 15, 2025 23:44

MayureshV1 self-requested a review December 15, 2025 23:45

MayureshV1 approved these changes Dec 15, 2025

View reviewed changes

yuslepukhin reviewed Dec 16, 2025

View reviewed changes

onnxruntime/test/providers/cpu/nn/conv_transpose_op_test.cc Show resolved Hide resolved

onnxruntime/test/providers/cpu/tensor/quantize_linear_test.cc Outdated Show resolved Hide resolved

onnxruntime/test/providers/cpu/tensor/resize_op_test.cc Show resolved Hide resolved

sgbihu added 3 commits December 16, 2025 11:44

Fix update comments based on review inputs

bf10bd2

Enable cast

8b81a24

Update comments

850f17b

MayureshV1 requested review from MayureshV1 and Copilot December 16, 2025 06:43

MayureshV1 approved these changes Dec 16, 2025

View reviewed changes

Copilot AI reviewed Dec 16, 2025

View reviewed changes

MayureshV1 and others added 4 commits December 15, 2025 22:53

Update onnxruntime/test/providers/cpu/tensor/resize_op_test.cc

71e1247

Co-authored-by: Copilot <[email protected]>

Revert "Enable cast"

7514048

This reverts commit 8b81a24.

Fix review inputs

ac1e9f3

Try to handle input has optimized and dynamic shape

81ba841

yuslepukhin approved these changes Dec 16, 2025

View reviewed changes

ericcraw reviewed Dec 16, 2025

View reviewed changes

Add warnings

d0916b9

ericcraw reviewed Dec 19, 2025

View reviewed changes

CVS-178009: Fix provider tests based on code review #880

Are you sure you want to change the base?

CVS-178009: Fix provider tests based on code review #880

Uh oh!

Conversation

sgbihu commented Dec 9, 2025 • edited by MayureshV1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Uh oh!

yuslepukhin commented Dec 9, 2025

Uh oh!

MayureshV1 left a comment

Choose a reason for hiding this comment

Uh oh!

MayureshV1 commented Dec 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgbihu Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericcraw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sgbihu commented Dec 9, 2025 •

edited by MayureshV1

Loading

sgbihu Dec 18, 2025 •

edited

Loading