fix(models): build spatial_shapes from Python ints under torch.compile - #1411
Open
JESUSROYETH wants to merge 1 commit into
Open
fix(models): build spatial_shapes from Python ints under torch.compile#1411JESUSROYETH wants to merge 1 commit into
JESUSROYETH wants to merge 1 commit into
Conversation
JESUSROYETH
requested review from
Borda,
SkalskiP,
isaacrob and
probicheaux
as code owners
August 31, 2026 01:44
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1411 +/- ##
=======================================
Coverage 86% 86%
=======================================
Files 114 114
Lines 14880 14880
=======================================
Hits 12835 12835
Misses 2045 2045 🚀 New features to boost your workflow:
|
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.
What this fixes
compile=Trueaborts the training step.Transformer.forwardbuildsspatial_shapeswithand Dynamo polyfills
torch._shape_as_tensorto return atorch.Sizeinstead of a tensor, sotorch.stackraises:Minimal reproduction, no rf-detr involved:
And through the public API:
torch._dynamo.config.suppress_errors = True(set atmodule_model.py:436) does not absorb it. That flag only swallows errors Dynamo itself raises; here the polyfill makes user code raise a realTypeError, so Dynamo propagates it. Measured both ways,TrueandFalse, same result.Reproduced on torch 2.9.1+cu129 (L4) and 2.13.0+cu130 (RTX 4060 Laptop), and at
dynamic=True,dynamic=Noneanddynamic=Falsealike, so this is not a recent regression and not a dynamic-shapes problem.Under the shipped default
multi_scale=Truethe gate atmodule_model.py:417-418disables compilation before this line is reached, so a defaultcompile=Truerun does not crash .. it just runs eager and logs one line. The crash needsmulti_scale=False.The fix
The branch that works already exists two lines above.
torch.export/ExecuTorch cannot tracetorch._shape_as_tensoreither, and #1142 handled that by buildingspatial_shapesfrom the Python-int(H, W)pairs the function already collects attransformer.py:292-295.torch.compileneeds the same branch for a different reason, so the condition now covers both:The
_shape_as_tensorform stays for every path that needs it. #1155 introduced it because TensorRT accepts the Constant it produces while aScatterND-producing form is rejected, and that path goes throughtorch.jit.trace, where both guards are false. Checked directly rather than assumed:is_compiling()is_exporting()_shape_as_tensor(unchanged)torch.jit.trace_shape_as_tensor(unchanged)torch.onnx.export(dynamo=False)_shape_as_tensor(unchanged)torch.exporttorch.compilegetattrguards the same way the existingis_exportingcall does, for torch versions that lack the attribute. The existing private fallback was named_not_exporting, which would read wrong on the second call, so it is now_tracer_absentwith a docstring covering both predicates. It has no other caller and is not exported.torch.compiler.is_compiling()was added in torch 2.3 andis_exporting()in torch 2.7 (checked against the upstream source for each release). Below torch 2.3 thegetattrfallback returnsFalse, so this branch cannot detecttorch.compilethere andcompile=Truekeeps raising the sameTypeErrordevelopalready raises — unchanged by this fix. There is no working substitute on torch<2.3: bothtorch._dynamo.is_compiling()andtorch._utils.is_compiling()are hardcodedreturn Falsestubs on torch 2.2.x rather than real predicates, so torch 2.2.x (this project's declared floor) is unaffected either way, the same as it was already unaffected byis_exporting().The same public call that raises on
developnow runs to the end:Tests
Two in
tests/models/test_transformer_onnx_spatial_shapes.py, the file that already owns this line:test_spatial_shapes_survives_dynamo_shape_as_tensor_polyfillreproduces the polyfill (torch._shape_as_tensorpatched to return atorch.Size) together withis_compiling() == True, runs the realTransformer.forwardand checks the output still matches the eager one. Parametrised over one and two feature levels, since that is what changes the shape of the sourcespatial_shapesis built from. Both cases fail ondevelopwith the exact production error and pass with the fix. Neither invokes the compiler, so they cost nothing in CI.test_spatial_shapes_compile_guard_is_false_under_torchscript_tracepins the guard that keeps the TensorRT path on the old formulation.The file's module-level
onnximport moved to a per-fixturepytest.importorskipinside the three fixtures that build an ONNX graph, so these two tests (and the pre-existingtest_level_start_index_correctness_two_levels) collect and run underci-tests-cpu.yml'strain,augment,cli,visualinstall set, which does not include the[onnx]extra.pytest tests/models/test_transformer_onnx_spatial_shapes.pyis 10 passed, and the wholetests/modelswith the CI markers is 935 passed / 6 skipped.pytest tests/export tests/training/test_module_model.pywith the same markers is 623 passed / 48 skipped / 1 failed, the failure beingtests/export/test_onnx_notes.py::_export_tiny_model, a pre-existing doctest failure ondevelopunrelated to this change.pre-commit run --all-filesclean, includingmypy --strict.Scope
This makes the compile path reachable. It does not claim a speedup: no throughput number can be produced until compilation runs at all, and the measurement belongs in #1410. What I can say from that side is that the workload is worth compiling, the training step issues ~4200-4800 CUDA kernels regardless of batch size, and on a
g2-standard-8L4 the GPU is busy only 19-46% of the wall time.