[krisnaparahita]

Summary

Fixes #18125.

The issue explicitly requests deletion of the already-deprecated exir.capture API. This intentionally removes that public API, its legacy entry point, CallSpec, and CaptureConfig; external callers that still import them must migrate to torch.export.export and exir.to_edge.

Delete the obsolete capture and unlift implementations while retaining EdgeCompileConfig and ExecutorchBackendConfig. Preserve patch_forward unchanged in _utils, update its callers and Buck target, migrate the two pass-manager tests to torch.export.export plus to_edge, and update documentation examples. The migrated tests retain the original pass transformations, modified/result-type assertions, and invalid call_method rejection assertion.

Test plan

• Reviewed the complete 14-file diff (+57/-611). Repository-wide tracked-file searches, a hidden/ignored-file search outside .git, and Python AST import inspection found no dangling imports, calls, re-exports, or build references to the deleted APIs/modules. Remaining textual mentions are comments; imports of the retained config classes remain valid.
• python3 -m py_compile on all nine added/modified Python files: passed.
• git diff --check upstream/main...HEAD: passed.
• PATH=/tmp/executorch-review-venv/bin:$PATH lintrunner --data-path /tmp/executorch-review-lint -m upstream/main --skip MYPY: passed, using the repository's Flake8/plugins and UFMT (Black/usort) versions. The changed paths are outside the configured MYPY and Ruff-docstring scopes. An additional unscoped Ruff-docstring run reported legacy docstring findings; it is not a configured check for these paths.
• PYTHONPATH=src python -m unittest executorch.exir.tests.test_pass_infra: all 23 tests passed, including both migrated tests.
• A direct patch_forward smoke check verified replacement and restoration of forward, including restoration when the context raises: passed.

Local verification limitation: the development sandbox initially had no torch environment, so the previous assessment could not test-run this locally. For this review, pip install torch --index-url https://download.pytorch.org/whl/cpu succeeded in an isolated temporary environment (PyTorch 2.14.1+cpu), allowing the limited Python tests above. The full ExecuTorch development/native runtime environment was not provisioned. A targeted memory-planning test could not even import because parameterized was missing; no assertion from that test ran. The full memory-planning suite, end-to-end/native runtime tests, Buck build, and documentation build were not run. Please use CI and reviewer testing as the primary verification of broader runtime behavior.

The branch is unchanged from the existing fork commit a95dd46e; no additional code changes were needed. CLA acceptance must be confirmed by the repository's CLA check.

cc @JacobSzwejbka @angelayi @nil-is-all

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure

As of commit 33e95f0 with merge base 0fc43f6 ():

NEW FAILURE - The following job has failed:

• pull / android / build-android (gh)
  ModuleNotFoundError: No module named 'executorch.exir.capture._utils'

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

[meta-cla]

Hi @krisnaparahita!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: krisnaparahita / name: krisna (0e82307, 805d18c, ab65412)
• ✅ login: krisnaparahita / name: krisnaparahita (89ad6e8, e201155)

[github-actions]

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.

[nil-is-all]

Thanks for putting up the fix, @krisnaparahita! While the CI workflows run, could you sign EasyCLA?

[krisnaparahita]

https://code.facebook.com/cla

Hi,
Yes I have sign the EasyCLA

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[JakeStevens]

/easycla

[JakeStevens]

Can you please make sure that all accounts you have used are certified?

Or, update the commits to use only the certified account?

There are multiple commit authors that seem to be perhaps different profiles you have, if there is more than one, then they are tracked separately. this could be tripping it up

[krisnaparahita]

Can you please make sure that all accounts you have used are certified?

Or, update the commits to use only the certified account?

There are multiple commit authors that seem to be perhaps different profiles you have, if there is more than one, then they are tracked separately. this could be tripping it up

Done tidy up commit history.

thanks

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[nil-is-all]

CLA looks good, running CI

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[krisnaparahita]

Can you please make sure that all accounts you have used are certified?

Or, update the commits to use only the certified account?

There are multiple commit authors that seem to be perhaps different profiles you have, if there is more than one, then they are tracked separately. this could be tripping it up

Please Review.

[krisnaparahita]

Hi @JakeStevens could you please review?

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[krisnaparahita]

Hi @JacobSzwejbka could you please review it?

[nil-is-all]

@krisnaparahita could you resolve the merge conflict and update the branch? @JacobSzwejbka will review this once the branch is updated.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[krisnaparahita]

@krisnaparahita could you resolve the merge conflict and update the branch? @JacobSzwejbka will review this once the branch is updated.

yes I have updated and resolved the conflict @nil-is-all @JacobSzwejbka

[mergennachin]

@claude /pr-review

[claude]

Claude finished @mergennachin's task in 13m 50s —— View job

---

This PR removes the deprecated exir.capture API, but I'm requesting changes: the rewritten control_flow and pattern-partitioner docstrings now point users to torch.export usages that don't work, and the deletion leaves capture-only code behind, including the public ExirDynamoConfig export.

Full review

PR Review: #23592

Summary

