Repository navigation
Preserve captured subgraph dependencies in quantization graph utilities - #34184
Open
Afloat (Afloat16) wants to merge 3 commits into
Open
Afloat (Afloat16) wants to merge 3 commits into
Afloat (Afloat16) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 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
| 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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
ff57d2442bf826c42f0dd70c96b1e6b7001447e1: 13 methods passed and 13 subtests passed, with no skips.16f7f332eac743749ade445f8cecc7f2d4d688b2in the same runtime makes all four sparse subcases fail withGraph is not a DAG. The runner reports4 failed, 2 passed, 8 deselected: the four failures are subcases across the two selected methods.lintrunnerinitialization, changed-file formatting and lint, remote Ruff checks, andgit diff --checkpassed.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 basea8246c0d094f1ab0d6bf236228e5e67683321cd7. 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.