fix(grpo): exclude filtered samples from legacy advantage metrics
- Dominant language
- Python
- Stars
- 2k
- Forks
- 561
- Avg merge
- 4d 5h
- Merged PRs (30d)
- 145
Description
## Problem
Legacy GRPO computes `advantages/mean`, `advantages/max`, and `advantages/min` by masking `train_data["advantages"]` with `flat_messages["token_loss_mask"]` alone. When overlong filtering or another sample-level filter sets `train_data["sample_mask"]` to zero, those filtered tokens therefore remain in the reported advantage metrics even though they do not contribute to the policy loss.
SingleController and legacy PPO use the effective loss mask (`token_mask * sample_mask`) for these metrics, so legacy GRPO currently reports different values for the same filtered batch.
This was identified while reviewing #3927: https://github.com/NVIDIA-NeMo/RL/pull/3927#discussion_r3906654277
## Proposed fix
- Use the effective loss mask for `advantages/{mean,max,min}` in both synchronous and asynchronous legacy GRPO paths.
- Preserve the current empty-selection behavior (report `0.0`).
- Add a regression test with a batch containing both valid and truncated/filtered samples to prove the filtered sample does not affect the metrics.
Contributor guide
Assessment
This issue has not been assessed yet.