Skip to content

Preserve captured subgraph dependencies in quantization graph utilities - #34184

Open
Afloat (Afloat16) wants to merge 3 commits into
microsoft:mainfrom
Afloat16:fix/quantization-subgraph-captured-inputs-20261112
Open

Afloat (Afloat16) wants to merge 3 commits into
microsoft:mainfrom
Afloat16:fix/quantization-subgraph-captured-inputs-20261112

Conversation

@Afloat16

@Afloat16 Afloat (Afloat16) commented Nov 13, 2026 •

Copy link
Copy Markdown

Description

The quantization ONNXModel utilities only consider a node's explicit inputs. Values captured by an If or Loop subgraph are consequently missing from topological dependencies and unused-constant analysis. A valid model can be reordered so a capture precedes its producer, or lose a constant/initializer used only inside a subgraph.

Collect lexical dependencies recursively from GRAPH and GRAPHS attributes, including graph outputs that directly reference an outer value. Subtract each subgraph's formal inputs, dense/sparse initializers and local node outputs so local names correctly shadow outer values. Use this input set for both topological sorting and constant cleanup. Seed sorting from graph inputs and both dense and sparse root initializers, using each sparse initializer's values.name.

This addresses one missing-dependency problem in both consumers. It does not reorder subgraph-internal nodes or change the separate transformers.OnnxModel implementation.

Validation

  • Ten new unittest methods cover native If/Loop execution, nested captures, save/reload, captured constants and initializers, local-name shadowing, duplicate explicit inputs, missing/cyclic capture rejection, and sparse-only captures.
  • New and existing model graph tests on ff57d2442bf826c42f0dd70c96b1e6b7001447e1: 13 methods passed and 13 subtests passed, with no skips.
  • The two sparse-capture methods exercise four subcases: both If branches and Loop trip counts 0 and 2. They execute valid models on the native CPU runtime, sort/save/reload them, retain the sparse initializer without a dense duplicate, and check the resulting values.
  • Restoring the helper from the preceding PR head 16f7f332eac743749ade445f8cecc7f2d4d688b2 in the same runtime makes all four sparse subcases fail with Graph is not a DAG. The runner reports 4 failed, 2 passed, 8 deselected: the four failures are subcases across the two selected methods.
  • The GRAPHS attribute control checks dependency ordering only; no custom operator execution is claimed.
  • Repository lintrunner initialization, changed-file formatting and lint, remote Ruff checks, and git diff --check passed.

Exact-source native CPU validation and parent comparison. The earlier lexical-capture comparison is retained in the preceding validation run.

The native runtime is the unchanged wheel from the repository's official CPU Release build.py/CMake build at 8dea1194e974fa07b1d2f07a190949c3a92cab80. Its C++ sources match base a8246c0d094f1ab0d6bf236228e5e67683321cd7. The validation installs that built wheel and overlays the complete quantization package from the exact patch, verifying all 60 Python files byte-for-byte and recording a SHA-256 manifest. Tests use normal native imports on the hosted Ubuntu runner. This is targeted Linux CPU validation, not GPU execution or the full C++ suite.

Local native imports were blocked before testing by this host's executable-stack policy; the native results above come from the linked hosted run.

Motivation and Context

Control-flow subgraphs use ONNX lexical scoping. Graph transformations must retain and order those dependencies just as they do explicit node inputs.

Copilot AI balanced review requested due to automatic review settings November 13, 2026 09:53
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI 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.

🟡 Changes recommended

Control-flow captures of root-level sparse initializers can incorrectly fail topological sorting with Graph is not a DAG and block model saving because sparse initializer names are omitted from dependency sources.

1 open issue
What changed in this PR

Updates quantization graph utilities to account for values captured by control-flow subgraphs, preserving producer ordering and preventing removal of live constants and initializers.

Changes:

  • Recursively collects dependencies from graph attributes while respecting local-name shadowing.
  • Uses captured dependencies in topological sorting and unused-constant cleanup.
  • Adds regression tests for If/Loop execution, nested captures, save/reload, and invalid dependencies.
File Description
onnxruntime/​test/​python/​quantization/​test_captured_subgraph_inputs.py Adds eight test methods covering captured dependencies and related edge cases.
onnxruntime/​python/​tools/​quantization/​onnx_model.py Adds recursive dependency collection and applies it to sorting and cleanup.

Review effort: Balanced

@mirounga
mirounga requested review from mirounga and a balanced review from Copilot November 13, 2026 16:07

Copilot AI 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.

🟢 Approval recommended

No active review issues remain for the changes that preserve values captured by control-flow subgraphs.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

from pathlib import Path

import numpy as np
import onnx
Remove the mixed import forms flagged by CodeQL without changing the test behavior.

This branch has not been deployed

No deployments
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.

3 participants