fix: order the placeholders first before computing serde range constraints - #4648
Open
tp5uiuc wants to merge 1 commit into
Open
fix: order the placeholders first before computing serde range constraints#4648tp5uiuc wants to merge 1 commit into
tp5uiuc wants to merge 1 commit into
Conversation
github-actions
Bot
added
component: tests
This was referenced Aug 29, 2026
tp5uiuc
force-pushed
the
fix/serde-placeholder-ordering
branch
2 times, most recently
from
August 30, 2026 04:34
2c7aae6 to
1358631
Compare
…aints
torch_tensorrt.save() raises IndexError: list index out of range whenever the
partitioner has sent ops back to PyTorch. It is not architecture-specific --
any caller that hits fallback reaches it on any GPU:
trt_gm = torchtrt.dynamo.compile(
exp_program, inputs=[...], min_block_size=1,
torch_executed_ops={"torch.ops.aten.linear.default"},
)
torchtrt.save(trt_gm, path, inputs=[Input(shape=(4, 10))], retrace=False)
IndexError: list index out of range
torch_tensorrt/dynamo/_exporter.py:491 range_constraints = make_constraints(...)
torch/_export/non_strict_utils.py:935 shape_spec = flat_dynamic_shapes[...]
save(retrace=True, use_legacy_exporter=True) with dynamic shapes fails the same
way; both options land in create_trt_exp_program().
make_constraints() walks enumerate(gm.graph.nodes) and indexes
flat_dynamic_shapes with the *node* index rather than the placeholder ordinal,
so it silently requires the placeholders to be the leading nodes of the graph.
Every graph torch.export produces satisfies that. A graph whose ops fell back to
PyTorch does not: unlifting reinstates the module parameters as get_attr nodes
ahead of the user inputs, so a lone input at node index 2 reads
flat_dynamic_shapes[2] on a one-element list. Measured on an L40S with the same
model compiled twice:
converted fallback
[0] placeholder x [0] get_attr linear_weight
[1] get_attr ..._engine [1] get_attr linear_bias
[2] call_function execute_engine [2] placeholder x
[3] call_function getitem [3] call_function aten.linear.default
[4] output [4] output
save -> OK save -> IndexError
Both have one placeholder, dynamic_shapes {'x': {}} and a one-element
flat_dynamic_shapes; only the placeholder's node position differs. That is also
why the failure surfaces as a bare IndexError: make_constraints()' own length
check, len(flat_dynamic_shapes) == num_placeholders - num_lifted_inputs, passes.
create_trt_exp_program() now normalizes its graph to the placeholders-first
shape torch.export emits before computing range_constraints. Placeholders take
no arguments, so hoisting them cannot break the topological order, and
preserving their relative order leaves the forward signature, in_spec and the
InputSpec ordering unchanged; the helper returns immediately when the
placeholders already lead, so a fully converted graph is untouched. Fixing this
in torch instead -- indexing by placeholder ordinal -- would be the better
repair, but torch._export.non_strict_utils is a private API and the ordering
contract, though unstated, is the caller's to meet.
Not fixed in transform(): the non-legacy exporter retraces the inlined module
through torch.export.export() and is unaffected, and transform() is also
reachable from refit and executorch paths that should not change.
The regression test needs neither a GPU nor a TensorRT build: an exported
module reproduces the get_attr-before-placeholder node order exactly, so
create_trt_exp_program() can be driven directly with a Dim spec. The one-word
"mis-wired" -> "miswired" edit in the same file is unrelated: it is a
pre-existing typo the repo's own typos hook rejects, which blocks any commit
touching this file.
Testing (T4 ipp1-2023 and L40S a1u1g-mil-0572, driver 595.58.03, identical
stacks, -n 1, full models/ runs on both arms, test_hf_gqa_model.py ignored):
* The standalone reproducer above prints "save -> OK" for both the converted
and the fallback row on both arms; before, the fallback row raised
IndexError on both.
* models/ on the T4: 266 collected, 222 passed / 19 failed / 25 skipped ->
228 passed / 13 failed / 25 skipped. Two of the six recovered tests are
this fix (test_save_load_input_objects_retrace_false and
test_save_load_retrace_true_legacy_true_dynamic); neutralising the fix at
this tip leaves the other four passing, so they belong to commits that
landed after the baseline was taken.
* models/ on the L40S: 266 collected, no pre-existing test changes status.
The only difference on either arm is the new test appearing as a pass.
* The 3 view_as_real refit failures and the 10 remaining SM 7.5 failures are
untouched, and no test regressed on either arm.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tp5uiuc
force-pushed
the
fix/serde-placeholder-ordering
branch
from
August 30, 2026 04:40
1358631 to
07743d7
Compare
Collaborator
Author
CI summary
Why these are not from this PR: the same Worth stating explicitly for this PR, since it changes |
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
1 participant
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.
What — Hoists every placeholder to the front of the node list before
make_constraints()runs,preserving their relative order.
Why —
torch_tensorrt.save(..., retrace=False)raises for any graph that fell entirely backto PyTorch, on any device:
make_constraints()walksenumerate(gm.graph.nodes)and indexesflat_dynamic_shapesby thenode index, so it carries an unstated precondition: placeholders must be the leading nodes. For some rewritten graphs, this is not satisifed.
How — Hoist placeholders to the front of the node list before the constraints are computed,
keeping their relative order, so the precondition holds for exporter-rewritten graphs too.
Testing — Reproduced on an L40S (SM 8.9) with
torch_executed_opsforcing full fallback andno capability guards involved: a normal compile saves fine, forcing
linearto PyTorch raises.Confirmation sweep, both arms: 2
models/failures closed, no status change on the L40S.🤖 Generated with Claude Code