Fix TransposeOptimizer type error on zero-point-less DequantizeLinear (#28716)#29192
Conversation
|
Heads up — #28729 also fixes #28716, but from a different angle (the duplicate-name theory). The real break on the repro is a type mismatch: a zero-point-less int8 |
There was a problem hiding this comment.
Pull request overview
Fixes a TransposeOptimizer session-initialization type error triggered when commuting a Transpose through an ONNX-domain DequantizeLinear that omits the optional zero-point (opset 21+). The optimizer synthesizes a QuantizeLinear; without output_dtype, ONNX type inference defaults that output to uint8, conflicting with copied value-info (e.g., int8) and causing Graph::Resolve() to fail.
Changes:
- When inserting a
QuantizeLinearfor a zero-point-lessDequantizeLinear, setoutput_dtypeto match the DQ input element type (ONNX domain, opset 21+ only). - Add a regression test ensuring the transpose pair cancels in the no-zero-point int8 DQ pattern (and that optimization/resolve succeeds).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc |
Pins inserted QuantizeLinear’s output_dtype for zero-point-less ONNX DequantizeLinear to prevent type-inference mismatches during Resolve(). |
onnxruntime/test/optimizer/transpose_optimizer_test.cc |
Adds a targeted regression test for the no-zero-point DQ case (opset 21), verifying transposes cancel and the optimized graph resolves. |
tianleiwu
left a comment
There was a problem hiding this comment.
Thanks for the careful re-diagnosis — the root cause (a zero-point-less DQ making the inserted QuantizeLinear default to uint8 while the copied value-info is int8) is correct, and pinning output_dtype is the right minimal fix. A couple of confirmations and one non-blocking suggestion:
static_cast<int64_t>(dq_input_dtype)is sound:api::DataTypeshares the ONNXTensorProto_DataTypenumeric values, so it is a validoutput_dtype.- Excluding
UINT8/UNDEFINEDand gating onIsOnnxDomainare both correct and keep the zero-point andcom.microsoftpaths unchanged.
Non-blocking suggestion (inline): the corrective output_dtype is only emitted for ONNX opset ≥ 21, but there is no opset gate around MakeQDQNodeUnit, so the original Resolve() mismatch still occurs for an opset 19/20 zero-point-less int8 DQ (and the pinned type would itself be invalid if the DQ input were a type QuantizeLinear can't produce, e.g. int32). Consider skipping the push-through (return false) when the needed output_dtype can't be represented. Pre-existing limitation, so not a blocker.
tianleiwu
left a comment
There was a problem hiding this comment.
Review Summary
The approach is sound — using double accumulators for CPU integer reductions and saturating abs for CUDA no-op paths correctly eliminates UB and overflow. Tests are comprehensive.
One blocking issue: missing CUDA template instantiations will cause linker failures (see inline comment).
Dismissing: this review was posted in error to the wrong PR (content was unrelated to #29192). Re-reviewing with correct feedback.
tianleiwu
left a comment
There was a problem hiding this comment.
Re-reviewed on head ec6e7b4.
The fix is correct, minimal, and conservative. Pinning the inserted QuantizeLinear's output_dtype to the DQ input type only on the no-zero-point path is the right root-cause fix; the zero-point path (where the Q output type is inferred from the zero-point) is left unchanged.
The earlier concern about there being no opset gate around MakeQDQNodeUnit is now addressed: the !IsOnnxDomain(dq_domain) || !domain_opset || *domain_opset < 21 || !IsQuantizeLinearOutputType(...) guard bails out (returns false) whenever output_dtype can't be represented (pre-opset-21, non-ONNX domain, or a non-quantizable type), and the bail-out happens before any node is added, so the graph stays valid. All call sites tolerate a false return and leave a valid DQ -> Transpose -> consumer graph.
Other checks:
IsQuantizeLinearOutputTypeis complete relative to theapi::DataTypeenum (int8/uint8, int16/uint16, four fp8 variants; the enum has no int4/uint4).- The regression test exercises both branches: opset 21 (transposes cancel) and opset 19 (push-through skipped, model still initializes).
LGTM.
Problem
With
graph_optimization_level >= ORT_ENABLE_BASIC, a model with mixed int8/uint8 QDQ inside a local function fails session init:Root cause
The issue's "duplicate NodeArg name" title is a misdiagnosis —
QuantizeLinear_out0is created exactly once. The real cause: whenTransposeOptimizerpushes a Transpose through aDequantizeLinearthat has no zero-point (MakeQDQNodeUnit), it inserts a newQuantizeLinearreusing the DQ's inputs. With no zero-point and nooutput_dtype, ONNX type inference defaults the new Q's output to uint8, but the value-info copied from the DQ input is int8 →Graph::Resolve()rejects it.Fix
In
MakeQDQNodeUnit, when the DQ has no zero-point, set the inserted Q'soutput_dtypeto the DQ input's element type (ONNX domain only —output_dtypeexists from opset 21; the zero-point path is unchanged).Verification
Reporter's repro model:
onnx.checker.check_model(full_check=True); the inserted node now carriesoutput_dtype=int8.TransposeOptimizerTests.TestDequantizeLinearNoZeroPoint(Transpose → no-zp int8 DequantizeLinear → Transpose; expects the Transposes to cancel).Scope
ONNX domain, opset 21+ (where
output_dtypeexists). Pre-21 andcom.microsoftQDQ rely on an explicit zero-point and are unaffected.Fixes #28716