The PR deletes exir.capture, _capture_legacy_do_not_use, CallSpec, CaptureConfig and _unlift.py. It moves patch_forward to exir/capture/_utils.py and migrates the two pass-manager tests to torch.export.export + to_edge. Two rewritten docstrings now describe usage that doesn't work, and the removal is incomplete. All 14 changed files were reviewed. This was a static review; no tests were run.

API Design

1. exir/control_flow.py:45-49 and :183: the new docstrings say these helpers work with torch.export.export(). They don't.

   • shape (:76-78), while_loop (:157-160) and _make_submodule (:108) only record graph nodes when DispatchTracer.get() is set.
   • torch.export never installs ExecuTorch's DispatchTracer. Only dispatch_trace did, and it was reached through the deleted _capture_legacy_do_not_use.
   • Trigger: a user follows the docstring and exports a module that calls control_flow.while_loop. No while_loop node is recorded. The Python while runs during tracing, so export either fails on the data-dependent condition or unrolls the loop.
   • The module docstring at :10-12 still says "captured by DispatchTracer", which contradicts the new line 45.
   • Fix: don't point this text at torch.export. Either say the helpers only work under the legacy DispatchTracer, or point users to torch.cond / torch._higher_order_ops.while_loop. Fix this →

2. exir/backend/canonical_partitioners/pattern_op_partitioner.py:120 and docs/source/compiler-custom-compiler-passes.md:449 now recommend plain torch.export as the source of pattern graphs.

   • torch.export.export returns an ATen-dialect program. The sentence, though, is about getting an accurate edge-dialect graph.
   • SubgraphMatcher (:143) matches on node targets, so torch.ops.aten.* pattern nodes won't match the exir_ops.edge.* nodes of the edge graph being partitioned.
   • The old exir.capture wording had the same gap, but this line is being rewritten anyway.
   • Fix: recommend the graph of to_edge(torch.export.export(...)).exported_program(). That is how backends/xnnpack/partition/graphs/bilinear_2d.py:37-40 builds its patterns. Fix this →

3. exir/__init__.py:22,43 still exports ExirDynamoConfig, and other capture-only code stays behind.

   • ExirDynamoConfig was consumed by CaptureConfig._dynamo_config and dynamo_trace. from executorch.exir import ExirDynamoConfig still works but now configures nothing. Removing it later would be a second BC break, when this PR is already an intentional one.
   • These now have no callers; they existed only for the deleted _capture.py:
     • dispatch_trace (exir/tracer.py:776)
     • flatten_output (:608)
     • HackedUpExportedProgramDONOTUSE (exir/program/_program.py:325, which still takes a call_spec)
   • dynamo_trace is now called only from dispatch_trace (:796).
   • With dispatch_trace uncalled, no in-repo path creates a DispatchTracer; it is only constructed in flattened_dispatch_trace (exir/tracer.py:585). That makes the tracer paths in exir/control_flow.py unreachable as well.
   • Stale references remain:
     • exir/program/_program.py:463 ("After exir.capture is gone I will clean up…")
     • exir/lowered_backend_module.py:359 (CallSpec(None, None))
     • CONTRIBUTING.md:50 ("capture - Program capture.")
   • Fix: remove at least the ExirDynamoConfig export here. Delete the rest here or in a tracked follow-up. Fix this →

Infrastructure

4. exir/capture/BUCK:19: :lib depends on :utils, but exir/capture/__init__.py never imports _utils.
   • Before this PR, the :lib → :capture edge was real because __init__.py imported _capture.
   • Now the edge exists only so two targets get patch_forward transitively through //executorch/exir:lib:
     • memory_planning (exir/tests/targets.bzl:201-218)
     • exported_module (test/end2end/targets.bzl:12-27)
   • Neither target declares //executorch/exir/capture:utils. Dropping the dep from :lib, where it looks unused, would break both.
   • patch_forward has only test callers, and exir/emit/test/test_emit.py:79-96 already has an identical copy. It doesn't need its own module in the shipped capture package.
   • Fix: move it into shared test code and remove the test_emit.py copy. At minimum, declare :utils directly on the two test targets and drop it from :lib. Fix this →

Code Quality

5. exir/tests/test_pass_infra.py:40-46: the new module-level _FunctionModule wrapper doesn't match the file's pattern.
   • The sibling PassManager tests at :131-141 and :177-187 define an inline torch.nn.Module and call to_edge(export(M(), ..., strict=True)).
   • Fix: give the two migrated tests the same structure around their f and delete the wrapper. Fix this →

Recommendation

Request Changes

Fix the two docstrings (1 and 2) before merging. As written, the PR replaces references to a removed API with guidance that doesn't work, and both are small edits. Removing the ExirDynamoConfig export (3) belongs in this PR, because the BC break is already intentional. The rest of 3, and 4 and 5, can land here or in a follow-up.

Review tasks

• Gather context (diff, commit log, changed files)
• Run three independent reviewers against the review checklist
• Consolidate findings
• Fact-check findings (one candidate dropped: the removed functions already raised deprecation warnings, and the release-notes label has already been requested)
• Post final review