Skip to content

Fix incorrect output for QDQ Resize with non-nearest interpolation modes under ORT_ENABLE_ALL - #28454

Merged
tianleiwu merged 5 commits into
mainfrom
copilot/fix-ort-enable-all-output
Jun 3, 2026
Merged

Fix incorrect output for QDQ Resize with non-nearest interpolation modes under ORT_ENABLE_ALL#28454
tianleiwu merged 5 commits into
mainfrom
copilot/fix-ort-enable-all-output

Conversation

Copilot AI commented May 11, 2026

Copy link
Copy Markdown
Contributor

Description

  • qdq_selectors.cc: In DropQDQNodeGroupSelector::Check, add a guard that skips the QDQ-drop optimization when the target node is Resize with a non-nearest mode attribute ("linear", "cubic", etc.). When mode is absent the ONNX default is "nearest", so the optimization remains allowed in that case.
  • qdq_transformer_test.cc: Add Resize_Linear_No_QDQ_Drop test asserting that DQ/Q nodes are preserved around Resize for linear and cubic modes after Level2 optimization.

Motivation and Context

The DropQDQ optimizer incorrectly dropped the surrounding DQ/Q nodes for all Resize modes. Dropping these nodes bypasses float-space interpolation — Resize ends up operating directly on raw quantized integers, producing wrong results.

The optimization is only semantically valid for mode="nearest", which copies existing quantized values without arithmetic. For interpolating modes, the full DQ → Resize → Q chain must execute in float space:

# With ORT_DISABLE_ALL (correct):   DQ → float → Resize(linear) → Q  ✓
# With ORT_ENABLE_ALL (before fix): Resize(linear) on raw int8       ✗

Input [2.0, 7.0, 4.0] upsampled 1×1×1×3 → 1×1×1×6 with half_pixel:

  • Expected / DISABLE_ALL: [2.0, 3.0, 6.0, 6.0, 5.0, 4.0]
  • ENABLE_ALL before fix: [2.0, 3.0, 5.0, 6.0, 4.0, 4.0]

Copilot AI and others added 2 commits May 11, 2026 17:48
Copilot AI changed the title [WIP] Fix incorrect output for QuantizeLinear-DequantizeLinear Resize pattern Fix incorrect output for QDQ Resize with non-nearest interpolation modes under ORT_ENABLE_ALL May 11, 2026
Copilot AI requested a review from tianleiwu May 11, 2026 17:52
@tianleiwu
tianleiwu marked this pull request as ready for review May 12, 2026 17:27
@titaiwangms

Copy link
Copy Markdown
Contributor

Review — QDQ drop guard for non-nearest Resize

Verdict: Fix is correct and well-tested. One substantive caveat worth confirming; the rest are polish. The change correctly prevents DropQDQ from removing DQ/Q around interpolating Resize (linear/cubic), which must run in float space. Registration wiring, the inverse test, and .s() usage were verified against the repo.

🟠 Major (worth addressing / confirming scope)

Nearest is not a pure value copy under coordinate_transformation_mode="tf_crop_and_resize".

  • upsample.cc:1110 passes extrapolation_value_ into UpsampleNearest; it fires when the coordinate mode is tf_crop_and_resize (upsamplebase.h:255). Out-of-crop positions get static_cast<T>(extrapolation_value) written raw into the quantized domain, but extrapolation_value is authored in the dequantized float domain. The correct stored value would be quantize(extrapolation_value). With the default extrapolation_value=0.0 and a nonzero zero_point, the dropped graph writes 0 where it should write zero_point → observable mismatch.
  • This is pre-existing (the PR doesn't introduce it) and narrow, but it falsifies the new comment's claim that nearest "copies existing values." Recommendation: either also bail when coordinate_transformation_mode == "tf_crop_and_resize", or soften the comment so it doesn't assert an unconditional pure copy.
  • For completeness: antialias and exclude_outside do not affect the nearest path, so the guard is complete with respect to those.

🟡 Minor

  • Missing test for omitted mode. The fix relies on absent-mode ⇒ nearest ⇒ drop allowed, but no test builds Resize without a mode attribute asserting QDQ is dropped. Worth locking in the fallback semantics.
  • Comment slightly misleading on the absence branch. After Graph::Resolve(), schema defaults are injected (graph.cc:3588-3598), so mode is always present at runtime — the != end clause does the real work and the absence branch is a defensive fallback for unresolved/hand-built graphs. Correct either way; just clarify the comment.
  • Structural. This adds an op-specific OpType() == "Resize" guard inside the generic "data-preserving ops" DropQDQNodeGroupSelector. Acceptable as a targeted patch, but if per-op exceptions accumulate, consider a dedicated selector or a per-op predicate table near the DropQDQ registration.
  • Naming. mode_attr_it breaks this file's _iter convention (e.g., block_size_iter, a_iter); mode_iter is the local idiom.

🔵 Nits

  • Test name Resize_Linear_No_QDQ_Drop also covers cubic → consider Resize_NonNearest_No_QDQ_Drop.
  • The inline comment largely duplicates the block comment above it; the only new info is the default-when-absent behavior.
  • Magic "" for the nearest_mode argument (vs. the helper's declared default), and a minor sibling-test naming inconsistency (sizes_shape vs sizes_data).

✅ Praise

  • Correct math distinction (value-copy vs. float interpolation), fails closed for non-nearest modes, and leaves the co-registered DepthToSpace selector path untouched.
  • The new test is a genuine inverse of the existing Resize test and covers linear/cubic × contrib/non-contrib QDQ.

Reviewed with a multi-model agent team (readability, code, adversarial, and deep spec reviewers).

@titaiwangms titaiwangms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking:

The new tf_crop_and_resize guard branch is untested (code + critical reviewers). The shared helper BuildQDQResizeTestCase always builds an empty {0} roi (qdq_test_utils.h:303), and a valid tf_crop_and_resize requires a 2*rank roi (upsample.cc:1091), so the new branch can't be reached through it. A regression deleting/misspelling that condition wouldn't be caught. Fix: a dedicated test builder supplying a valid roi + mode="nearest" + tf_crop_and_resize, asserting Q/DQ preserved. (The author honestly documented this gap in a test comment.)

@tianleiwu
tianleiwu merged commit 0324561 into main Jun 3, 2026
86 of 87 checks passed
@tianleiwu
tianleiwu deleted the copilot/fix-ort-enable-all-output branch June 3, 2026 23:41
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.

ORT_ENABLE_ALL produces incorrect output for QuantizeLinear-DequantizeLinear Resize(linear) pattern

3 participants