executorch: persist external .ptd weights and preserve lifted constant dtype/device#4404
executorch: persist external .ptd weights and preserve lifted constant dtype/device#4404Conarnar wants to merge 2 commits into
Conversation
| "(AOTInductor) backend." | ||
| ) | ||
| else: | ||
| cuda_specs = [CudaBackend.generate_method_name_compile_spec("forward")] |
There was a problem hiding this comment.
can we dynamic choose the method name, instead of statically use forward?
Also in the test lets add a multi-method one.
There was a problem hiding this comment.
At the moment save only supports a single exported program which to_edge_transform_and_lower will default to the method name forward.
| ) | ||
|
|
||
| out = tmp_path / "erfinv_cuda_fallback.pte" | ||
| torch_tensorrt.save( |
There was a problem hiding this comment.
is the model runnable now? Can we handle the data transfer between different delegate blobs?
shoumikhin
left a comment
There was a problem hiding this comment.
A few points from a closer look, most important first.
-
Dropped external CUDA weights.
_save_as_executorchonly callsexecutorch_program.write_to_file(f), but the CUDA backend emits its weights as external named data (save_data_externallyis True, taggedaoti_<device>_blob) that ExecuTorch serializes only throughwrite_tensor_data_to_file. Since that is never called, any residual partition that carries weights loses its blob and the.ptecannot load. The new test does not catch this becausecos(erfinv(tanh(x)))has no parameters, so please persist the external data next to the.pteand add a test model with weights in the fallback partition. -
The import guard is too narrow. The
try/exceptonly catchesImportErroron the module import, but the real CUDA compile runs later insideto_edge_transform_and_lowerviaCudaBackend.preprocesscallingtorch._inductor.aot_compilewithmax_autotuneand the Triton backends, which needsnvcc/ptxas. On a host that has the executorch cuda wheel but no cuda toolkit, export now throws where before the residual op stayed non-delegated andsaveproduced a working CPU.pte, so the docstring line about degrading gracefully only holds for the missing-import case. Consider gating the fallback onptxasavailability or making it opt-in, and narrowing the docstring. -
Always on with no opt-out. Appending
CudaPartitionerunconditionally means any model with even one non-TRT op now embeds aCudaBackendblob and requires a CUDA runtime at load, where before those ops stayed non-delegated and the.pteran on CPU. Most real models are not fully TRT-convertible, and this also silently turns onmax_autotuneautotuning at save time, so please add asave()kwarg to keep the portable CPU-fallback behavior. -
target_deviceis not propagated to the fallback. It is forwarded only toTensorRTPartitioner; the fallback is built ascuda_specs = [CudaBackend.generate_method_name_compile_spec("forward")]with notarget_device, soCudaPartitionerdefaults tocuda:0. If a user pins TRT tocuda:1the two delegates end up on different devices, so derive the fallbacktarget_devicefrom the samecompile_specs. -
The test proves composition, not runnability. It only deserializes the program and checks that
CudaBackendandTensorRTBackendappear in the delegate ids; it never builds aMethod, runs, or compares to eager. The mixed ATen-mode roundtrip is also still gated on thetensor_parser_atenfix (planned CUDA buffers get a CPU data ptr soMethod::initfails ingetDeviceFromPtr), so scope the assertion to "produces both delegates" and add a real load-run-allclose once that lands.
|
Always enjoy reading Anthony's comment
|
|
Thanks for working on this. The direction is right and I want to use it, but I Short versionThere are three separate things in this PR:
I would land (1) and (2) as one small PR with tests, and document the CUDA path 1. The
|
4a56e26 to
99bc336
Compare
99bc336 to
c9748c8
Compare
|
Thanks for turning this around — trimming to the two fixes + documenting the Should-fix
if executorch_program._tensor_data:
out_dir = os.path.dirname(os.path.abspath(file_path))
executorch_program.write_tensor_data_to_file(out_dir)
logger.info(
"Wrote external delegate weights (.ptd) to %s; point the runtime's "
"data-files at this directory to load them.", out_dir,
)
Nice-to-have LGTM once the should-fix items are addressed. |
| torch_tensorrt.save( | ||
| gm, "model.pte", output_format="executorch", | ||
| arg_inputs=inputs, | ||
| partitioners=[CudaPartitioner( |
There was a problem hiding this comment.
The docstring recipe is missing retrace=False (the .rst examples have it). Without it a copy-paste from here runs the default retrace=True path rather than the tested one — suggest adding retrace=False, next to arg_inputs=inputs. Since this ~15-line recipe is also duplicated in saving_models.rst (and the two already differ, gm vs trt_gm), consider trimming the docstring to a one-line summary plus a :ref: to the guide for a single source of truth.
| # Direct attribute access (no getattr default) so a future rename fails loud. | ||
| if executorch_program._tensor_data: | ||
| executorch_program.write_tensor_data_to_file( | ||
| os.path.dirname(os.path.abspath(file_path)) |
There was a problem hiding this comment.
Consider a breadcrumb so the required runtime step is discoverable — co-location is only a convention, so a program that needs the .ptd but isn't given it fails at load with a null-weights dereference and no hint:
if executorch_program._tensor_data:
out_dir = os.path.dirname(os.path.abspath(file_path))
executorch_program.write_tensor_data_to_file(out_dir)
logger.info(
"Wrote external delegate weights (.ptd) to %s; point the runtime's "
"data-files at this directory to load them.", out_dir,
)lift() built the placeholder's fake meta["val"] with torch.empty_strided from shape only, so a lifted non-fp32 or non-CPU constant silently got fp32/cpu meta. Pass dtype=lift_val.dtype and device=lift_val.device so the placeholder meta matches the source. Adds unit tests (bf16 dtype + CUDA device).
… delegation
The CUDA (AOTInductor) backend externalizes its weight constants, so save() must
write the .ptd next to the .pte or the runtime loads a null weights blob and reads
out of bounds. Keep that write, extracted into _write_external_tensor_data() and
guarded on _tensor_data directly (no getattr default) so a future rename fails loud
instead of silently skipping. Document that the runtime must be pointed at the .ptd
(co-location is a convention, not an automatic search).
Drop the cuda_fallback=True flag: save(partitioners=[CudaPartitioner(...)]) already
appends a CUDA catch-all after the TensorRT partitioner, so the flag only duplicated
that, and 'fallback' wrongly implied a downgrade rather than a second GPU delegate.
Document the partitioners= recipe in saving_models.rst and the save() docstring.
Composition-only test added (build, not load/run); a load-run test follows the ATen
device-pointer runtime fix.
Test-Plan: pytest tests/py/dynamo/executorch/{test_api,test_edge_cases}.py -m unit
c9748c8 to
06dd4e7
Compare
| tuple(lift_val.shape), | ||
| tuple([1] * len(lift_val.shape)), | ||
| dtype=lift_val.dtype, | ||
| device=lift_val.device, |
There was a problem hiding this comment.
detect_fake_mode() only retrieves the mode; it does not activate it, and cast(FakeTensor, ...) has no runtime effect. Consequently, this calls torch.empty_strided outside the detected mode and creates a real tensor on lift_val.device. For CUDA constants, saving can now allocate on or initialize the target GPU, potentially causing OOM or making an otherwise device-independent export require a live CUDA device. It also stores a real tensor in metadata associated with the graph's FakeTensorMode.
Could we construct this through the detected mode instead?
const_placeholder_node.meta["val"] = fake_mode.from_tensor(
lift_val, static_shapes=True
)This avoids real device allocation and preserves the source tensor's layout, unlike the synthetic all-ones stride. The regression test currently checks only dtype/device, which a real CUDA tensor also satisfies; please additionally assert isinstance(val, FakeTensor), that it belongs to fake_mode, and that its stride matches lift_val.stride().
| # non-CPU lifted constant silently gets fp32/cpu meta. | ||
|
|
||
|
|
||
| def _traced_gm_with_buffer(dtype, device): |
There was a problem hiding this comment.
Minor follow-up: _traced_gm_with_buffer creates self.c as an nn.Parameter, so this exercises the PARAMETER branch rather than the buffer branch described by the helper name/docstring. Could we rename it to _traced_gm_with_parameter, or register an actual buffer if buffer coverage is intended?
Description
Reworked per review (previously "executorch: fall back to the CUDA backend..."):
this PR is trimmed to two standalone fixes and drops the
cuda_fallback=Trueflag.
save(output_format="executorch")already appends caller partitionersafter the
TensorRTPartitioner, so the supported way to get a coalescedTensorRT + CUDA
.pteis to passpartitioners=[CudaPartitioner(...)](nowdocumented in
saving_models.rstand thesave()docstring). The flag onlyduplicated that — and "fallback" wrongly implied a downgrade rather than a second
GPU delegate — so it's removed. This PR keeps the two pieces that are real fixes:
External weights (
.ptd). The CUDA (AOTInductor) backend emits its weights asexternal named data, which ExecuTorch serializes only via
write_tensor_data_to_file—write_to_filedoes not persist it. Without this, apartition that carries external weights (e.g. a
CudaPartitionerdelegate) losesits blob and the
.ptecan't load.save()now writes the.ptdnext to the.pte(extracted into_write_external_tensor_data, guarded on_tensor_datadirectly so a future rename fails loud instead of silently skipping) and logs the
target directory, so a missing runtime data-file is an obvious breadcrumb rather
than a silent null-weights load. It's a no-op for TRT-only programs
(
_tensor_datais empty). Co-locating the.ptdis a convention — the runtimemust still be pointed at the data file(s) to load them. The docs also warn that the
CUDA backend names its blob per-device, so each coalesced model must be saved into
its own directory.
Lifted constant dtype/device (
_exporter.py).lift()built the liftedplaceholder's fake
meta["val"]withtorch.empty_strided(...)from shape only,defaulting it to
cpu/float32. A lifted non-fp32 or non-CPU parameter/buffertherefore got the wrong meta (e.g. a
FakeTensorDeviceMismatchError, cuda:0 vscpu, when a weighted op is excluded from TensorRT). Now it passes
dtype=lift_val.dtype, device=lift_val.device. Independent bug fix; can standalone.
Testing
tests/py/dynamo/executorch/test_api.py—test_lift_preserves_constant_dtype_device,parametrized over
(bfloat16, cpu)and(float32, cuda)(the CUDA case isruntime-skipped when no GPU), asserts
lift()keeps the lifted constant's dtypeand device.
tests/py/dynamo/executorch/test_edge_cases.py— unit tests for_write_external_tensor_data: writes the.ptdwhen external data is present,no-op when empty, and raises (fails loud) if
_tensor_datais absent.tests/py/dynamo/executorch/test_cuda_partitioner_composition.py(build-only;skips without a GPU / CUDA backend / nvcc):
test_erfinv_routes_to_cuda_backend—erfinv(no TRT converter) routes to theCUDA backend via
partitioners=[CudaPartitioner(...)]whiletanh/cosstay onTensorRT.
test_weighted_partition_persists_external_data— a weighted, TRT-excludedmmlands on the CUDA backend and its external
.ptdis written next to the.pte(asserted non-empty).
test_trt_only_writes_no_ptd— a TRT-only program writes no.ptdand has noCUDA delegate (exercises the real
write_to_file/_tensor_datapath).Composition tests are build-only (delegate composition + external-data persistence,
not load/run) and skip cleanly without a GPU/CUDA backend/nvcc; a load-run-allclose
test is deferred to the runtime follow-up below.
Follow-up
A coalesced ATen-mode
.ptecan be built but not yet loaded/run: memory-plannedCUDA buffers get a CPU data pointer (
tensor_parser_aten.cpphardcodes CPU), soMethod::initfails the CUDA backend's device check. That runtime fix (and theload-run-allclose test) will land separately.