Skip to content

fix(grpo): group advantages by rollout identity - #3959

Merged
yfw merged 3 commits into
super-v3.5-posttrainingfrom
yifu/cherry-pick-pr3882-super-v3.5
Sep 2, 2026
Merged

fix(grpo): group advantages by rollout identity#3959
yfw merged 3 commits into
super-v3.5-posttrainingfrom
yifu/cherry-pick-pr3882-super-v3.5

Conversation

@yfw

@yfw yfw commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Backports the current state of fix(grpo): group advantages by rollout identity #3882 at source head fc1a3c193357ee7a64374141a7a0c5ec8e597ed0 to super-v3.5-posttraining.
  • Groups GRPO advantages by rollout sampling identity instead of tokenized prompt equality.
  • Preserves rollout-group identity through legacy, TransferQueue, dynamic-sampling, and Single Controller paths.

The source PR is still open. This side-branch backport intentionally snapshots its current three-commit state while the remaining main-branch review discussion continues.

Backport notes

  • Cherry-picked all three source commits with -x, preserving authorship and trailers.
  • Adapted the Single Controller regression changes from the renamed main-branch test file to the release branch's existing tests/unit/single_controller/test_single_controller.py; no unrelated main-only tests were brought over.

Testing

  • 17 targeted unit tests passed.
  • pre-commit run --all-files passed, including Ruff, Ruff format, Pyrefly, and recipe checks.

Source: #3882

aroshanghias-nvd and others added 3 commits September 1, 2026 18:57
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
(cherry picked from commit d5398b7)
ruff-format (v0.9.9, as pinned in .pre-commit-config.yaml) was not clean on
four files touched by this PR; every offending hunk is a line this PR adds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
(cherry picked from commit 1508125)
input_ids was the GRPO group key before this PR; both consumers are now
reward_group_ids, leaving the upstream flatten with no readers. Every
generation batch built an unused [B, S_prompt] tensor, multiplied by
dynamic_sampling_num_gen_batches under DAPO, and re-ran
PackedTensor.flattened_concat over every image on VLM recipes.

Removes the flatten, the NeMo-Gym branch assignment, and the trailing
del input_ids (which would otherwise NameError).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
(cherry picked from commit fc1a3c1)
@yfw
yfw requested review from a team as code owners September 2, 2026 02:00
@copy-pr-bot

copy-pr-bot Bot commented Sep 2, 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.

@yfw
yfw merged commit 4c58cc1 into super-v3.5-posttraining Sep 2, 2026
10 checks passed
@yfw
yfw deleted the yifu/cherry-pick-pr3882-super-v3.5 branch September 2, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants