Skip to content

fix(ppo): restore offload_after_refit semantics - #2782

Merged
yuki-97 merged 2 commits into
NVIDIA-NeMo:mainfrom
bg51717:fix/ppo-offload-after-refit-onload-regression
Jun 11, 2026
Merged

fix(ppo): restore offload_after_refit semantics#2782
yuki-97 merged 2 commits into
NVIDIA-NeMo:mainfrom
bg51717:fix/ppo-offload-after-refit-onload-regression

Conversation

@bg51717

@bg51717 bg51717 commented Jun 11, 2026

Copy link
Copy Markdown
Member

What does this PR do ?

Restores the offload_after_refit() contract that PR #2530 accidentally broke, so the Megatron PPO policy worker actually offloads params to CPU after a colocated refit instead of leaving them on GPU. Aligns the policy lifecycle with GRPO/SFT/DPO (no finish_training on the policy side), and adds an explicit prepare_for_lp_inference() before refit_policy_generation so params are guaranteed on GPU when entering refit (covers the ckpt-only-step path that previously relied on the removed auto-onload).

Issues

close #2775

@copy-pr-bot

copy-pr-bot Bot commented Jun 11, 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.

@bg51717 bg51717 added the CI:L1 Run doctests, unit tests, and functional tests label Jun 11, 2026
@bg51717

bg51717 commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

/ok to test 76dd30f

@bg51717
bg51717 marked this pull request as ready for review June 11, 2026 16:05
@bg51717
bg51717 requested review from a team as code owners June 11, 2026 16:05
Comment thread nemo_rl/algorithms/ppo.py
@yuki-97 yuki-97 added CI:docs Run doctest and removed CI:L1 Run doctests, unit tests, and functional tests labels Jun 11, 2026
@yuki-97
yuki-97 enabled auto-merge (squash) June 11, 2026 17:11
@yuki-97

yuki-97 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

https://github.com/NVIDIA-NeMo/RL/actions/runs/27351602878/attempts/3?pr=2782

CI:L1 only fail at L1_Functional_Tests_Gym and gb200_L1_Functional_Tests_Gym, which is a known issue in #2784, so run CI:docs instead to merge.

@yuki-97
yuki-97 merged commit 0bffa12 into NVIDIA-NeMo:main Jun 11, 2026
219 of 230 checks passed
pengdurice pushed a commit to pengdurice/RL that referenced this pull request Jun 12, 2026
ashors1 pushed a commit that referenced this pull request Jun 27, 2026
Signed-off-by: bg51717 <biguo@nvidia.com>
Signed-off-by: Anna Shors <ashors@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:docs Run doctest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Megatron offload_after_refit leaves params on GPU (re-onload via offload_before_refit)

2 participants