Repository navigation
Delete deprecated exir.capture API - #23592
krisnaparahita wants to merge 4 commits into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23592
Note: Links to docs will display an error until the docs builds have been completed.
|
|
Hi @krisnaparahita! Thank you for your pull request and welcome to our community. Action RequiredIn 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. ProcessIn 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 If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
This PR needs a
|
|
Thanks for putting up the fix, @krisnaparahita! While the CI workflows run, could you sign EasyCLA? |
|
Hi, |
|
/easycla |
|
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 |
6950f37 to
7c49f3c
Compare
Remove the legacy capture()/_capture_legacy_do_not_use/CallSpec/CaptureConfig API and the unused _unlift helper. EdgeCompileConfig and ExecutorchBackendConfig stay in exir.capture._config (widely imported). patch_forward moves to exir.capture._utils. Migrate the two remaining exir.capture() callers in test_pass_infra to torch.export + to_edge and update stale docs. Fixes pytorch#18125
f4c3eb6 to
89ad6e8
Compare
Done tidy up commit history. thanks |
Summary
Fixes #18125.
The issue explicitly requests deletion of the already-deprecated
exir.captureAPI. This intentionally removes that public API, its legacy entry point,CallSpec, andCaptureConfig; external callers that still import them must migrate totorch.export.exportandexir.to_edge.Delete the obsolete capture and unlift implementations while retaining
EdgeCompileConfigandExecutorchBackendConfig. Preservepatch_forwardunchanged in_utils, update its callers and Buck target, migrate the two pass-manager tests totorch.export.exportplusto_edge, and update documentation examples. The migrated tests retain the original pass transformations, modified/result-type assertions, and invalidcall_methodrejection assertion.Test plan
.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_compileon 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.patch_forwardsmoke check verified replacement and restoration offorward, 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/cpusucceeded 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 becauseparameterizedwas 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