Make partitioning utils QDQ aware so it does not break up QDQ node units #19723
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ioningUtilsQDQAware
…ioningUtilsQDQAware
Fix order of nodes in partition. DQ -> target -> Q must be added sequentially. Fix x86 build error. Fix android issue with gtest flags not being processed (couldn't debug failing test easily when --gtest_filter didn't work)
Is it guaranteed that the node order is topological sorted? Otherwise, I think it's safe to build up the node_outputs first before processing any inputs. Refers to: onnxruntime/core/providers/partitioning_utils.cc:328 in 956412b. [](commit_id = 956412b, deletion_comment = False) |
It is currently as we process nodes in topological order when doing partitioning. We could process all outputs first before inputs. Is there a benefit apart from not requiring things to be topologically sorted if we do that? In reply to: 1980232432 Refers to: onnxruntime/core/providers/partitioning_utils.cc:328 in 956412b. [](commit_id = 956412b, deletion_comment = False) |
…ns variant. Fix issue with QDQ node group that has no Q nodes. TODO: Fix QnnHTPBackendTests.TopK_LargestFloats_U8_LastAxis
It happened when I was trying to fix it in my PR which made the nodes not in topological order any more. That's why I was asking. I was wondering in case someone else do that again without the awareness the code here which requires nodes in topological order. In reply to: 1980424111 Refers to: onnxruntime/core/providers/partitioning_utils.cc:328 in 956412b. [](commit_id = 956412b, deletion_comment = False) |
…et node (int64_t indices output that is not quantized) as well as edge through Q node (values output)
…ioningUtilsQDQAware
- The whole QDQ setup needs a rethink at some point as it's currently spread across too many places (framework, optimizer, base providers lib, EP specific providers lib) - move NodeGroup to framework/node_unit.h and ValidateNodeGroupQDQNodes to NodeGroup::CanCreateNodeGroup so it's in the framework lib as it's used by NodeUnit - move GetAllNodeUnits to optimizer - doesn't quite belong there but this works will all the current EPs that use it.
|
/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline |
|
/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models,Android CI Pipeline |
|
/azp run iOS CI Pipeline,ONNX Runtime React Native CI Pipeline |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Azure Pipelines successfully started running 10 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 10 pipeline(s). |
…ioningUtilsQDQAware
Co-authored-by: Edward Chen <[email protected]>
### Description Enable code in QNN UT to verify the fix for partition issue relate to QDQ model. #19723
Description
If the EP handles QDQ node units, we need to make sure we do not split those into different partitions.
Update the partitioning utils to be QDQ aware. If there are node units we process the logical nodes they represent instead of individual nodes. This ensure we process all nodes in a QDQ node unit at the same time so that they are always in the same partition.
Motivation and Context
Fix one of the issues in #19590