-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[QNN-EP] Complement PoolOpBuilder to support Pool3d. #25100
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: main
Are you sure you want to change the base?
Conversation
Revise existing PoolOpBuilder to support rank-5 inputs. Note that HTP only supports PoolAvg3d but not PoolMax3d. Test: Add UT testcases.
/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 extends the PoolOpBuilder to support 3D pooling (rank‑5 inputs) and adds corresponding tests for CPU and HTP backends.
- Added new tests for MaxPool_3D, GlobalMaxPool_3D, and various AveragePool 3D cases.
- Updated PoolOpBuilder to accept rank‑5 inputs and to correctly configure pooling parameters, including error handling for unsupported PoolMax3d on NPU backends.
- Adjusted parameter handling and renaming (ceil_mode to rounding_mode) to better reflect 3D pooling requirements.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
onnxruntime/test/providers/qnn/pool_op_test.cpp | Added tests for 3D MaxPool and GlobalMaxPool on CPU; updated HTP tests. |
onnxruntime/test/providers/qnn/average_pool_test.cc | Added tests for 3D and QDQ 3D AveragePool variations. |
onnxruntime/core/providers/qnn/builder/opbuilder/pool_op_builder.cc | Updated PoolOpBuilder to support rank‑5 inputs, adjusted parameter handling, and refactored auto_pad logic. |
if (auto_pad.compare("SAME_LOWER") != 0) { | ||
pad_amount[axis] = total_pads / 2; | ||
pad_amount[axis + rank - 2] = total_pads - pad_amount[axis]; | ||
} else if (auto_pad.compare("SAME_UPPER") != 0) { | ||
pad_amount[axis + rank - 2] = total_pads / 2; | ||
pad_amount[axis] = total_pads - pad_amount[axis + rank - 2]; |
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.
The condition for handling auto_pad values appears inverted; it is recommended to check for equality (e.g., if(auto_pad.compare("SAME_LOWER") == 0)) to correctly differentiate between SAME_LOWER and SAME_UPPER padding strategies.
Copilot uses AI. Check for mistakes.
} | ||
} | ||
ORT_RETURN_IF_NOT(pad_amount.size() == 4, "QNN only support pads with shape[2, 2]."); | ||
ReArranagePads(pad_amount); |
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.
[nitpick] The function call 'ReArranagePads' may be a typo; consider renaming it to 'RearrangePads' for clarity and consistency.
ReArranagePads(pad_amount); | |
RearrangePads(pad_amount); |
Copilot uses AI. Check for mistakes.
Description
Revise existing PoolOpBuilder to support rank-5 inputs.
Note that HTP only supports PoolAvg3d but not PoolMax3d.
Motivation and Context
Enable HSM-Net model support, which contains 3D AveragePool.