Skip to content

Migrate to Graph.materialize_symints; stop passing raw SymInts#20199

Closed
laithsakka wants to merge 1 commit into
pytorch:mainfrom
laithsakka:export-D107938874
Closed

Migrate to Graph.materialize_symints; stop passing raw SymInts#20199
laithsakka wants to merge 1 commit into
pytorch:mainfrom
laithsakka:export-D107938874

Conversation

@laithsakka

@laithsakka laithsakka commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary:
reland plan for pytorch/pytorch#186272 due to executorch dependency have to stage changes !!

Updates the ARM backend passes (convert_full_like_to_full,
decompose_linear, decompose_select, insert_dynamic_padding,
rewrite_conv, size_adjust_input) and the ExportPass base to materialize
SymInt-typed FX-node arguments via Graph.materialize_symints /
create_size_node, so they no longer pass raw SymInts to
Graph.create_node and the new pytorch warning falls silent.

Test assertions in test_decompose_select_pass /
test_insert_dynamic_padding_pass / test_dynamic_shape_propagation are
updated to expect FX Nodes (with the SymInt stored in meta['val']).

This is the second of three diffs split from D107573548.

Differential Revision: D107938874

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani

Summary:
reland plan for pytorch/pytorch#186272 due to executorch dependency have to stage changes !!

Updates the ARM backend passes (convert_full_like_to_full,
decompose_linear, decompose_select, insert_dynamic_padding,
rewrite_conv, size_adjust_input) and the ExportPass base to materialize
SymInt-typed FX-node arguments via Graph.materialize_symints /
create_size_node, so they no longer pass raw SymInts to
Graph.create_node and the new pytorch warning falls silent.

Test assertions in test_decompose_select_pass /
test_insert_dynamic_padding_pass / test_dynamic_shape_propagation are
updated to expect FX Nodes (with the SymInt stored in meta['val']).

This is the second of three diffs split from D107573548.

Differential Revision: D107938874
@pytorch-bot

pytorch-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20199

Note: Links to docs will display an error until the docs builds have been completed.

❌ 67 New Failures, 139 Cancelled Jobs, 3 Pending, 1 Unrelated Failure, 26 Unclassified Failures

As of commit 148986d with merge base e0be283 (image):

NEW FAILURES - The following jobs have failed:

UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 10, 2026
@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Jun 10, 2026
@meta-codesync

meta-codesync Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@laithsakka has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107938874.

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@zingo

zingo commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Hi @laithsakka thanks for this, do you mind fixing the linting/mypy so this can be approved/merged? 🙏 We have some patches on it way that would look different if this gets merged so we are now planning to do them after. :)

@digantdesai digantdesai added the help wanted Extra attention is needed label Jun 17, 2026
@digantdesai

Copy link
Copy Markdown
Contributor

@claude review this PR.

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Claude finished @digantdesai's task in 5m 9s —— View job


PR Review: Migrate to Graph.materialize_symints

I reviewed the diff against origin/main, the changed passes, the new pass_base.py helpers, and the failing CI logs.

Summary

The change is well-motivated and the approach is sound: raw SymInts are no longer passed into Graph.create_node call_function args. Two materialization strategies are used, and the split is reasonable:

  • Passes that build nodes directly on graph_module.graph use graph.materialize_symints(...) (decompose_linear, decompose_select, rewrite_conv, size_adjust_input).
  • Passes that go through call_operator use the new call_size_operator_all helper on _ExportPassBase (convert_full_like, the test_dynamic_shape_propagation test pass).

The comments are thorough and the test updates correctly assert on FX Nodes with the SymInt preserved in meta['val'].

Blocking: lint / mypy (what @zingo asked about)

The Lint / lintrunner-mypy job fails with attr-defined on four sites, because the materialize_symints API does not exist in the torch version mypy resolves against:

  • backends/arm/_passes/size_adjust_input_pass.py:244
  • backends/arm/_passes/decompose_select.py:63
  • backends/arm/_passes/rewrite_conv_pass.py:628
  • backends/arm/_passes/decompose_linear_pass.py:69

Each graph.materialize_symints(...) call needs a # type: ignore[attr-defined]. The call_size_operator* helpers don't trip mypy because they route through self.call_operator. Fix this →

Runtime dependency on the pytorch pin

CI also shows real runtime failures, e.g. zephyr:

AttributeError: DecomposeLinearPass: 'Graph' object has no attribute 'materialize_symints'

This is consistent with the PR being the staged reland of pytorch#186272 — .ci/docker/ci_commit_pins/pytorch.txt is bumped release/2.12 → release/2.13, but jobs that don't pick up the new pin (and the locally-installed torch) still lack the API. This PR can't go green until the pinned torch actually exports materialize_symints / create_size_node. Worth confirming the landing order with the reviewers before merge. (The aten.transpose ... 'Dimname' and libkineto::DeviceInfo failures look unrelated to this change.)

Correctness notes (looks good, minor observations)

  • rewrite_conv_pass.py:610-631 — Good catch moving the fake-tensor computation before materialization, since target_op(...) expects ints/SymInts, not FX Nodes. conv_args index 4 correctly maps to pad given (x, weight, bias, stride, pad, dilation). ✅
  • size_adjust_input_pass.py:243-250 — The flatten → single materialize_symints → regroup via iter/next is correct and preserves per-slice arity. ✅
  • decompose_linear_pass.py:64-78 — Slicing all_sizes by n_in / n_w is correct; sharing one call lets repeated symbols hash-cons. ✅
  • pass_base.py call_size_operator — Returns plain int for static dims and an aten.sym_size.int proxy for dynamic dims; mirroring example_value only when present matches the surrounding meta convention. ✅
  • insert_dynamic_padding.py:39-45 — Now also treats ProxyValue entries in a list/tuple as dynamic, which is the right generalization. ✅

Nits

  • call_size_operator / call_size_operator_all are public-looking methods on _ExportPassBase but only emit aten.sym_size.int, which is an aten (not edge) op. Since callers here run on edge graphs, confirm that's intended (it appears to be — sym_size has no edge variant and is left to downstream handling). A one-line note on the helper would help future readers.
  • The two materialization paths (helper vs direct graph.materialize_symints) achieve the same goal differently. Not worth unifying, but a short comment pointing out why (call_operator context vs direct graph mutation) would reduce confusion.

Overall the logic is correct; the only true blocker is the mypy # type: ignore and the torch-pin landing dependency.
· branch export-D107938874

@digantdesai

Copy link
Copy Markdown
Contributor

Too many CI issues, rebase please?

@laithsakka

Copy link
Copy Markdown
Contributor Author

started new diff due to export issues https://www.internalfb.com/diff/D109610966

@laithsakka laithsakka closed this Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. help wanted Extra attention is needed meta-exported module: arm Issues related to arm backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants