Skip to content

fix(refit): enforce configured timeouts in legacy GRPO - #4019

Open
seonjinn wants to merge 8 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/fix-legacy-refit-timeout
Open

fix(refit): enforce configured timeouts in legacy GRPO#4019
seonjinn wants to merge 8 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/fix-legacy-refit-timeout

Conversation

@seonjinn

@seonjinn seonjinn commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Legacy GRPO did not pass a configured refit deadline into its weight synchronizer or raw IPC and collective waits. A failed refit could therefore leave run_grpo.py waiting indefinitely even though worker-side watchdog support already existed.

This change:

  • adds optional policy.generation.refit_timeout_s for legacy vLLM runs;
  • validates the value and forwards it to collective and NCCL Reshard synchronizers;
  • applies one shared deadline to the legacy driver waits.

Leaving the setting null preserves current behavior. SingleController keeps using async_rl.generation_fleet_health.refit_timeout_s.

Test plan

  • 92 focused unit, config, and watchdog tests
  • Python pre-commit hooks
  • pyrefly check

Signed-off-by: seonjinn <sna@nvidia.com>
Signed-off-by: seonjinn <sna@nvidia.com>
Signed-off-by: seonjinn <sna@nvidia.com>
…timeout

Signed-off-by: seonjinn <sna@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@seonjinn
seonjinn marked this pull request as ready for review September 10, 2026 08:07
@seonjinn
seonjinn requested review from a team as code owners September 10, 2026 08:07
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn seonjinn added the CI:L1 Run doctests, unit tests, and functional tests label Sep 12, 2026
Signed-off-by: seonjinn <sna@nvidia.com>

# Conflicts:
#	nemo_rl/algorithms/grpo.py
#	nemo_rl/weight_sync/factory.py
#	tests/unit/algorithms/test_grpo.py
#	tests/unit/weight_sync/test_weight_synchronizer.py
@seonjinn

Copy link
Copy Markdown
Contributor Author

/ok to test 99a3492

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.

1 participant