Repository navigation
Break the exir backend_api <-> program import cycle - #24802
Conversation
verifier.py imported EdgeCompileConfig from executorch.exir.capture._config at module scope. That runs exir/capture/__init__.py, which imports _capture.py and exir/program/_program.py, and both import back from the still half-initialised verifier. Normally exir/__init__.py loads capture first, so the cycle stays hidden. Under lazy imports (Cinder, Python 3.14) the first import can enter through verifier, e.g. via backends/xnnpack/xnnpack_preprocess.py, and fails with "cannot import name 'EXIRATenDialectVerifierBase' from partially initialized module". EdgeCompileConfig is now imported only for type checking, plus a local import at its single runtime use in _EXIREdgeDialectVerifier.__init__. Test plan: simulated lazy imports by deferring exir/__init__.py until first attribute access, then imported exir.verification.verifier and backends.xnnpack.partition.xnnpack_partitioner first; both failed before this change and succeed after. exir/verification/test/test_verifier.py and backends/xnnpack/test/ops/test_add.py pass. Authored with AI assistance (Claude Code).
Address review: the verifier.py-only fix still ran the cycle during verifier's own import (through the module-level EXIREdgeDialectVerifier() instance), left the exir.program and exir._serialize entry points broken, and stopped EdgeCompileConfig being importable from verifier. capture/__init__.py loads _capture.py, which imported exir.program and exir.verification.verifier at module scope, so any import of the leaf module capture._config pulled both back in. Those imports now live in the two functions that use them, and verifier.py is unchanged from main. The new test imports each entry point in a fresh interpreter with a bare executorch.exir package, so exir/__init__.py doesn't load capture first. It fails on main for all three modules and passes with this change. Authored with AI assistance (Claude Code).
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24802
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c56ce62 with merge base c3d2644 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
|
@claude review effort=max |
|
Claude finished @JakeStevens's task in 23m 7s —— View job The fix works for the Detailed reviewPR Review: #24802Summary
I reviewed only Testing
Backward Compatibility
Specific Comments
RecommendationApprove Nothing in |
pytest.ini ignores exir/verification/test/test_verifier.py, and that file's relative import needs a parent package, so OSS pytest never ran the test. It now lives in exir/tests/test_import_order.py, which OSS pytest collects, with a matching Buck target. Authored with AI assistance (Claude Code).
backend_api.py imports executorch.exir.program._fake_program, which runs exir/program/__init__.py and so _program.py. _program.py imported MethodProgramsPartitionerSpec and to_backend from the still half-initialised backend_api at module scope. Normally exir/__init__.py loads program first, so this only fails when backend_api is the first executorch.exir import, e.g. under lazy imports. _program.py uses those names only in EdgeProgramManager.to_backend, so the import moves there. backend_api is added to the import-order test. Authored with AI assistance (Claude Code).
a23d2ca to
121f6d6
Compare
Summary
Stacked on #24799. Until that lands this branch also contains its commits; review only the last commit, 121f6d6.
exir/backend/backend_api.pyimportsexecutorch.exir.program._fake_program, which runsexir/program/__init__.pyand so_program.py._program.pyimportedMethodProgramsPartitionerSpecandto_backendfrom the still half-initialisedbackend_apiat module scope:Normally
exir/__init__.pyloadsprogramfirst, so this only fails whenbackend_apiis the firstexecutorch.exirimport, for example under lazy imports (Cinder lazy imports on Python 3.14) when a backend importsto_backenddirectly. On main, that import order fails earlier on thecapturecycle fixed in #24799, which is why this is stacked._program.pyuses those two names only inEdgeProgramManager.to_backend, so the import moves into that method. Nothing imports them through_program.This PR was authored with AI assistance (Claude Code).
Test plan
Added
executorch.exir.backend.backend_apitoexir/tests/test_import_order.py(added in #24799). It fails with the error above without the_program.pychange and passes with it. The other entry points in that test still pass.python -m unittest executorch.exir.program.test.test_program executorch.exir.backend.test.test_backends_lifted executorch.exir.backend.test.test_partitioner executorch.backends.xnnpack.test.ops.test_addgives the same results with and without this commit, apart from the new import-order case. The shared failures come from my local runtime build, which lacksBackendWithCompilerDemoand some kernels such asaten::_embedding_bag.out.