-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[QNN EP] Revert workarounds for problems in old QNN versions #25171
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
[QNN EP] Revert workarounds for problems in old QNN versions #25171
Conversation
* Re-enable tests and remove workarounds that were introduced as part of a QNN <= 2.31 upgrade but are no longer necessary.
/azp run Linux QNN CI Pipeline,Win_TRT_Minimal_CUDA_Test_CI,Windows ARM64 QNN CI Pipeline,Windows GPU Doc Gen CI Pipeline,Windows x64 QNN CI Pipeline |
Azure Pipelines successfully started running 5 pipeline(s). |
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.
Pull Request Overview
This PR reverts legacy workarounds introduced for older QNN versions by removing or adjusting the use of tolerance parameters in multiple tests. Key changes include:
- Removing QDQTolerance parameters (and their associated comments) from several test cases.
- Adjusting tolerance values (e.g. 0.0041f for matmul and gemm tests) to reflect current QNN SDK accuracy.
- Removing obsolete broken test entries from the QNN test suite.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
onnxruntime/test/providers/qnn/resize_test.cc | Removed QDQTolerance parameter and outdated tolerance comments. |
onnxruntime/test/providers/qnn/pool_op_test.cpp | Removed QDQTolerance parameter and its comments for MaxPool tests. |
onnxruntime/test/providers/qnn/matmul_test.cpp | Adjusted tolerance value for per-channel matmul tests from 0.005f to 0.0041f. |
onnxruntime/test/providers/qnn/lrn_op_test.cc | Removed conditional tolerance usage and simplified test call. |
onnxruntime/test/providers/qnn/layer_norm_test.cc | Updated a TODO comment to reference the new QNN SDK version. |
onnxruntime/test/providers/qnn/gemm_op_test.cc | Adjusted tolerance values for Gemm tests from 0.0074f to 0.00410f. |
onnxruntime/test/providers/qnn/conv_test.cc | Removed QDQTolerance parameters and related comments from Conv tests. |
onnxruntime/test/providers/qnn/average_pool_test.cc | Removed QDQTolerance parameters and outdated comments in AveragePool tests. |
onnxruntime/test/onnx/TestCase.cc | Removed obsolete broken test entries no longer applicable to current QNN SDK. |
Comments suppressed due to low confidence (3)
onnxruntime/test/onnx/TestCase.cc:1396
- Confirm that the removal of these obsolete broken test entries is intentional; consider documenting external tracking if these tests are expected to be revisited later.
broken_tests->insert({"resize_downsample_scales_linear", "result differs"});
onnxruntime/test/providers/qnn/matmul_test.cpp:282
- Verify that the new tolerance value of 0.0041f is confirmed by extensive testing and is representative for the expected accuracy.
RunQDQPerChannelMatMulOpTest<uint16_t, int8_t, uint16_t>({2, 3, 3}, {3}, -1, QDQTolerance(0.0041f));
onnxruntime/test/providers/qnn/gemm_op_test.cc:339
- Ensure that the new tolerance threshold (0.00410f) for Gemm tests is validated across all target platforms.
QDQTolerance(0.00410f));
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.
* Re-enable facedetection_op8_qdq
/azp run Linux QNN CI Pipeline,Win_TRT_Minimal_CUDA_Test_CI,Windows ARM64 QNN CI Pipeline,Windows GPU Doc Gen CI Pipeline,Windows x64 QNN CI Pipeline |
Azure Pipelines successfully started running 5 pipeline(s). |
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.
* Re-enable tests and remove workarounds that were introduced as part of a QNN <= 2.31 upgrade but are no longer necessary. QNN/QAIRT releases about once a month. As ONNX Runtime adopts these new versions, some number of tests are often found to be impacted. Consequently, tests are skipped and tolerances are loosened. This change reverts as many of those workarounds as possible that were made for QNN upgrades between 2.17 and 2.31, inclusive. The most recent few releases were intentionally not examined to minimize impact on users on old versions and to avoid lock-in to the bleeding edge. --------- Co-authored-by: Jeff Kilpatrick <[email protected]>
* Re-enable tests and remove workarounds that were introduced as part of a QNN <= 2.31 upgrade but are no longer necessary. QNN/QAIRT releases about once a month. As ONNX Runtime adopts these new versions, some number of tests are often found to be impacted. Consequently, tests are skipped and tolerances are loosened. This change reverts as many of those workarounds as possible that were made for QNN upgrades between 2.17 and 2.31, inclusive. The most recent few releases were intentionally not examined to minimize impact on users on old versions and to avoid lock-in to the bleeding edge. --------- Co-authored-by: Jeff Kilpatrick <[email protected]>
### Description - #24265 - #24616 - #24640 - #24707 - #24646 - #24750 - #24809 - #24895 - #24820 - #25002 - #25171 - #25283 - #24818 - #25351 - #25361 - #25388 - #25520 - #25158 ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> --------- Co-authored-by: quic-zhaoxul <[email protected]> Co-authored-by: Yuduo Wu <[email protected]> Co-authored-by: Hector Li <[email protected]> Co-authored-by: chenweng-quic <[email protected]> Co-authored-by: qti-yuduo <[email protected]> Co-authored-by: Akupadhye <[email protected]> Co-authored-by: Jeff Kilpatrick <[email protected]> Co-authored-by: Jeff Kilpatrick <[email protected]> Co-authored-by: George Wu <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: quic-calvnguy <[email protected]> Co-authored-by: Changming Sun <[email protected]> Co-authored-by: Yulong Wang <[email protected]>
Description
Motivation and Context
QNN/QAIRT releases about once a month. As ONNX Runtime adopts these new versions, some number of tests are often found to be impacted. Consequently, tests are skipped and tolerances are loosened. This change reverts as many of those workarounds as possible that were made for QNN upgrades between 2.17 and 2.31, inclusive. The most recent few releases were intentionally not examined to minimize impact on users on old versions and to avoid lock-in to the bleeding edge.