wgan.critic_hidden_dim / critic_n_res_blocks are dead config keys #28

New Issue

2026-08-13T15:05:32+02:00

lars commented

2026-08-13 15:05:32 +02:00

Severity: medium.

giant/config.py:160-163 documents the sentinel contract:

# 0 means "inherit stage1_model.hidden_dim/n_res_blocks" — same
# round-trip-friendly sentinel as critic_lr above.
critic_hidden_dim: int = 0
critic_n_res_blocks: int = 0

build_critics (giant/model/builders.py:169-198) never reads them. It passes
s1_spec.hidden_dim / s1_spec.n_res_blocks (and the stage-2 equivalents)
straight through, so the critic is always the same size as the generator and
a non-zero critic_hidden_dim is silently discarded. Both stages are affected.
critic_lr's sentinel, by contrast, is honoured (StageSpec.critic_lr).

This matters for the WGAN track specifically: an under- or over-powered critic
relative to the generator is one of the standard first things to tune when
WGAN-GP training misbehaves, and right now that knob reads as available but
isn't. There is also no CLI flag for either (unlike --critic-lr).

Fix: read both in build_critics with the documented 0 → inherit
fallback, and add --stage{1,2}-critic-hidden-dim / --...-critic-n-res-blocks
to the FlagSpec table.

Migrated from issues.md (v0.3.0 branch review, 2026-08-13), Issue 2.

**Severity: medium.** `giant/config.py:160-163` documents the sentinel contract: ```python # 0 means "inherit stage1_model.hidden_dim/n_res_blocks" — same # round-trip-friendly sentinel as critic_lr above. critic_hidden_dim: int = 0 critic_n_res_blocks: int = 0 ``` `build_critics` (`giant/model/builders.py:169-198`) never reads them. It passes `s1_spec.hidden_dim` / `s1_spec.n_res_blocks` (and the stage-2 equivalents) straight through, so the critic is *always* the same size as the generator and a non-zero `critic_hidden_dim` is silently discarded. Both stages are affected. `critic_lr`'s sentinel, by contrast, is honoured (`StageSpec.critic_lr`). This matters for the WGAN track specifically: an under- or over-powered critic relative to the generator is one of the standard first things to tune when WGAN-GP training misbehaves, and right now that knob reads as available but isn't. There is also no CLI flag for either (unlike `--critic-lr`). **Fix:** read both in `build_critics` with the documented `0 → inherit` fallback, and add `--stage{1,2}-critic-hidden-dim` / `--...-critic-n-res-blocks` to the `FlagSpec` table. --- Migrated from `issues.md` (v0.3.0 branch review, 2026-08-13), Issue 2.

lars added the bug label 2026-08-13 15:05:32 +02:00

lars commented

2026-08-13 15:36:38 +02:00

Fixed in da71797: build_critics now reads stage{1,2}_model.wgan.critic_hidden_dim/critic_n_res_blocks with the documented 0 -> inherit fallback, instead of always sizing the critic off the generator's own hidden_dim/n_res_blocks. Added stage-scoped-only CLI flags (--stage{1,2}-critic-hidden-dim, --stage{1,2}-critic-n-res-blocks) -- no shared alias, since this is an architectural per-stage knob like --hidden-dim/--n-res-blocks rather than a shared training hyperparameter. Also removed the 4 now-stale _KNOWN_UNUSED entries in tests/test_config_consumed_keys.py and added regression tests in tests/test_network.py and tests/test_config.py.

Fixed in da71797: build_critics now reads stage{1,2}_model.wgan.critic_hidden_dim/critic_n_res_blocks with the documented 0 -> inherit fallback, instead of always sizing the critic off the generator's own hidden_dim/n_res_blocks. Added stage-scoped-only CLI flags (--stage{1,2}-critic-hidden-dim, --stage{1,2}-critic-n-res-blocks) -- no shared alias, since this is an architectural per-stage knob like --hidden-dim/--n-res-blocks rather than a shared training hyperparameter. Also removed the 4 now-stale _KNOWN_UNUSED entries in tests/test_config_consumed_keys.py and added regression tests in tests/test_network.py and tests/test_config.py.

lars closed this issue

2026-08-13 15:36:38 +02:00

lars referenced this issue from a commit

2026-08-13 16:21:01 +02:00

Honour wgan.critic_hidden_dim/critic_n_res_blocks in build_critics (gitea #28)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lars/giant#28