NVIDIA-NeMo / NVIDIA-NeMo/RL

Centralize validation settings across algorithms

Open
#3,425 3 comments 0 reactions 1 assignee Claimed by @terrykong View on GitHub
Feature
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.