Split the four monoliths along their real dependency layers, and delete the code that was repeated - #35
Merged
Conversation
…te the code that was repeated The largest file was 4,963 lines and the integration module had 98 top-level functions with no section markers. Splitting those by concern is the obvious move and it is also the wrong one: the first attempt at mlx_lm produced eleven import cycles. The symbol graph says why it should have worked -- 107 symbols in 100 strongly-connected components, the largest of size two, so the module is very nearly a DAG and a clean split has to exist. The partition was cutting across its layering. What the layering actually is: _mlx_lm_plan_key calls apply_metile_to_mlx_lm, because tuning works by applying a configuration and measuring it. apply is therefore a mid-layer primitive rather than the entry point everything else sits above, and once it is placed there the assignment comes out with zero ordering violations. Each of the four splits was solved the same way -- build the graph, find the strongly-connected components that must stay together, then run a fixpoint that pushes callers above callees -- rather than chosen by name. Import paths are unchanged. Every split module re-exports what it used to expose, including the private names the tests reach for. The duplication that came out was mostly one shape: eighteen _read_config/_write_config pairs across seven backends, differing only in a cache path. Their real bodies collapse to two serialisation schemes, so they became six helpers on metile.runtime.cache, verified for round trip, cache miss, changed config shape, and the disable-cache environment variable. Eight compressed calibration functions and two ninety-one line group tuners went the same way, the tuners only after normalising both and confirming they were identical. Two clone groups were left alone. The _persistent_key functions and the *_dispatches family are cache identity and reporting code, where sharing would add injection points that silently restore another kernel's tuned configuration. That failure mode costs more than the duplication does. Splitting the packages broke thirty tests that patched a name on the module and relied on caller and callee sharing one namespace. The patch targets were not rewritten by hand: twenty-three of the forty-four names are bound in more than one module now, and patching the wrong one leaves a test that passes without exercising anything. tests/module_patching.py replaces every binding instead, which is also the only correct thing to do for shared mutable state whose writers and readers now sit in different modules. Also: kernels/ moves under metile/ so installing the package no longer claims the top-level kernels name; validate_pass_order is wired into the pass pipeline and its stale constraint corrected; the spent precision-label backfill is deleted. Largest file 4,963 -> 1,604. Clone groups 29 -> 25. Dead symbols 0. 738 passed, 57 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What this is
A structural cleanup. No behaviour changes: 738 passed, 57 skipped — the same counts as before the first commit in this branch — and
make check/make code-qualare clean.The splits
test_mlx_backend.py4,963integrations/mlx_lm.py4,715codegen/msl_emitter.py2,717compiler/lowering.py2,429backends/mlx_quantized.py1,870kernels/(top-level)metile/kernels/Import paths are unchanged. Each package re-exports what its module used to expose, including the private names tests reach for.
Why they aren't split by concern
The first attempt at
mlx_lmgrouped by concern and produced 11 import cycles. The symbol graph says that shouldn't happen — 107 symbols in 100 strongly-connected components, largest of size 2, so it is nearly a DAG. The partition was cutting across its layering.The thing that was backwards:
_mlx_lm_plan_keycallsapply_metile_to_mlx_lm, because tuning works by applying a config and measuring it. Soapplyis a mid-layer primitive, not the top. Placed there, the assignment comes out with zero ordering violations.All four splits were solved the same way — build the dependency graph, find the SCCs that must stay together, run a fixpoint pushing callers above callees — rather than chosen by name.
The duplication
Mostly one shape: 18
_read_config/_write_configpairs across 7 backends differing only in a cache path. Their real bodies collapse to two serialisation schemes, so they became 6 helpers onmetile.runtime.cache, verified for round-trip, cache miss, changed config shape, and theMETILE_DISABLE_DISK_CACHEpath.Also: 8 compressed-calibration functions, two 91-line group tuners (merged only after normalising both and confirming they were identical), 8
_print_tablecopies (byte-identical output verified), a duplicated matplotlib helper, and_decode_logitscopied from the library into a test.Clone groups 29 → 25. Dead symbols 0.
Two groups deliberately left
_persistent_keyand the*_dispatchesfamily are cache-identity and reporting code. Sharing them would add injection points that silently restore another kernel's tuned config — that failure mode costs more than the duplication.The test-suite accommodation — worth a look
Splitting the packages broke 30 tests that did
monkeypatch.setattr(pkg, "_x", ...), which worked when caller and callee shared one namespace.The patch targets were not rewritten by hand: 23 of the 44 names are now bound in more than one module, and patching the wrong one leaves a test that passes without exercising anything.
tests/module_patching.pyreplaces every binding instead — also the only correct option for shared mutable state whose writers and readers now sit in different modules. It is mutation-tested: degrading it to package-only patching reproduces exactly the failures it fixes.Also in here
validate_pass_orderis wired into the pass pipeline against the passes that actually ran. Doing so surfaced that one recorded constraint was backwards —double_buffer_k_loopalways runs andsplit_k_loopis its fallback, so any f16 GEMM over the 30KB threadgroup budget violated it. Corrected, withtests/test_pass_order.pycovering it and a source-level guard against a pass silently bypassing the recorder.backfill_precision_labels.pydeleted — a one-time migration, verified complete (--checkreports nothing to write, all 8 result files carry labels, nothing imports it).One pre-existing issue, not from this branch
test_agx_isa.py::test_nopping_an_instruction_removes_exactly_its_effectis flaky — it failed twice during this work and passed on re-run and on a clean checkout ofmain. Worth chasing separately.Deviation from plan
No compatibility shim for the old top-level
import kernels. A shim would reinstall the generic name and defeat the reason for moving it; at version 0.0.1 there are no external users.import kernelsnow fails cleanly.🤖 Generated with Claude Code