Skip to content

add torch-tensorrt-executorch-delegate wheel build#4398

Open
lanluo-nvidia wants to merge 4 commits into
mainfrom
lluo/executorch_wheel_build
Open

add torch-tensorrt-executorch-delegate wheel build#4398
lanluo-nvidia wants to merge 4 commits into
mainfrom
lluo/executorch_wheel_build

Conversation

@lanluo-nvidia

Copy link
Copy Markdown
Collaborator

Description

export EXECUTORCH_SOURCE_DIR=/home/lanl/git/executorch
export EXECUTORCH_ROOT=/home/lanl/git/executorch

build the torch-tensorrt-executorch-delegate.whl

python -m pip wheel
--no-build-isolation
--no-deps
--wheel-dir dist
packaging/executorch_delegate

install the wheel

python -m pip install --no-deps --force-reinstall
dist/torch_tensorrt_executorch_delegate-*.whl

export the pte

python /home/lanl/git/py312/TensorRT/examples/torchtrt_executorch_example/export_static_shape.py

load the pte

python examples/executorch_reference_runner/load_model.py
--model_path=/path/to/model.pte

Fixes # (issue)

Type of change

Please delete options that are not relevant and/or add your own.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes
  • I have added the relevant labels to my PR in so that relevant reviewers are notified

@meta-cla meta-cla Bot added the cla signed label Jul 11, 2026
@github-actions github-actions Bot added component: tests Issues re: Tests component: build system Issues re: Build system component: api [Python] Issues re: Python API labels Jul 11, 2026
@github-actions github-actions Bot requested a review from zewenli98 July 11, 2026 09:49
@narendasan

Copy link
Copy Markdown
Collaborator

@lanluo-nvidia the new package should be in

//py
-> torch-tensorrt
-> torch-tensorrt-executorch-delegate

See: https://packaging.python.org/en/latest/guides/packaging-namespace-packages/

@shoumikhin shoumikhin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this. The core idea is right. You build _portable_lib from source and pull TensorRTBackend into the same module that holds the backend registry, so the two always share one copy of the ExecuTorch core. That is the correct way to fix the undefined symbol crash people hit today. I checked the linking part and it holds up: one registry, no duplicate core, and the whole-archive flag is actually needed.

Comment thread MODULE.bazel
# latest commit in release/1.3 branch
commit = "6118688a095fd9697224f5cad72ce42db641c9cd",
# PyTorch 2.14-compatible ExecuTorch main commit
commit = "a6d812a082df57898b8608f56c867140cc9da32c",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this change never takes effect in CI, so the build still uses the old ExecuTorch.

packaging/pre_build_script.sh line 84 rebuilds this whole file from a template:

cat toolchains/ci_workspaces/MODULE.bazel.tmpl | envsubst > MODULE.bazel

That template (toolchains/ci_workspaces/MODULE.bazel.tmpl) still points at the old commit 6118688. So when CI runs, it overwrites your edit and goes back to the old version, and that old version is what the delegate gets compiled against. It also means it works on your machine but not in CI.

Please put the new commit in toolchains/ci_workspaces/MODULE.bazel.tmpl (and the Windows template too if there is one). The root MODULE.bazel is generated, so it is cleaner to leave it alone and only edit the template.

.github/scripts/verify-executorch-reference-runner.sh "${RUNNER_TEMP}/torchtrt-python.pte"

# Build the no-compile-for-users Python runtime/delegate wheel.
python -m pip wheel --no-build-isolation --no-deps --wheel-dir dist packaging/executorch_delegate

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this step builds the delegate wheel, but the wheel build needs a file that this job never creates, so it will fail here.

The delegate's setup.py calls torchtrt_version(), which reads py/torch_tensorrt/_version.py. That file is not checked in. It is written only by the main setup.py. This job runs bazel build //:libtorchtrt (C++ only) and never runs the main setup.py, so the file is missing and torchtrt_version() raises RuntimeError("Could not determine the Torch-TensorRT version"). Anyone following the README to build the wheel on its own hits the same error.

A simple fix is to fall back to version.txt and the env override when the file is not there:

def torchtrt_version() -> str:
    if value := os.getenv("TORCH_TENSORRT_EXECUTORCH_DELEGATE_VERSION"):
        return value
    version_py = REPO_ROOT / "py/torch_tensorrt/_version.py"
    if version_py.exists():
        if m := re.search(r'__version__\s*=\s*["\']([^"\']+)', version_py.read_text()):
            return m.group(1)
    return (REPO_ROOT / "version.txt").read_text().strip()


import pytest

RUNTIME_PATH = Path(__file__).parents[3] / "py/torch_tensorrt/executorch/runtime.py"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix: this test never runs in CI, so it will not catch regressions.

