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

Integrate onnx 1.17.0 #21897

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Integrate onnx 1.17.0 #21897

wants to merge 29 commits into from

Conversation

liqunfu
Copy link
Contributor

@liqunfu liqunfu commented Aug 28, 2024

Description

for ORT 1.21.0 release

Create following related issues to track skipped tests due to updated ONNX operators in the ONNX 1.17.0 release:
#23162
#23164
#23163
#23161

Motivation and Context

Signed-off-by: Liqun Fu <[email protected]>
@liqunfu liqunfu requested a review from a team as a code owner August 28, 2024 16:53
@liqunfu liqunfu marked this pull request as draft August 28, 2024 16:53
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
@snnn snnn linked an issue Sep 13, 2024 that may be closed by this pull request
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
@liqunfu liqunfu marked this pull request as ready for review October 3, 2024 03:15
@liqunfu liqunfu requested a review from a team as a code owner October 3, 2024 03:15
@guschmue
Copy link
Contributor

guschmue commented Oct 3, 2024

this pr is not complete, tests etc need to be aware of the new onnx version.
we are going to take a look at the ort-web part.

@jywu-msft
Copy link
Member

the disabled onnx_test_runner testing in qnn ep pipelines are failing on "test_reduce_max_empty_set" running on CPU EP , so it's not specifically a QNN EP issue.

@yf711
Copy link
Contributor

yf711 commented Oct 9, 2024

For failing test_reduce_max_empty_set, it seems ORT CPUEP fail to yield -inf array when data input is empty set, according to ReduceMax def

Here's error log when testing onnx/backend/test/data/node/test_reduce_max_empty_set:

[E:onnxruntime:, sequential_executor.cc:516 ExecuteKernel] Non-zero status code returned while running ReduceMax node. Name:'' Status Message:
Traceback (most recent call last):
  File "/root/Documents/ort-cpu/Release/_deps/onnx-src/onnx/backend/test/data/node/test_reduce_max_empty_set/test.py", line 17, in <module>
    outputs = sess.run(
              ^^^^^^^^^
  File "/miniconda/lib/python3.12/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 220, in run
    return self._sess.run(output_names, input_feed, run_options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
onnxruntime.capi.onnxruntime_pybind11_state.RuntimeException: [ONNXRuntimeError] : 6 : RUNTIME_EXCEPTION : Non-zero status code returned while running ReduceMax node. Name:'' Status Message:

Btw, when testing test_reduce_min_empty_set, ORT-CPU EP is able to yield [array([[[inf, inf, inf, inf]] when input data is [2,?,4] and input axes [1]
(for reduceMin op, it yields +inf when input is empty set theoretically)

Signed-off-by: Liqun Fu <[email protected]>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

onnxruntime/test/onnx/TestCase.cc Outdated Show resolved Hide resolved
@liqunfu
Copy link
Contributor Author

liqunfu commented Dec 20, 2024

For failing test_reduce_max_empty_set, it seems ORT CPUEP fail to yield -inf array when data input is empty set, according to ReduceMax def

Here's error log when testing onnx/backend/test/data/node/test_reduce_max_empty_set:

[E:onnxruntime:, sequential_executor.cc:516 ExecuteKernel] Non-zero status code returned while running ReduceMax node. Name:'' Status Message:
Traceback (most recent call last):
  File "/root/Documents/ort-cpu/Release/_deps/onnx-src/onnx/backend/test/data/node/test_reduce_max_empty_set/test.py", line 17, in <module>
    outputs = sess.run(
              ^^^^^^^^^
  File "/miniconda/lib/python3.12/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 220, in run
    return self._sess.run(output_names, input_feed, run_options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
onnxruntime.capi.onnxruntime_pybind11_state.RuntimeException: [ONNXRuntimeError] : 6 : RUNTIME_EXCEPTION : Non-zero status code returned while running ReduceMax node. Name:'' Status Message:

Btw, when testing test_reduce_min_empty_set, ORT-CPU EP is able to yield [array([[[inf, inf, inf, inf]] when input data is [2,?,4] and input axes [1] (for reduceMin op, it yields +inf when input is empty set theoretically)

Thanks for the investigation. It is a bug in ORT. the issue is fixed in this PR.

@liqunfu
Copy link
Contributor Author

liqunfu commented Dec 20, 2024

the disabled onnx_test_runner testing in qnn ep pipelines are failing on "test_reduce_max_empty_set" running on CPU EP , so it's not specifically a QNN EP issue.

This was a bug in ORT which is fixed with this PR.

@liqunfu liqunfu requested a review from a team December 20, 2024 17:15
@jywu-msft jywu-msft requested review from snnn and yf711 December 20, 2024 17:41
@liqunfu
Copy link
Contributor Author

liqunfu commented Dec 20, 2024

the failing CIs are existing issues

@yf711
Copy link
Contributor

yf711 commented Dec 20, 2024

For failing test_reduce_max_empty_set, it seems ORT CPUEP fail to yield -inf array when data input is empty set, according to ReduceMax def
Here's error log when testing onnx/backend/test/data/node/test_reduce_max_empty_set:

[E:onnxruntime:, sequential_executor.cc:516 ExecuteKernel] Non-zero status code returned while running ReduceMax node. Name:'' Status Message:
Traceback (most recent call last):
  File "/root/Documents/ort-cpu/Release/_deps/onnx-src/onnx/backend/test/data/node/test_reduce_max_empty_set/test.py", line 17, in <module>
    outputs = sess.run(
              ^^^^^^^^^
  File "/miniconda/lib/python3.12/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 220, in run
    return self._sess.run(output_names, input_feed, run_options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
onnxruntime.capi.onnxruntime_pybind11_state.RuntimeException: [ONNXRuntimeError] : 6 : RUNTIME_EXCEPTION : Non-zero status code returned while running ReduceMax node. Name:'' Status Message:

Btw, when testing test_reduce_min_empty_set, ORT-CPU EP is able to yield [array([[[inf, inf, inf, inf]] when input data is [2,?,4] and input axes [1] (for reduceMin op, it yields +inf when input is empty set theoretically)

Thanks for the investigation. It is a bug in ORT. the issue is fixed in this PR.

Thanks for the fix! This looks good to me

@@ -323,7 +323,46 @@
"^test_dequantizelinear_int4",
"^test_dequantizelinear_uint4",
"^test_quantizelinear_int4",
"^test_quantizelinear_uint4"
"^test_quantizelinear_uint4",
// onnx 1.17.0 op tests: skip until implemented in ORT
Copy link
Member

Choose a reason for hiding this comment

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

But these ops almost have no change. Should we update the registrations to include them?

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 created #23162 to track unimplemented kernels. because new types were added, it requires more than registration. It is better to keep the PR limited and have other PRs to complete the kernel implementation and testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Integrate with ONNX 1.17.0 release branch
5 participants