NVIDIA-NeMo / NVIDIA-NeMo/RL

fix(grpo): exclude filtered samples from legacy advantage metrics

Open
#3,957 0 comments 0 reactions 1 assignee Claimed by @terrykong View on GitHub
accuracy bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.