Repository navigation
Do not assert on a graph output that carries no tensors - #22479
Open
msluszniak wants to merge 4 commits into
Open
msluszniak wants to merge 4 commits into
msluszniak wants to merge 4 commits into
Conversation
msluszniak
requested review from
JacobSzwejbka and
larryliu0820
as code owners
September 2, 2026 18:49
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22479
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit dabfb40 with merge base bf9bef3 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
Contributor
|
cc: @JacobSzwejbka since these changes touch memory planning. From my side, changes LGTM |
Contributor
Author
|
Nothing outstanding from my side here; @nil-is-all could you help this one find a second reviewer? |
Contributor
Author
|
@pytorchbot label "module: exir" |
pytorch-bot
Bot
added
the
module: exir
nil-is-all
added
the
release notes: exir
The memory planning verifier walks the placeholder and output nodes and records, for each one, whether the planner allocated it. A node with no tensor specs is skipped, and the skip happens before graph_output_allocated is set, so a graph whose output node carries only non-tensor values leaves that flag at None and trips "assert graph_output_allocated is not None". An output node like that is legitimate. A Vulkan partition of a dynamic-shape LLM ends up with one whose only results are symints (sym_size / et_vk.select_as_symint), and the graph is fully planned when the verifier rejects it. The neighbouring len(specs) == 0 branch already treats "nothing to allocate" as success for the all-const case; do the same here. Repro before this change: export any llama-family model with enable_dynamic_shape=True and backend.vulkan.enabled=True, e.g. qwen3 0.6B, and lowering fails with "graph_output_allocated not set".
msluszniak
force-pushed
the
ms/memory-planning-tensorless-output
branch
from
September 15, 2026 11:03
61227e9 to
ae8d1e0
Compare
executorch-triage
Bot
added
the
community: contribution
JacobSzwejbka
approved these changes
Oct 9, 2026
Contributor
|
@msluszniak I think we should be good to merge once the branch is updated. Thanks |
This branch has not been deployed
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.
Fixes #22477.
The memory planning verifier skips nodes with no tensor specs before it sets
graph_output_allocated, so a graph whose output node carries only non-tensor values leaves the flag atNoneand tripsassert graph_output_allocated is not Nonedespite being fully planned.An output node like that is legitimate: a Vulkan partition of a dynamic-shape LLM ends up with one whose only results are symints, and every such export currently aborts here. The neighbouring
len(specs) == 0branch already treats "nothing to allocate" as success for the all-const case, so this does the same.Test:
TestTensorlessGraphOutput::test_verifier_accepts_output_carrying_no_tensorsreproduces the assertion onmainand passes with the change. The rest ofexir/tests/test_memory_planning.py(38 tests) still passes.With this in place, a qwen3 0.6B Vulkan export with
enable_dynamic_shape=Truelowers and runs.cc @JacobSzwejbka @angelayi