Add the shared backend registry (item 5, step 1 of 3) - #162
Merged
Conversation
The mechanism only. No caller uses it yet -- `ModelFactory` and
`IsomerEngineFactory` move onto it in the next two commits, and this should not
merge on its own, because a registry nobody registers with is dead code.
Auto3D has two families of swappable backend and until now each carried its own
bespoke lookup: a dict plus an if-chain for models, a tuple plus an if/elif
ladder for isomer engines. Adding a model backend means editing five places and
an isomer backend six. One of those five is the *presentation* layer, where
`cli/commands/models.py`'s `ENGINE_INFO` keeps a parallel table of display
metadata keyed by engine name that nothing checks against the set of real
backends -- so a backend registered without an entry there silently stops
appearing in `auto3d models info`. `Entry.info` is what removes that table.
Three things it deliberately does not do:
- **It does not own construction.** A model adapter is built with
`(device, compile_model)`; an isomer engine takes eight-odd keyword
arguments. A signature both satisfy is a bag of keywords that hides what
each backend needs and stops the type checker helping -- the same defect the
plan records for `threshold` meaning three different quantities. Each
factory resolves a name and then calls its own constructor.
- **It is not a plugin system.** Auto3D already accepts a third-party model as
a file path, checked against the `CustomNNP` contract at load, so
entry-point discovery would add only *named, installable* backends, and
would mean freezing `ModelAdapter` while it is still gaining members
(`analytic_hessian` in 3.0.0, `native_dtype` still wanted for D4). A
registry is what such a loader would populate, so this does not foreclose
it.
- **It does not decide case-folding.** Model names resolve case-insensitively
and isomer engine types are exact lowercase; both are current, tested
behavior, so it is a constructor flag.
Registering a name twice raises rather than overwriting -- the failure a plain
dict gives you, where a duplicate registration makes behavior depend on import
order.
`Auto3D.registry` imports only `Auto3D.exceptions`, so it is a genuine
foundation leaf, and it is mapped into L0. The layer map's totality check failed
the moment the file appeared, which is what it is for.
Suite: 1756 passed, 1 skipped, 70 deselected.
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
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.
This lands a module nothing calls yet.
ModelFactoryandIsomerEngineFactorymove onto it in steps 2 and 3; until thenmaincarries~130 lines with no consumers. Merged deliberately rather than by oversight, so
the next reader doesn't have to wonder.
Non-breaking. Suite: 1756 passed, 1 skipped, 70 deselected.
Why a registry
Auto3D has two families of swappable backend, and each carries its own bespoke
lookup — a dict plus an if-chain for models, a tuple plus an if/elif ladder for
isomer engines. Adding a model backend means editing five places; an isomer
backend, six. One of the five is the presentation layer.
Scope, narrowed from the plan on purpose
The plan's item 5 reads "one registry + entry-point discovery, promote
ModelAdapterto public and version it". Entry points and the public interfaceare out of scope, because they solve a problem this package doesn't have:
Auto3D already accepts a third-party model as a file path
(
--engine /path/to/my_nnp.pt), checked against theCustomNNPcontract atload. Entry points would add only named, installable backends — no
demonstrated demand — and would mean freezing
ModelAdapterwhile it is stillgaining members (
analytic_hessianin 3.0.0,native_dtypestill wanted forD4). A registry is what an entry-point loader would populate, so this doesn't
foreclose that.
Three things it deliberately does not do
It does not own construction. A model adapter is built with
(device, compile_model); an isomer engine takes eight-odd keyword arguments. Asignature both satisfy is a keyword bag that hides what each backend needs and
stops the type checker helping — the same defect the plan records for
thresholdmeaning three different quantities.It is not a plugin system. See scope above.
It does not decide case-folding. Model names resolve case-insensitively;
isomer engine types are exact lowercase. Both are current, tested behavior, so
it is a constructor flag rather than a policy this module picks.
Registering a name twice raises rather than overwriting — the failure a plain
dict gives you, where a duplicate makes behavior depend on import order.
Found while starting step 2, and worth recording here
There are three parallel lists of engine names, not the two I had measured:
ModelFactory._adaptersANI2X,ANI2XTonlyModelFactory.available_models()ENGINE_INFOAIMNET, three aimnet variants, two ANIThey hold different sets.
_adaptersholds only engines built from a localadapter class; the four aimnet entries are names resolved through the aimnet
registry by
resolve_engine_name. Nothing connects the three — they agree byhand.
So the win is larger than estimated (three lists collapse to one) and step 2 is
broader than "swap the dict for a
Registry": the registry must be keyed byuser-facing engine name, with each entry saying how the engine is built and
carrying its display metadata, and
available_models()derived from it.This is recorded in the PR body because the design spec lives under
docs/superpowers/, which is gitignored — so the spec's own copy of thiscorrection is not durable.
Layering
Auto3D.registryimports onlyAuto3D.exceptions, so it is a genuinefoundation leaf, and it is mapped into L0. The layer map's totality check failed
the moment the file appeared, which is what it is for.