Make HistoryEncoder a pluggable registry, like Router/Objective (gitea #35)
CI / Lint (ruff check) (push) Successful in 30s
CI / Format (ruff format) (push) Successful in 30s
CI / Sync project version with tag (push) Has been skipped
CI / Type check (ty) (push) Successful in 34s
CI / Lint (ruff check) (pull_request) Successful in 33s
CI / Format (ruff format) (pull_request) Successful in 42s
CI / Type check (ty) (pull_request) Successful in 42s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Tests (pull_request) Successful in 3m52s
CI / Tests (push) Successful in 4m0s
CI / Lint (ruff check) (push) Successful in 30s
CI / Format (ruff format) (push) Successful in 30s
CI / Sync project version with tag (push) Has been skipped
CI / Type check (ty) (push) Successful in 34s
CI / Lint (ruff check) (pull_request) Successful in 33s
CI / Format (ruff format) (pull_request) Successful in 42s
CI / Type check (ty) (pull_request) Successful in 42s
CI / Sync project version with tag (pull_request) Has been skipped
CI / Tests (pull_request) Successful in 3m52s
CI / Tests (push) Successful in 4m0s
Stage2Autoregressive.init_history_cache and .history_step both isinstance-checked self.history_encoder against AttentionHistory to decide whether to use its real incremental-cache methods or a no-op fallback, so a third history type couldn't be added without editing Stage2Autoregressive itself. The two-value "markov"/"attention" enum was also independently hardcoded in three places (Stage2Autoregressive's own validation, config.py's validate_config, and AutoregressiveConfig.from_dict's default). Mirrors the Router (giant/model/routers.py) and Objective (giant/model/objectives.py, gitea #32) pattern: HistoryEncoder now declares working O(1) init_cache/step defaults (init_cache -> None, step -> one forward() call), so every registered history type satisfies the incremental interface without opting in; AttentionHistory overrides both with its real KV-cache versions since its forward() needs the full prefix. Added HISTORY_REGISTRY/register_history/build_history, registered "markov" and "attention", and deleted both isinstance checks in models.py. Per user decision during planning, config.py's validate_config now imports HISTORY_REGISTRY and checks membership dynamically instead of keeping its own hardcoded tuple, making the registry the single source of truth end to end (verified no import cycle: config.py had no prior dependency on giant.model, and giant.model.history has none on giant.config). No config-schema change and no checkpoint impact — this is a pure internal-interface refactor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,14 +5,17 @@ import torch
|
||||
from giant import config as gconfig
|
||||
from giant.constants import CONT_SLOT_DIM, COND_DIM, PARTICLE_PHYS_DIM, SEC_SLOT_DIM
|
||||
from giant.model.network import (
|
||||
HISTORY_REGISTRY,
|
||||
AttentionHistory,
|
||||
ConditionEncoder,
|
||||
HistoryEncoder,
|
||||
MarkovHistory,
|
||||
SinusoidalEmbedding,
|
||||
Stage1Model,
|
||||
Stage2Autoregressive,
|
||||
Stage2OneShot,
|
||||
build_critics,
|
||||
build_history,
|
||||
build_models,
|
||||
cat_col_layout,
|
||||
stage2_trunk_sec_dim,
|
||||
@@ -407,6 +410,48 @@ def test_attention_history_step_matches_forward():
|
||||
assert torch.allclose(stepped, expected, atol=1e-5)
|
||||
|
||||
|
||||
# --- HISTORY_REGISTRY / build_history (gitea #35) ----------------------------
|
||||
|
||||
|
||||
def test_history_registry_has_exactly_the_two_known_histories():
|
||||
assert set(HISTORY_REGISTRY) == {"markov", "attention"}
|
||||
|
||||
|
||||
def test_build_history_returns_correct_concrete_type():
|
||||
assert isinstance(build_history("markov", 4, 6), MarkovHistory)
|
||||
assert isinstance(build_history("attention", 4, 8), AttentionHistory)
|
||||
|
||||
|
||||
def test_build_history_unknown_name_raises():
|
||||
with pytest.raises(ValueError):
|
||||
build_history("bogus", 4, 6)
|
||||
|
||||
|
||||
def test_build_history_filters_kwargs_by_signature():
|
||||
"""Attention-only kwargs (n_heads/n_layers) must be silently dropped when
|
||||
building a MarkovHistory, matching build_router's documented behavior for
|
||||
per-type hyperparameters coexisting in one config."""
|
||||
hist = build_history("markov", 4, 6, n_heads=2, n_layers=1)
|
||||
assert isinstance(hist, MarkovHistory)
|
||||
|
||||
|
||||
def test_history_encoder_base_default_init_cache_and_step():
|
||||
"""A HistoryEncoder subclass implementing only forward() must still get
|
||||
working O(1) init_cache/step defaults from the base class."""
|
||||
|
||||
class _StubHistory(HistoryEncoder):
|
||||
def forward(self, feat, has_prev):
|
||||
return feat * 2
|
||||
|
||||
hist = _StubHistory()
|
||||
assert hist.init_cache() is None
|
||||
feat = torch.randn(2, 1, 4)
|
||||
has_prev = torch.ones(2, 1, dtype=torch.bool)
|
||||
out, cache = hist.step(feat, has_prev, "unused-cache")
|
||||
assert torch.equal(out, hist.forward(feat, has_prev))
|
||||
assert cache == "unused-cache"
|
||||
|
||||
|
||||
# --- Stage2Autoregressive (v0.3.0 step 5) -----------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user