You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Depends on: INF-1 (#1550) · Blocks: all of Phase 2+ (permanent gate)
Context: The rewrite keeps the legacy mace/ package byte-frozen in-tree as a live numerical oracle while the new stack grows in packages/ (mace-core, mace-torch, mace-jax, mace-launcher, created by INF-1 (#1550)). The fresh-start's integrity comes from this guard, not from deletion: as long as a legacy class is physically unreachable from the new zone, it cannot be subclassed, re-exported, or imported — so v1 reproduces legacy behaviour without inheriting legacy structure, and the oracle stays trustworthy. This ticket turns that from a review aspiration into three CI-enforced mechanisms: a static import-linter contract, a runtime sys.addaudithook that closes the dynamic importlib loophole the static check cannot see, and a meta-lint that blocks structural copying by name (e.g. ScaleShiftMACE from mace/modules/models.py:447, the interaction_classes registry at mace/modules/__init__.py:71, AtomicData from mace/data/atomic_data.py:28). Once merged, the guard gates every PR permanently — every Phase 2+ ticket writes code under it.
Interface & constraints:
The invariants being enforced (binding for the whole coexistence window):
packages/** NEVER imports mace (legacy); mace/ is NEVER edited to import packages/**.
mace_core imports no torch, no jax, no e3nn, no mace_torch, no mace_jax, no mace.
mace_torch imports mace_core; never mace_jax or mace. mace_jax imports mace_core; never mace_torch or mace.
Internal layering of mace_torch: a layer only imports layers at or below it — cli > train,data > models > nn > kernels > mace_core; nn/models do not import train/cli; kernels does not import nn (no god-__init__ that eagerly imports train).
Double-import exception: exactly two modules — mace_launcher.dispatch (the --engine {legacy,v1} entry-point dispatcher) and tests/parity/** (the harness that loads both stacks to compare them numerically). Nothing else, ever.
Static gate — import-linter config (.importlinter or the [tool.importlinter] table in pyproject.toml), checked over the whole tree:
forbidden/layers contracts for framework purity: mace_core ⊥ {torch, jax, e3nn, mace_torch, mace_jax}; mace_jax ⊥ mace_torch; the internal mace_torch layering above.
Explicit allowlist: mace_launcher.dispatch and tests.parity may import both sides.
Runtime gate — audit-hook. import-linter is static and has a known false-negative: importlib.import_module("mace.modules..."), __import__, or an entry-point reach-in are invisible to it. Close it with a reusable sys.addaudithook that intercepts import audit events and aborts any dynamic packages → mace import outside the two-module allowlist. Its home is packages/mace-launcher/src/mace_launcher/audit.py — the launcher is already on the allowlist, so the hook itself never lives in a pure zone (an earlier sketch placed it in mace_core._import_audit; do not — mace_core stays free of policy hooks). It is activated in the tests/parity/ conftest and by the launcher when running under --engine v1. Note audit hooks cannot be removed once added — install it once per process, idempotently.
Anti-structural-copying meta-lint. A fast check (natural home: tests/architecture/test_no_legacy_leak.py) asserting that no packages/** file references a legacy class or registry by name. This reinforces "reproduce behavior, never structure": even without an import, naming a legacy symbol in new code is the tell of a structural port.
CI job. A dedicated fast, CPU-only job runs lint-imports plus the architecture tests over the whole tree and gates every PR. tests/architecture runs inside the unit job today, so this job takes it over rather than adding a second runner for it: remove the path from unit's selection in the same change. The lint env installs mace/ AND packages/ editable together. It joins ci-core.yaml, whose only lint job today is lint (.github/workflows/ci-core.yaml:22-34).
Task:
Write the import-linter config with the forbidden contract (mace_core/mace_torch/mace_jax → mace), the purity layers contracts (mace_core ⊥ torch/jax/e3nn; mace_jax ⊥ mace_torch; mace_torch internal layering), and the two-module allowlist.
The runtime guard in mace_launcher.audit already exists and fires on both spellings; it is a sys.meta_path finder with an audit hook beside it, because the audit event is raised by the C __import__ and importlib does not go through it. What is left here is its second activation point: the tests/parity/ conftest. That directory is created by PAR-1 (PAR-1 — Continuous in-process legacy-vs-v1 parity harness #1572), so if it does not exist yet, say so in the PR and leave the wiring to the ticket that creates it rather than adding an empty package for it.
Add the dedicated CI job (fast, CPU) that installs both trees editable and runs lint-imports + tests/architecture/ on every PR, and drop tests/architecture from the unit job's selection so the suite runs once.
Implement the anti-structural-copying meta-lint over packages/** with the legacy-name denylist, as part of the same gating job. The denylist is an explicit enumerated list inside the meta-lint, not an open-ended reading of the legacy public surface. It holds only legacy symbols v1 does not carry over:
the four string→class registries — interaction_classes (mace/modules/__init__.py:71), readout_classes (:82), scaling_classes (:91), gate_dict (:97);
the legacy data types — AtomicData (mace/data/atomic_data.py:28) and the vendored torch_geometric names under mace/tools/torch_geometric/.
Names v1 deliberately reuses for its own ported code — BesselBasis, PolynomialCutoff, ZBLBasis, RadialEmbeddingBlock, LinearNodeEmbeddingBlock, AtomicNumberTable, Configuration, KeySpecification, DefaultKeys — are explicitly excluded. Adding or removing an entry is a reviewed edit to that one file.
Out of scope: the launcher/dispatcher itself (INF-2 (#1551)); the path-scoped dual toolchain and its "no file matches two toolchains" meta-lint (INF-4 (#1553)); any parity tests' numerical content (Phase 2+).
Acceptance criteria:
A deliberate from mace import … inside packages/mace_torch/fails the CI job (static gate).
A dynamic reach-in via importlib.import_module("mace.modules…") in a test under --engine v1fails via the audit-hook (runtime gate).
The meta-lint fails on a packages/** file naming a legacy class (e.g. ScaleShiftMACE), and its denylist covers all eleven legacy model classes (MACE, ScaleShiftMACE, AtomicDipolesMACE, AtomicDielectricMACE, EnergyDipolesMACE, MACELES, PolarMACE, MagneticMACE, MagneticScaleShiftMACE, MagneticSCFMACE, TimeReversalSymmetrizedMACE) and all four registries — a count asserted against the enumerated list, so a future legacy class added on develop cannot slip past silently.
mace_launcher and tests/parity/ pass despite importing both sides — and they are the only two entries on the allowlist.
The job runs on every PR, CPU-only, with mace/ and packages/ installed editable together.
Review focus: that the allowlist is EXACTLY two entries (mace_launcher, which holds the dispatcher and the guard, and tests.parity), both outside the pure zones — any third entry or a hook placed inside mace_core defeats the guard; and that the audit-hook actually fires on importlib/__import__ paths, not just on static import statements.
Depends on: INF-1 (#1550) · Blocks: all of Phase 2+ (permanent gate)
Context: The rewrite keeps the legacy
mace/package byte-frozen in-tree as a live numerical oracle while the new stack grows inpackages/(mace-core,mace-torch,mace-jax,mace-launcher, created by INF-1 (#1550)). The fresh-start's integrity comes from this guard, not from deletion: as long as a legacy class is physically unreachable from the new zone, it cannot be subclassed, re-exported, or imported — so v1 reproduces legacy behaviour without inheriting legacy structure, and the oracle stays trustworthy. This ticket turns that from a review aspiration into three CI-enforced mechanisms: a static import-linter contract, a runtimesys.addaudithookthat closes the dynamicimportlibloophole the static check cannot see, and a meta-lint that blocks structural copying by name (e.g.ScaleShiftMACEfrommace/modules/models.py:447, theinteraction_classesregistry atmace/modules/__init__.py:71,AtomicDatafrommace/data/atomic_data.py:28). Once merged, the guard gates every PR permanently — every Phase 2+ ticket writes code under it.Interface & constraints:
The invariants being enforced (binding for the whole coexistence window):
packages/**NEVER importsmace(legacy);mace/is NEVER edited to importpackages/**.mace_coreimports no torch, no jax, no e3nn, nomace_torch, nomace_jax, nomace.mace_torchimportsmace_core; nevermace_jaxormace.mace_jaximportsmace_core; nevermace_torchormace.mace_torch: a layer only imports layers at or below it —cli>train,data>models>nn>kernels>mace_core;nn/modelsdo not importtrain/cli;kernelsdoes not importnn(no god-__init__that eagerly importstrain).mace_launcher.dispatch(the--engine {legacy,v1}entry-point dispatcher) andtests/parity/**(the harness that loads both stacks to compare them numerically). Nothing else, ever.Static gate — import-linter config (
.importlinteror the[tool.importlinter]table inpyproject.toml), checked over the whole tree:mace_core,mace_torch,mace_jax→mace.mace_core⊥ {torch, jax, e3nn,mace_torch,mace_jax};mace_jax⊥mace_torch; the internalmace_torchlayering above.mace_launcher.dispatchandtests.paritymay import both sides.Runtime gate — audit-hook. import-linter is static and has a known false-negative:
importlib.import_module("mace.modules..."),__import__, or an entry-point reach-in are invisible to it. Close it with a reusablesys.addaudithookthat interceptsimportaudit events and aborts any dynamicpackages → maceimport outside the two-module allowlist. Its home ispackages/mace-launcher/src/mace_launcher/audit.py— the launcher is already on the allowlist, so the hook itself never lives in a pure zone (an earlier sketch placed it inmace_core._import_audit; do not —mace_corestays free of policy hooks). It is activated in thetests/parity/conftest and by the launcher when running under--engine v1. Note audit hooks cannot be removed once added — install it once per process, idempotently.Anti-structural-copying meta-lint. A fast check (natural home:
tests/architecture/test_no_legacy_leak.py) asserting that nopackages/**file references a legacy class or registry by name. This reinforces "reproduce behavior, never structure": even without an import, naming a legacy symbol in new code is the tell of a structural port.CI job. A dedicated fast, CPU-only job runs
lint-importsplus the architecture tests over the whole tree and gates every PR.tests/architectureruns inside theunitjob today, so this job takes it over rather than adding a second runner for it: remove the path fromunit's selection in the same change. The lint env installsmace/ANDpackages/editable together. It joinsci-core.yaml, whose only lint job today islint(.github/workflows/ci-core.yaml:22-34).Task:
Write the import-linter config with the forbidden contract (
mace_core/mace_torch/mace_jax→mace), the purity layers contracts (mace_core⊥ torch/jax/e3nn;mace_jax⊥mace_torch;mace_torchinternal layering), and the two-module allowlist.The runtime guard in
mace_launcher.auditalready exists and fires on both spellings; it is asys.meta_pathfinder with an audit hook beside it, because the audit event is raised by the C__import__andimportlibdoes not go through it. What is left here is its second activation point: thetests/parity/conftest. That directory is created by PAR-1 (PAR-1 — Continuous in-process legacy-vs-v1 parity harness #1572), so if it does not exist yet, say so in the PR and leave the wiring to the ticket that creates it rather than adding an empty package for it.Add the dedicated CI job (fast, CPU) that installs both trees editable and runs
lint-imports+tests/architecture/on every PR, and droptests/architecturefrom theunitjob's selection so the suite runs once.Implement the anti-structural-copying meta-lint over
packages/**with the legacy-name denylist, as part of the same gating job. The denylist is an explicit enumerated list inside the meta-lint, not an open-ended reading of the legacy public surface. It holds only legacy symbols v1 does not carry over:MACE(mace/modules/models.py:47),ScaleShiftMACE(:447),AtomicDipolesMACE(:632),AtomicDielectricMACE(:848),EnergyDipolesMACE(:1205),MACELES(mace/modules/extensions.py:142),PolarMACE(:666),MagneticMACE(:1434),MagneticScaleShiftMACE(:1712),MagneticSCFMACE(:1976),TimeReversalSymmetrizedMACE(:2116);interaction_classes(mace/modules/__init__.py:71),readout_classes(:82),scaling_classes(:91),gate_dict(:97);AtomicData(mace/data/atomic_data.py:28) and the vendoredtorch_geometricnames undermace/tools/torch_geometric/.Names v1 deliberately reuses for its own ported code —
BesselBasis,PolynomialCutoff,ZBLBasis,RadialEmbeddingBlock,LinearNodeEmbeddingBlock,AtomicNumberTable,Configuration,KeySpecification,DefaultKeys— are explicitly excluded. Adding or removing an entry is a reviewed edit to that one file.Out of scope: the launcher/dispatcher itself (INF-2 (#1551)); the path-scoped dual toolchain and its "no file matches two toolchains" meta-lint (INF-4 (#1553)); any parity tests' numerical content (Phase 2+).
Acceptance criteria:
from mace import …insidepackages/mace_torch/fails the CI job (static gate).importlib.import_module("mace.modules…")in a test under--engine v1fails via the audit-hook (runtime gate).packages/**file naming a legacy class (e.g.ScaleShiftMACE), and its denylist covers all eleven legacy model classes (MACE,ScaleShiftMACE,AtomicDipolesMACE,AtomicDielectricMACE,EnergyDipolesMACE,MACELES,PolarMACE,MagneticMACE,MagneticScaleShiftMACE,MagneticSCFMACE,TimeReversalSymmetrizedMACE) and all four registries — a count asserted against the enumerated list, so a future legacy class added ondevelopcannot slip past silently.mace_launcherandtests/parity/pass despite importing both sides — and they are the only two entries on the allowlist.mace/andpackages/installed editable together.Verify:
Review focus: that the allowlist is EXACTLY two entries (
mace_launcher, which holds the dispatcher and the guard, andtests.parity), both outside the pure zones — any third entry or a hook placed insidemace_coredefeats the guard; and that the audit-hook actually fires onimportlib/__import__paths, not just on staticimportstatements.