Skip to content

feat(grpo): add val_start_at to delay periodic validation - #3400

Merged
yuki-97 merged 3 commits into
NVIDIA-NeMo:mainfrom
michal2409:split/val-start-at
Jul 30, 2026
Merged

feat(grpo): add val_start_at to delay periodic validation#3400
yuki-97 merged 3 commits into
NVIDIA-NeMo:mainfrom
michal2409:split/val-start-at

Conversation

@michal2409

@michal2409 michal2409 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Adds grpo.val_start_at: the first training step eligible for periodic validation. It mirrors val_period's required-int convention: -1 (the exemplar default) disables the delay and keeps today's behavior exactly. When set, periodic validation (val_period) only fires once step >= val_start_at; val_at_start and val_at_end are unaffected.

Adds grpo.val_start_at (NotRequired): the first training step eligible
for periodic validation. Absent keeps today's behavior exactly. When
set, periodic validation (val_period) only fires once step >= val_start_at;
val_at_start and val_at_end are unaffected.

Motivation: on long-horizon agentic RL (e.g. SWE agents), early-training
validation is expensive (hours of rollouts) and uninformative because the
policy cannot plausibly have reached a useful accuracy yet. A validation
lower bound recovers that wall-clock without changing the cadence
afterwards: with val_period=2 and val_start_at=3, validation runs at
steps 4, 6, 8, ...

Covered by test_periodic_validation_starts_at_configured_step for both
grpo_train and async_grpo_train, including the val_at_end interaction.

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
@michal2409
michal2409 requested review from a team as code owners July 29, 2026 11:06
@copy-pr-bot

copy-pr-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yuki-97 yuki-97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @michal2409 , overall LGTM and left some minor comments.

Comment thread tests/unit/algorithms/test_grpo.py Outdated
Comment thread nemo_rl/algorithms/grpo.py Outdated
Comment thread nemo_rl/algorithms/grpo.py Outdated
Address review on NVIDIA-NeMo#3400:
- val_start_at is now a required GRPOConfig int mirroring val_period's
  convention (-1 disables the delay). The default is recorded in every
  standalone GRPO-family root config (grpo_math_1B.yaml, the nemo_gym
  roots, and the nemotron-3-super stage configs — all other grpo YAMLs
  inherit one of these via defaults:) plus the v2 reference config.
- Trainers direct-read master_config.grpo["val_start_at"] and the
  guards drop the None branch: (step + 1) >= val_start_at.
- test_periodic_validation_starts_at_configured_step now also covers
  grpo_train_sync via a new mock_sync_grpo_infrastructure helper that
  stubs the SyncRolloutActor and the TQ data-plane policy surface so
  the sync driver loop runs for real.

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
yuki-97
yuki-97 previously approved these changes Jul 30, 2026

@yuki-97 yuki-97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @michal2409

@yuki-97 yuki-97 added the CI:L1 Run doctests, unit tests, and functional tests label Jul 30, 2026
@yuki-97

yuki-97 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

/ok to test 0a4e31a

research/template_project/single_update.py validates the full pydantic
MasterConfig, so its config copy needs every required grpo key too.
Fixes the L1_Functional_Tests_Other_1 failure.

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
@michal2409
michal2409 requested a review from terrykong as a code owner July 30, 2026 12:44
michal2409 added a commit to michal2409/RL that referenced this pull request Jul 30, 2026
research/template_project/single_update.py validates the full pydantic
MasterConfig, so its config copy needs every required grpo key too
(same failure mode NVIDIA-NeMo#3400 hit in L1_Functional_Tests_Other_1).

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
michal2409 added a commit to michal2409/RL that referenced this pull request Jul 30, 2026
research/template_project/single_update.py validates the full pydantic
MasterConfig, so its config copy needs every required grpo key too
(same failure mode NVIDIA-NeMo#3400 hit in L1_Functional_Tests_Other_1).

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
@michal2409

Copy link
Copy Markdown
Contributor Author

The L1 failure was research/template_project/single_update.py — it validates the full MasterConfig and its config copy was missing the new required key. Added val_start_at there in 127db4d.

@michal2409

Copy link
Copy Markdown
Contributor Author

/ok to test 127db4d

@yuki-97
yuki-97 merged commit 83753ed into NVIDIA-NeMo:main Jul 30, 2026
182 of 184 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L1 Run doctests, unit tests, and functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants