Skip to content

fix(executorch): accept only portable names for external_engine_data - #4917

Closed
shoumikhin wants to merge 1 commit into
pytorch:mainfrom
shoumikhin:executorch-ptd-name-check
Closed

shoumikhin wants to merge 1 commit into
pytorch:mainfrom
shoumikhin:executorch-ptd-name-check

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What is wrong today

external_engine_data= names the .ptd file that holds the TensorRT engines. Its check refuses directories, drive prefixes and a trailing .ptd, but it accepts names that cannot become a normal file:

  • Windows device names, such as NUL, CON, COM1 or LPT1, in any case. On Windows these open a device, not a file, so the engines may be lost with no error.
  • Characters Windows does not allow in a file name: ? * | < > and the double quote, plus control characters.
  • Names too long for the file system. A 252 character name fails with "file name too long".

save() only fails when it writes the .ptd. By then the .pte is already on disk, and it cannot load without its engines.

Also, one error message covers every rule, so engines:v2 is told it has "a directory, a drive or the .ptd extension". And the code comment gives a reason for refusing . and .. that does not apply.

What this change does

The name must now:

  • use only ASCII letters, digits, ., - and _,
  • start with a letter, a digit or _,
  • be at most 251 characters, so <name>.ptd fits the usual 255 byte limit,
  • not end with .ptd, which ExecuTorch adds,
  • not be a Windows device name, with or without an extension.

Each kind of problem gets its own error message. The check still runs before anything is compiled or written, in export(), save(), the partitioner and the backend, so a bad name fails at once.

torch_tensorrt.save(trt_gm, "model.pte", output_format="executorch", external_engine_data="NUL")
# ValueError: external_engine_data must not be a Windows device name such as NUL or CON, got 'NUL'.

Names like model, my-model_v2.1 and aoti_cuda_blob (to share the CUDA backend's file) still work. Some names that worked on Linux before are now refused, such as ones with a space, a + or a non-ASCII letter. That is on purpose: a short portable name keeps a saved program usable on every system.

What was tested

On Linux x86-64 with the pinned ExecuTorch nightly:

  • test_external_engine_data.py, test_backend.py and test_partitioner.py: 101 passed.
  • With the old check and the new tests, the 14 new rejected names and the 2 updated message checks fail, as expected.
  • Nothing was run on Windows. The device name and character rules follow Microsoft's file naming rules.

@meta-cla meta-cla Bot added the cla signed label Oct 22, 2026
@github-actions github-actions Bot added component: tests Issues re: Tests component: api [Python] Issues re: Python API labels Oct 22, 2026
@github-actions
github-actions Bot requested a review from narendasan October 22, 2026 18:47
The .ptd name check refused directories, drive prefixes and a trailing .ptd,
but let through names that cannot become a file: Windows device names such as
NUL or CON, characters Windows does not allow (? * | < > and the double
quote), control characters, and names too long for a file system. save()
then failed only when it wrote the .ptd, after the .pte was already on disk,
and a device name might not fail at all. Its comment also gave a reason for
refusing "." and ".." that does not apply, and one message covered every
rule, so "engines:v2" was told it had a directory or a drive.

Now the name must use only ASCII letters, digits, ".", "-" and "_", start
with a letter, a digit or "_", and fit the 255 byte file name limit once .ptd
is added. It may not end with .ptd or be a Windows device name. Each kind of
problem gets its own message. This also refuses some names that worked on
Linux before, such as ones with a space, a "+" or a non-ASCII letter, so a
program saved on one system does not fail to save or load on another.
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: tests Issues re: Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant