Centralize validation settings across algorithms
- Dominant language
- Python
- Stars
- 2k
- Forks
- 561
- Avg merge
- 4d 5h
- Merged PRs (30d)
- 145
Description
The validation cadence pattern is duplicated verbatim across every algorithm:
```python
if (val_period > 0 and (total_steps + 1) % val_period == 0) or (val_at_end and is_last_step):
...
```
Occurrences:
- `nemo_rl/algorithms/grpo.py` (`grpo_train`, `async_grpo_train`)
- `nemo_rl/algorithms/grpo_sync.py` (`grpo_train_sync`)
- `nemo_rl/algorithms/ppo.py:1348`
- `nemo_rl/algorithms/dpo.py:607`
- `nemo_rl/algorithms/sft.py:484`
- `nemo_rl/algorithms/rm.py:550`
- `nemo_rl/algorithms/distillation.py:930`
- `nemo_rl/algorithms/xtoken_off_policy_distillation.py:679`
- `nemo_rl/algorithms/single_controller_utils/setup.py:288-297` — validation not yet wired up on the SingleController path (currently `raise NotImplementedError` when any of `val_period > 0 / val_at_start / val_at_end` is set); centralizing before it lands avoids a 7th duplicate.
Each algo also re-declares the settings themselves (`val_period`, `val_at_start`, `val_at_end`, `val_batch_size`, `max_val_samples`) under its own `TypedDict`/`BaseModel`, so the family drifts one field at a time. #3400 just added `val_start_at` to `GRPOConfig` alone, immediately creating asymmetry — the same knob is useful for every algo above.
### Proposal
Centralize into a single `ValidationConfig` (Pydantic `BaseModel(extra="allow")` per the v2 convention in `docs/design-docs/design-and-philosophy.md`):
```python
class ValidationConfig(BaseModel, extra="allow"):
period: int = -1 # -1 disables periodic validation
start_at: int = -1 # -1 disables the delay
at_start: bool = False
at_end: bool = False
batch_size: int | None = None
max_samples: int | None = None
```
Each `MasterConfig` mounts one `validation: ValidationConfig` field, replacing the algo-scoped duplicates. A single helper — e.g. `nemo_rl/algorithms/validation.py::should_validate(step, is_last_step, cfg)` — consumes it, so each trainer collapses to:
```python
if should_validate(step + 1, is_last_step, master_config.validation):
...
```
### Notes
- BC via `extra="allow"` plus a `tools/config_cli.py` migration step for existing exemplars/recipes (`grpo.val_period` → `validation.period`, etc.).
- Aligns with the v1 (TypedDict) → v2 (BaseModel) migration tracked by `tests/unit/test_config_v2.py`.
- Non-goals: validation logic itself (metric computation, dataloader construction) stays where it is — this is only about the cadence + knobs.
Followup from https://github.com/NVIDIA-NeMo/RL/pull/3400 review.
Contributor guide
Assessment
This issue has not been assessed yet.