The ExecuTorch test group in tests/py/utils/ci_helpers.sh lines 157 to 162 does cd tests/py/dynamo and then runs pytest executorch/, so it only picks up tests in tests/py/dynamo/executorch/. This file is in tests/py/executorch/, a different folder that nothing runs.

On top of that, the test replaces the real runtime with a fake (FakeRuntime), so even if it did run it would not touch the tricky new code: activate(), the sys.modules swap, or the failure path.

Please move it under tests/py/dynamo/executorch/, and add a few plain Python tests (no GPU needed) that use fake modules to check: calling activate() twice is safe, it raises DelegateCompatibilityError when the stock runtime was already loaded, and it does not leave a leftover data_loader entry when the native import fails.

return existing
try:
data_loader = importlib.import_module(__name__ + ".data_loader")
sys.modules[_DATA_LOADER_NAME] = data_loader

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix: this can leave Python half set up if the native import fails.

activate() puts data_loader into sys.modules first, then imports _portable_lib. If that second import fails (the same crash this wheel is meant to fix), the error is raised but the data_loader entry is left behind, and from then on anything importing the normal ExecuTorch data_loader gets your version instead.

Import both first, and only register them once both succeed:

try:
    data_loader = importlib.import_module(__name__ + ".data_loader")
    native = importlib.import_module(__name__ + "._portable_lib")
except ImportError as error:
    sys.modules.pop(_DATA_LOADER_NAME, None)   # do not leave a half state
    raise DelegateCompatibilityError(...) from error
sys.modules[_DATA_LOADER_NAME] = data_loader
sys.modules[_NATIVE_NAME] = native
sys.modules.pop(_WRAPPER_NAME, None)

Separate note, not a change request: the existing.__name__ check just above is actually correct and safe to call more than once. I built a small single-phase C extension to confirm its __name__ comes out as the full pkg._portable_lib, so repeat calls do not throw. Worth leaving that one as is.

model_path = Path(path)
if not model_path.is_file():
raise FileNotFoundError(f"ExecuTorch model not found: {model_path}")
return Program(_runtime().load_program(model_path.read_bytes()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix: small lifetime question. load_program(model_path.read_bytes()) reads the file into a bytes object and passes it straight in, without keeping a reference to it. If ExecuTorch points at that memory instead of copying it, the bytes get freed right after this line and you get a use after free.

Can you check whether load_program copies the data? If it does not, keep the bytes alive by storing them on the returned Program, for example self._data = data.

)

output = pathlib.Path(self.get_ext_fullpath(ext.name)).resolve()
if output.exists():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: if output.exists(): return skips the build when the .so is already there. On a clean CI run that is fine, but on a local rebuild it can ship an old .so from a previous build without telling you. Consider a build stamp, or just remove the early return.

if existing is not None:
if existing.__name__ != __name__ + "._portable_lib":
raise DelegateCompatibilityError(
"ExecuTorch's stock runtime was imported first. Import "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the docstring implies importing torch_tensorrt.executorch.runtime reserves the slots, but activation is lazy and runs the first time you call load(). So the real rule is: call a delegate function before you import executorch.runtime. Please reword that, and maybe also have the check look at _WRAPPER_NAME.

export EXECUTORCH_SOURCE_DIR="$(dirname "${executorch_cmake_location%%:*}")"
export TensorRT_ROOT=/path/to/TensorRT

python -m pip install executorch==1.3.1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the README says pip install executorch==1.3.1, but MODULE.bazel now targets main for the 2.14 ABI. Please make the doc match the version you actually build against.

requires = [
"setuptools>=68",
"wheel>=0.40",
"torch",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: torch is listed here without a version. It does not matter in CI because CI uses --no-build-isolation, but a normal build would pull the newest torch into a clean build environment and mismatch the ABI. Please pin it, or say clearly that --no-build-isolation is required.

Comment thread .gitignore
/external/
build-executorch/
packaging/executorch_delegate/build/
packaging/executorch_delegate/dist/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this adds a trailing blank line at the end of the file.

@shoumikhin

Copy link
Copy Markdown
Contributor

One more note, on a file this PR does not change so I cannot leave it inline. In cpp/src/torch_tensorrt/executorch/TensorRTBackend.cpp around line 653, the result of register_backend is stored in kRegistered but never checked, so if registration fails you find out much later with a confusing "Backend TensorRTBackend is not registered" message. It would help to notice the failure at registration time. One thing to watch out for: this runs very early, before logging is set up, so do not call ET_LOG here. A safer option is to save the result and check it the first time the backend is used.

@lanluo-nvidia lanluo-nvidia marked this pull request as ready for review July 15, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed component: api [Python] Issues re: Python API component: build system Issues re: Build system component: tests Issues re: Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants