Reject stage2_model.stage1_context = 'sampled' as unimplemented (issues.md Issue 1)
trainers.py unconditionally trains stage 2 against the ground-truth stage-1 output (stage1_ctx = x1_s1.detach()), but 'sampled' was accepted by validate_config, stored in config.toml and the checkpoint's model_config, and silently trained identically to 'truth' — mislabeling every downstream artifact for a run launched with --stage2-stage1-context sampled. Mirrors the existing stop_token validate_config pattern. User chose the immediate fix (reject loudly) over the proper fix (actually implement sampled context), which is scoped to Issue 16. Also updates the _KNOWN_UNUSED reason for stage2_model.stage1_context (added by Issue 5's consumed-keys audit) to reflect that the value is now rejected rather than silently accepted, and drops the now-invalid --stage2-stage1-context sampled case from test_stage2_only_knobs (a full CLI invocation) — that flag's plumbing is still covered at the overrides-dict level by test_overrides_from_flags_stage2_only_knobs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1256,6 +1256,14 @@ def validate_config(cfg: dict) -> None:
|
||||
"(standalone stage-2 evaluation only, never for rollout)"
|
||||
)
|
||||
|
||||
if _get_path(cfg, "stage2_model.stage1_context") == "sampled":
|
||||
raise ValueError(
|
||||
"stage2_model.stage1_context = 'sampled' is accepted by the schema "
|
||||
"but not implemented — trainers.py always trains stage 2 against "
|
||||
"the ground-truth stage-1 output; use 'truth' (default) instead "
|
||||
"(see issues.md Issue 16 for the planned implementation)"
|
||||
)
|
||||
|
||||
if (
|
||||
_get_path(cfg, "stage2_model.n_sec.mode") == "truth"
|
||||
and _get_path(cfg, "stage1_model.active")
|
||||
|
||||
@@ -41,6 +41,10 @@ def test_stage_prefixed_generator_overrides_shared_mode(monkeypatch, tmp_path):
|
||||
|
||||
|
||||
def test_stage2_only_knobs(monkeypatch, tmp_path):
|
||||
# --stage2-stage1-context is exercised separately at the overrides-dict
|
||||
# level (test_overrides_from_flags_stage2_only_knobs in test_config.py):
|
||||
# its only non-default value, "sampled", is rejected by validate_config
|
||||
# (issues.md Issue 1), so it can't appear in a full CLI invocation here.
|
||||
cfg = _invoke_and_capture_cfg(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
@@ -53,15 +57,12 @@ def test_stage2_only_knobs(monkeypatch, tmp_path):
|
||||
"32",
|
||||
"--stage2-context-dim",
|
||||
"16",
|
||||
"--stage2-stage1-context",
|
||||
"sampled",
|
||||
],
|
||||
)
|
||||
assert cfg["stage2_model"]["decoder"] == "one_shot"
|
||||
assert cfg["stage2_model"]["k_max"] == 8
|
||||
assert cfg["stage2_model"]["hidden_dim"] == 32
|
||||
assert cfg["stage2_model"]["context_dim"] == 16
|
||||
assert cfg["stage2_model"]["stage1_context"] == "sampled"
|
||||
# untouched stage1 defaults
|
||||
assert cfg["stage1_model"]["hidden_dim"] == 256
|
||||
|
||||
|
||||
@@ -687,6 +687,15 @@ def test_validate_config_stop_token_not_implemented():
|
||||
assert "stop_token" in str(e)
|
||||
|
||||
|
||||
def test_validate_config_stage1_context_sampled_not_implemented():
|
||||
cfg = _cfg_with(**{"stage2_model.stage1_context": "sampled"})
|
||||
try:
|
||||
gconfig.validate_config(cfg)
|
||||
assert False, "expected ValueError"
|
||||
except ValueError as e:
|
||||
assert "sampled" in str(e)
|
||||
|
||||
|
||||
def test_validate_config_n_sec_truth_rejected_for_rollout_capable_checkpoint():
|
||||
"""'n_sec.mode = "truth" is invalid for a rollout-capable checkpoint' —
|
||||
both stages active means giant rollout
|
||||
|
||||
@@ -55,8 +55,10 @@ _EXCLUDED_FILES = ("giant/model/_legacy.py",)
|
||||
_KNOWN_UNUSED = {
|
||||
"stage2_model.stage1_context": (
|
||||
"issues.md Issue 1 — trainers.py hardcodes stage1_ctx to the "
|
||||
"ground-truth stage-1 output; 'sampled' is accepted and stored but "
|
||||
"never read"
|
||||
"ground-truth stage-1 output; 'sampled' is now rejected loudly by "
|
||||
"validate_config (not silently accepted), but the key still isn't "
|
||||
"read by any build/train consumer file since only 'truth' can pass "
|
||||
"validation — see Issue 16 for the real implementation"
|
||||
),
|
||||
"stage1_model.wgan.critic_hidden_dim": (
|
||||
"issues.md Issue 2 — build_critics always sizes the critic off the generator's own hidden_dim, never this key"
|
||||
|
||||
Reference in New Issue
Block a user