fix: keep PyTorch-executed weights on the device when the mutable module offloads - #4647
fix: keep PyTorch-executed weights on the device when the mutable module offloads#4647tp5uiuc wants to merge 2 commits into
Conversation
…ule offloads
A hybrid graph module does not own its weights. torch.export.export keeps the source
module's Parameter objects rather than copying them, ExportedProgram.module() re-registers
those same objects, and both partitioners share attributes by setattr -- so the compiled
module and the module it was compiled from hold the identical Parameter instances, and
nn.Module.to() rebinds .data on them. Moving one moves the other.
deallocate_module() is a .to("cpu"), and MutableTorchTensorRTModule called it
unconditionally in three places. Any graph the partitioner did not hand to TensorRT in full
therefore had its PyTorch-executed weights dragged off the device while their activations
stayed on it:
RuntimeError: Expected all tensors to be on the same device, but got mat1 is on
cuda:0, different from other tensors on cpu (wrapper_CUDA_addmm)
The TensorRT submodules can spare their weights -- those live inside the engine -- which is
why a fully converted graph never noticed.
This is not a recent regression. pytorch#3418 added the same unconditional deallocate at all four
call sites; pytorch#3736 later put it behind the user-facing offload_module_to_cpu setting in
compile(), and only there. update_refit_condition(), refit_gm() and the static load() kept
offloading on every call regardless of what the user asked for. It surfaced now during a
triage on a card whose capability guards push addmm into PyTorch, which makes partial
fallback the common case; forcing the same fallback with torch_executed_ops reproduces it
byte-for-byte on an L40S, so it is neither card- nor backend-specific.
* Both halves are fixed together, because either alone leaves the bug reachable. The
three missed sites now consult offload_module_to_cpu the way compile() already did,
which is what stops an offload nobody asked for. And every offload now puts the
PyTorch-executed submodules back on the target device afterwards, mirroring what
compile_module already does at the end of a compile -- without that,
offload_module_to_cpu=True still strands a partial-fallback model.
* All three unguarded sites could strand weights, each by a different route.
update_refit_condition() runs the source model, offloads, and on matching output marks
the module LIVE, so the next forward hits CPU weights with no refit having run at all.
refit_gm() offloads after an in-place refit that deliberately left those weights on the
device; on second and later refits it offloads weights it never hoisted, since only
exp_program.module() is moved to the device there. load() offloads after re-exporting,
and nothing afterwards re-creates the graph module or re-attaches its parameters --
save() pickles the source model and the graph module together, so pickle memoisation
preserves the shared-Parameter identity across the round trip.
* Rejected alternatives. Gating alone is a smaller change but leaves
offload_module_to_cpu=True broken for every partial-fallback model. Teaching
deallocate_module() to skip the parameters a graph module still owns is the most
general, but it is a shared utility with eight call sites and would duplicate the
placement compile_module already performs.
* Not fixed here: with use_fast_partitioner=False the PyTorch-executed nodes stay in the
parent graph and nothing ever places them on the device, so offload_module_to_cpu=True
strands them in torch_tensorrt.compile() too. That is a separate, pre-existing gap in
the compiler rather than in this module, and the helper deliberately mirrors the
placement compile_module does instead of half-covering it.
Testing (L40S / SM 8.9 and T4 / SM 7.5, driver 595.58.03, identical stacks):
* test_mutable_torchtrt_module.py with -n 0. On SM 8.9, with addmm and linear forced
into PyTorch it goes from 6 passed / 5 failed to 13 passed; with no forcing at all it
goes from 11 passed to 13 passed. The five failures before the change are test_resnet18,
test_save, test_resnet18_modify_attribute, test_custom_model_with_kwarg and
test_custom_model_with_inplace_init, every one of them on the mixed-device addmm.
* Two new tests. Both force the fallback with torch_executed_ops so they run on any GPU,
refit, and then assert the device of every parameter and buffer rather than only that
the forward did not throw. Each also asserts the graph really did keep something in
PyTorch, so a future lowering change cannot quietly make them vacuous -- that guard
fired during development, when addmm alone turned out not to fall back. Run against the
unfixed module both fail with the reported error.
* The offload still offloads. With offload_module_to_cpu=True, everything TensorRT
absorbed is on the CPU after a refit and only what the graph module still executes in
PyTorch stays on the device. That is asserted by the second new test, not just observed.
* Full runtime/ module on both arms, every test accounted for (pass + fail + skip ==
collected, 220 on each). Zero pass -> fail and zero pass -> skip on either arm.
SM 8.9: every one of the 218 previously recorded tests keeps its status exactly, plus
the two new tests passing. SM 7.5: the nine tests in this file that were failing all
pass, five of them because of this change and four that other changes already on this
branch had fixed; the sweep also shows five weight-streaming tests recovering, which
this commit cannot have touched -- it changes one module that test file never uses.
The three failures left on each arm are test_aliased_io x2 and
test_dynamic_workspace_allocation, identical on both arms and unrelated to this work.
…nverted model test_bert_base_uncased_cpu_offload compiles BERT with offload_module_to_cpu=True and then asserts the source model is entirely on the CPU. That is not something the setting can promise. A compiled hybrid module does not own its weights: torch.export keeps the source module's Parameter objects rather than copying them, ExportedProgram.module() re-registers those same objects, and both partitioners share attributes by setattr, so the compiled module and the module it was compiled from hold the identical Parameter instances. compile() offloads with deallocate_module(gm), and compile_module then puts every submodule the partitioner did not hand to TensorRT back on the target device -- which moves the source model's weights back with them. That re-attach is deliberate. 4e31e2d extended it to update_refit_condition(), refit_gm() and load() precisely because without it a graph that keeps a weight-carrying op in PyTorch has those weights stranded off-device and dies with "Expected all tensors to be on the same device". So the contract, for a model that does not fully convert, is: offload_module_to_cpu=True releases the weights TensorRT absorbed. The weights the compiled module still executes in PyTorch stay on the device, because they are the very same objects the source model holds and the compiled module needs them there. The test now asserts that instead of "the whole source model is on the CPU". This is the same answer 4e31e2d gave for the mutable module, and its test_offload_module_to_cpu_keeps_torch_executed_weights_on_device asserts it the same way. * The two halves cover each other. Every parameter lands in exactly one of them, so neither can go vacuous: on a full conversion the released set is every parameter, and on a graph that kept work in PyTorch the resident set is non-empty. * Parameters, not parameters and buffers. BERT registers position_ids and token_type_ids with persistent=False, so torch.export never lifts them into the graph module and compile() has no handle with which to offload them. They are still on the GPU after the compile on every card, including one where the model converted in full. * get_model_device() is left alone and the other offload tests still use it. It returns the device of the first parameter it finds and never checks that the rest agree, so the old assertion was really "is the first parameter on the CPU" and its answer depended on parameter iteration order -- which is why it held on SM 8.9 despite the two buffers above. Making it strict is a wider change, since tests/py/dynamo/conversion/harness.py depends on the current behaviour, and belongs on its own. * Rejected: changing the product so the offload really does leave the source fully on the CPU. That is what 4e31e2d removed, and it strands fallback weights. Also rejected: forcing the model to convert in full so the old assertion holds, which would leave the same assertion failing for anyone whose model falls back on any card. Not architecture-specific. Forcing a fallback with torch_executed_ops reproduces it on an L40S (SM 8.9) with no capability guard involved: the same compile leaves the model on the CPU when it converts in full, and on cuda as soon as one conv goes to PyTorch. Testing (T4 / SM 7.5 and L40S / SM 8.9, driver 595.58.03, identical stacks): * Measured the compile the test actually performs, on both arms, rather than inferring it. SM 8.9 converts BERT in full: one _run_on_acc_0, no PyTorch block, all 199 parameters released to the CPU. SM 7.5 converts none of it -- the FP32 GEMM guard from ce480a0 rejects BERT's GEMMs, no run of >= 15 supported ops survives min_block_size=15, and the whole model lands in a single _run_on_gpu_0 with 197 parameters resident and exactly two released, embeddings.position_embeddings.weight and embeddings.token_type_embeddings.weight, which constant folding had already absorbed into the block's five _frozen_param entries. On both arms the invariant asserted here holds with no exceptions. Note this is a total fallback on SM 7.5, not the partial one the triage recorded. * Full models/ module on both arms, before and after, with --ignore=models/test_hf_gqa_model.py (4 tests that pass on both arms and dominate the runtime). 266 collected every run, pass + fail + skip + xpass == 266 every run, no test added or dropped. SM 7.5: 9 failed / 228 passed / 25 skipped / 4 xpassed -> 8 / 229 / 25 / 4. Exactly one status change, this test, fail -> pass. SM 8.9: 4 / 231 / 27 / 4 -> unchanged. Zero status changes. What still fails on SM 7.5 is 3 view_as_real refit tests that fail identically on both arms and 5 cosine-similarity tests, both separate pre-existing items. * Not covered here: test_resnet18_cpu_offload in this file and in tests/py/dynamo/models/test_export_serde.py, and the same assert in runtime/test_002_lazy_engine_init.py and runtime/test_003_cross_compile_for_windows.py, all make the same assumption. resnet18 converts in full on both arms so they pass today; they will fail the moment it does not. test_hybrid_conv_fallback_cpu_offload already omits the device assertion entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4e69b47 to
97a04e5
Compare
| continue | ||
| if "_run_on_acc" in name: | ||
| continue | ||
| submodule.to(device) |
There was a problem hiding this comment.
This is maybe a behavior change, but it is needed to keep the code from breaking with the offload_module_to_cpu flag. It is needed because export share Parameter identity between the original model and the graph module. Having a .to("cpu") on the original model yanks the weights that _run_on_gpu_* needs to execute in PyTorch. Pinning these submodules back is actually the same placement compile_module already does here
TensorRT/py/torch_tensorrt/dynamo/_compiler.py
Lines 1428 to 1442 in 0fad50d
and so this restores an invariant. Without it we get errors in the MR description.
CI summary
Why these are not from this PR: the same This PR changes only |
What — Gates
deallocate_module()on the user-facingoffload_module_to_cpusetting atthe three call sites on ToT that ignores it. Also puts PyTorch-executed submodules back on the target
device after every offload. Adds a test pinning what the setting guarantees for a partially
converted model.
Why — A hybrid graph module does not own its weights.
torch.export.exportkeeps the sourcemodule's
Parameterobjects,ExportedProgram.module()re-registers those same objects, and bothpartitioners share attributes by
setattr— so the compiled module and the module it was compiledfrom hold identical
Parameterinstances. Ann.Module.to()rebinds.dataon them and so moving onemoves the other.
deallocate_module()is a.to('cpu'), so any graph the partitioner did not handto TensorRT in full had its PyTorch-executed weights dragged off the device while activations stayed
on it, leading to errors like this
A fully converted graph does not see this error, because TensorRT submodules keep their weights inside the
engine. Related PRs #3418 added the unconditional deallocate at all four call sites and
#3736 later gated it behind
offload_module_to_cpuincompile()only. The other three call sites ofupdate_refit_condition(),refit_gm()and the staticload()kept offloading regardless.How — Do both halves together: gate the three offload sites that ignored the setting, and put
PyTorch-executed submodules back on the target device afterwards. Doing only one of them means the bug is still open. The
offload_module_to_cputest now asserts what the contract actually guarantees for apartially converted model rather than that the whole source model lands on CPU.
Testing — Reproduced on an L40S (SM 8.9) with
torch_executed_opsforcing the fallback and no capability guards involved, so this is neither card- nor backend-specific. Confirmation sweep (T4 SM 7.5 + L40S, driver 595.58.03): 5runtime/and 1models/failures closed, no status change on the L40S.Cost / Gotchas — Behaviour change:
offload_module_to_cpu=Trueon a partially converted modelnow leaves PyTorch-executed weights on the device. That is what makes the model runnable, but a
caller relying on "everything ends up on CPU" will see a difference.
🤖 Generated with Claude Code