Feat/external architecture registration#1307
Open
huseyincavusbi wants to merge 7 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a public mechanism for registering custom architecture adapters in TransformerLens without forking the project, plus accompanying tests and documentation.
Changes:
- New
ArchitectureAdapterFactory.register_adapter()classmethod for runtime registration, anddiscover_entry_points()that reads adapters from thetransformer_lens.architecturesentry-point group on first use. - New unit test module covering registration, discovery idempotency, and select error paths.
- New documentation page under Contributing describing both registration paths and an example external package layout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| transformer_lens/factories/architecture_adapter_factory.py | Adds register_adapter and discover_entry_points classmethods; calls discovery at the start of select_architecture_adapter. |
| tests/unit/model_bridge/test_architecture_adapter_factory.py | New tests for SUPPORTED_ARCHITECTURES contents, runtime registration, error handling, and discovery idempotency. |
| docs/source/content/contributing.md | Adds the new doc page to the toctree. |
| docs/source/content/adapter_development/external-adapter-registration.md | New doc page explaining runtime and entry-point registration, with example package layout. |
Comments suppressed due to low confidence (1)
tests/unit/model_bridge/test_architecture_adapter_factory.py:58
- This test does not actually verify "overwrite" behavior. It registers
MockArchitectureAdaptertwice for the same key, so the post-conditioncls._adapters[key] is firstis trivially true regardless of whether the secondregister_adaptercall overwrote anything. To meaningfully test overwrite behavior, register a second, different adapter class and assert that the registry now points at the new class (and not atfirst).
def test_register_overwrites_existing(self):
key = "TestOverwriteForCausalLM"
ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter)
first = ArchitectureAdapterFactory._adapters[key]
ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter)
assert ArchitectureAdapterFactory._adapters[key] is first
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+176
to
+181
| try: | ||
| eps = entry_points(group="transformer_lens.architectures") | ||
| for ep in eps: | ||
| cls._adapters[ep.name] = ep.load() | ||
| except Exception: | ||
| pass |
Comment on lines
+45
to
+97
| class TestRegisterAdapter: | ||
| """Verify runtime adapter registration.""" | ||
|
|
||
| def test_register_adds_to_adapters(self): | ||
| key = "TestMockForCausalLM" | ||
| ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter) | ||
| assert key in ArchitectureAdapterFactory._adapters | ||
|
|
||
| def test_register_overwrites_existing(self): | ||
| key = "TestOverwriteForCausalLM" | ||
| ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter) | ||
| first = ArchitectureAdapterFactory._adapters[key] | ||
| ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter) | ||
| assert ArchitectureAdapterFactory._adapters[key] is first | ||
|
|
||
| def test_select_returns_registered_adapter(self): | ||
| key = "TestSelectForCausalLM" | ||
| ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter) | ||
| cfg = _make_cfg(architecture=key) | ||
| adapter = ArchitectureAdapterFactory.select_architecture_adapter(cfg) | ||
| assert isinstance(adapter, MockArchitectureAdapter) | ||
|
|
||
|
|
||
| class TestSelectErrors: | ||
| """Verify error handling in select_architecture_adapter.""" | ||
|
|
||
| def test_unknown_architecture_raises(self): | ||
| cfg = _make_cfg(architecture="NonExistentForCausalLM") | ||
| with pytest.raises(ValueError, match="Unsupported architecture"): | ||
| ArchitectureAdapterFactory.select_architecture_adapter(cfg) | ||
|
|
||
| def test_none_architecture_raises(self): | ||
| cfg = _make_cfg(architecture=None) | ||
| with pytest.raises(ValueError, match="must have architecture set"): | ||
| ArchitectureAdapterFactory.select_architecture_adapter(cfg) | ||
|
|
||
|
|
||
| class TestDiscoverEntryPoints: | ||
| """Verify entry-point discovery behavior.""" | ||
|
|
||
| def test_discover_is_idempotent(self): | ||
| ArchitectureAdapterFactory._entry_points_discovered = False | ||
| ArchitectureAdapterFactory.discover_entry_points() | ||
| first_run = ArchitectureAdapterFactory._entry_points_discovered | ||
| ArchitectureAdapterFactory.discover_entry_points() | ||
| assert ArchitectureAdapterFactory._entry_points_discovered is first_run is True | ||
|
|
||
| def test_discover_does_not_remove_existing(self): | ||
| key = "TestPreserveForCausalLM" | ||
| ArchitectureAdapterFactory.register_adapter(key, MockArchitectureAdapter) | ||
| ArchitectureAdapterFactory._entry_points_discovered = False | ||
| ArchitectureAdapterFactory.discover_entry_points() | ||
| assert key in ArchitectureAdapterFactory._adapters |
| discovery of adapters from installed packages via entry points. | ||
| """ | ||
|
|
||
| _adapters = SUPPORTED_ARCHITECTURES |
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.
Hi @jlarson4,
Description
Three changes to allow users to register custom architecture adapters without forking TransformerLens:
register_adapter()method — Classmethod onArchitectureAdapterFactorythat adds custom architectures to the registry at runtimediscover_entry_points()scans installed packages for adapters declared viatransformer_lens.architecturesentry group, auto-called on firstselect_architecture_adapter()Closes #1298
Type of change
Checklist