Validate stage2_model.autoregressive.order in validate_config (gitea #30)
order was documented as single-valued ("energy_desc" only, placeholder for a
future alternative ordering) but validate_config only checked its siblings
history/teacher_forcing, so e.g. order = "energy_asc" was silently accepted
and trained as if it were energy_desc. Add the missing check alongside the
other two, gated the same way (only meaningful under
stage2_model.decoder = "autoregressive"). Also updates the stale reason
string on the pre-existing _KNOWN_UNUSED allow-list entry for this key in
tests/test_config_consumed_keys.py, since half of it ("validate_config ...
never [checks] order") is no longer true after this fix — the key stays
allow-listed because validate_config itself isn't in that test's
build/train/rollout consumer whitelist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1287,6 +1287,13 @@ def validate_config(cfg: dict) -> None:
|
||||
)
|
||||
|
||||
if _get_path(cfg, "stage2_model.decoder") == "autoregressive":
|
||||
order = _get_path(cfg, "stage2_model.autoregressive.order")
|
||||
if order != "energy_desc":
|
||||
raise ValueError(
|
||||
f"stage2_model.autoregressive.order = {order!r} — must be "
|
||||
"'energy_desc' (the only implemented ordering; see "
|
||||
"AutoregressiveConfig.order's docstring)"
|
||||
)
|
||||
history = _get_path(cfg, "stage2_model.autoregressive.history")
|
||||
if history not in ("markov", "attention"):
|
||||
raise ValueError(f"stage2_model.autoregressive.history = {history!r} — must be 'markov' or 'attention'")
|
||||
|
||||
+28
-2
@@ -747,6 +747,31 @@ def test_validate_config_ar_default_markov_always_passes():
|
||||
gconfig.validate_config(cfg) # must not raise
|
||||
|
||||
|
||||
def test_validate_config_ar_order_energy_desc_passes():
|
||||
"""'energy_desc' is the only implemented order — must not raise."""
|
||||
cfg = _cfg_with(
|
||||
**{
|
||||
"stage2_model.decoder": "autoregressive",
|
||||
"stage2_model.autoregressive.order": "energy_desc",
|
||||
}
|
||||
)
|
||||
gconfig.validate_config(cfg) # must not raise
|
||||
|
||||
|
||||
def test_validate_config_ar_order_invalid_value_rejected():
|
||||
cfg = _cfg_with(
|
||||
**{
|
||||
"stage2_model.decoder": "autoregressive",
|
||||
"stage2_model.autoregressive.order": "energy_asc",
|
||||
}
|
||||
)
|
||||
try:
|
||||
gconfig.validate_config(cfg)
|
||||
assert False, "expected ValueError"
|
||||
except ValueError as e:
|
||||
assert "order" in str(e)
|
||||
|
||||
|
||||
def test_validate_config_ar_history_attention_passes():
|
||||
"""v0.3.0 step 7 implements history='attention' — must not raise."""
|
||||
cfg = _cfg_with(
|
||||
@@ -800,11 +825,12 @@ def test_validate_config_ar_teacher_forcing_invalid_value_rejected():
|
||||
|
||||
|
||||
def test_validate_config_ar_checks_skipped_under_one_shot():
|
||||
"""history/teacher_forcing values that would fail under AR are irrelevant
|
||||
(and unchecked) when decoder='one_shot'."""
|
||||
"""order/history/teacher_forcing values that would fail under AR are
|
||||
irrelevant (and unchecked) when decoder='one_shot'."""
|
||||
cfg = _cfg_with(
|
||||
**{
|
||||
"stage2_model.decoder": "one_shot",
|
||||
"stage2_model.autoregressive.order": "bogus",
|
||||
"stage2_model.autoregressive.history": "attention",
|
||||
"stage2_model.autoregressive.teacher_forcing": "scheduled",
|
||||
}
|
||||
|
||||
@@ -61,8 +61,9 @@ _KNOWN_UNUSED = {
|
||||
"validation — see Issue 16 for the real implementation"
|
||||
),
|
||||
"stage2_model.autoregressive.order": (
|
||||
"issues.md Issue 4 — validate_config checks history/teacher_forcing "
|
||||
"but never order, and nothing reads it either"
|
||||
"gitea #30 — validate_config now checks order is 'energy_desc', but "
|
||||
"nothing in the build/train/rollout consumer whitelist reads the "
|
||||
"value itself since it's still single-valued"
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user