Repository navigation
Filter zero-advantage groups in AsyncGRPO - #7609
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
thanks, as discussed yesterday, please link/close all the issues that asked for "dynamic sampling" |
…o-advantage # Conflicts: # tests/experimental/test_async_grpo_trainer.py
…o-advantage # Conflicts: # docs/source/async_grpo_trainer.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aa84c8a. Configure here.
qgallouedec
left a comment
qgallouedec
left a comment
There was a problem hiding this comment.
Thanks, lgtm overall. A few things before merging:
reward_std == 0.0(L1224) misses some constant groups: with a constant reward of 0.1 and 3, 6, 7, 12 or 64 generations, numpy gives a std around 1e-17 🥲. Checking that all scored rewards are equal is exact.- As planned in #7841 and #7835, let's log GRPO's
frac_reward_zero_std(a fraction of groups) instead ofrollout/zero_advantage_groups_total. rewardnow covers every scored group, butreward_stdandrewards/<func>still come from the kept samples (L1259-L1260), soreward_stdreads higher than in GRPO. Can we compute them in the worker before the filter, likereward, and dropreward_kept?- The description still has the old metric names and says
rewardcovers trained groups only. "Closes #3708" also needs to be in the description
lewtun
left a comment
lewtun
left a comment
There was a problem hiding this comment.
Thanks for adding this @AmineDiro ! I left a few comments on top of Quentin's review.
Turbo nit on naming: we use "filtered" in the code (_filtered_groups, TestFilterZeroAdvantageGroups), "dropped" in the docs and "zero advantage" in the metric. Should we stick to just one wording consistently (e.g. dropped)?
| | `rollout/vllm_preemptions_total` | requests the server preempted during the logging window (vLLM's `num_preemptions_total`, summed over engines). Reported only: a weight sync resets the server's caches, which preempts every running request, so it is not a pressure signal | | ||
| | `rollout/zero_advantage_groups_total`, `reward_kept` | groups dropped because every rollout got the same reward, and the mean reward of the groups that were trained on. `reward` is the policy's reward over every scored group, dropped ones included, so it is the one to track | | ||
|
|
||
| A group whose rollouts all received the same reward (or none of which could be scored) has zero advantage everywhere, so the worker drops it instead of enqueuing it: it would cost a forward and backward and move nothing. The loop does not resample to replace a dropped group, so `batch/groups_per_step` goes down while the batch keeps its `samples_per_step` samples. A dropped group still counts toward an epoch, which stays one pass over the prompts. |
There was a problem hiding this comment.
Are you sure batch/groups_per_step goes down? IIUC each step still fills up to samples_per_step from the kept groups, and the metric counts trained groups
There was a problem hiding this comment.
You're right! I mainly asked claude to right this feature it seemed to be easy enough to one shot 🫠 🫠 🫠 Thanks for reviewiing I should looked at this closer.
Fixed
There was a problem hiding this comment.
Good to see humans still have an edge :)
…o-advantage # Conflicts: # tests/experimental/test_async_grpo_trainer.py
lewtun
left a comment
lewtun
left a comment
There was a problem hiding this comment.
Thanks for iterating! LGTM with a small nit about the docs
| | `rollout/vllm_running_requests` | requests the server's engines are decoding (vLLM's `num_requests_running`, summed over engines). The gap to `rollout/inflight` is what sits in the API server process, building responses | | ||
| | `rollout/vllm_queued_requests` | requests the server has accepted but not yet scheduled (vLLM's `num_requests_waiting`, summed over engines). The cap does not grow while this is non-zero | | ||
| | `rollout/vllm_preemptions_total` | requests the server preempted during the logging window (vLLM's `num_preemptions_total`, summed over engines). Reported only: a weight sync resets the server's caches, which preempts every running request, so it is not a pressure signal | | ||
| | `frac_reward_zero_std`, `reward_kept` | fraction of groups dropped because every scored reward was equal, or fewer than two rollouts were scored (GRPO only counts the first case), and the mean reward of the groups that were trained on. `reward` is the policy's reward over every scored group, dropped ones included, so it is the one to track | |
There was a problem hiding this comment.
turbo nit: put reward_kept on a new line so it's easier to read
There was a problem hiding this comment.
ok noted I thiink we'll redo the docs in stable migration, I'll note this !
Rebase the group metrics on the zero-advantage group filtering from #7609: `frac_reward_zero_std` now covers `group/active_frac`, `group/active_total` and `group/scored_total`, so they are removed, and `group/inactive_reward_mean` becomes `group/dropped_reward_mean`, keyed on the same test that drops the group. Rename `group/unscorable_total` to `group/unscorable_count` after #8186, so `_reduce_metric` sums it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rollout worker now drops a group whose rollouts all got the same reward, or none of which could be scored, before it reaches the trainer. Every advantage in such a group is zero, so it costs a forward and backward and moves nothing. This is DAPO's dynamic sampling without resampling to a fixed group count.
The loop does not replace a dropped group, so
batch/groups_per_stepgoes down while the batch keepssamples_per_stepsamples. Filtered groups never reach the collator, so an epoch takes more prompts thanlen(train_dataset).Metrics:
rollout/groups_filtered_zero_advantagecounts dropped groups andreward_filteredis their mean reward.rewardcovers trained groups only, so the two are meant to be read together.Note
Medium Risk
Changes GRPO training dynamics, rollout queue protocol, epoch boundaries, and checkpoint resume semantics; incorrect wiring could skip prompts or stall on the queue, but scope is confined to async GRPO rollout/scoring.
Overview
Async GRPO now skips groups that cannot produce a learning signal (all scorable rollouts share the same reward, so advantages are zero everywhere). The rollout worker enqueues a bare
group_idon the buffer instead ofRolloutSamples; the trainer records these in_dropped_groupsand keeps pulling until the step still reachessamples_per_step, so more prompts may be generated per optimizer step when many groups are filtered.Epoch stop,
rollout_state.jsonresume, and the rollout worker protocol are updated so a dropped prompt still counts as consumed (trained ∪ dropped), matching one pass over the dataset without resampling to replace filtered groups.AsyncGRPOConfignow requiresnum_generations >= 2.Logging distinguishes
reward(mean over every scored group, including dropped) from per-samplereward_keptand rolloutfrac_reward_zero_std. Advantage scoring for partially unscorable groups is tightened so centering runs on scorable rollouts only. Docs and tests cover buffer behavior, checkpoints, and epoch counting with synthetic drops.Reviewed by Cursor Bugbot for commit 9dcb8b5. Bugbot is set up for automated code reviews on this repo. Configure here.