NVIDIA-NeMo / NVIDIA-NeMo/RL

fix(grpo): keep masked Gym failures out of prompt-group statistics

Open
#4,061 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

## Priority

**Wave 0 — immediate training correctness**, followed by Wave 2 ingestion of Gym's top-level verifier fields.

## Problem

NeMo-RL currently reads `mask_sample` from an agent-private nested path. Direct synchronous and asynchronous GRPO paths zero the sample loss multiplier but omit the estimator `valid_mask`, so a masked infrastructure reward can still alter valid siblings' group baseline and advantages. SingleController already passes its final sample mask.

## Wave 0 essential fix

- Pass the final sample mask as `valid_mask` in direct synchronous and asynchronous GRPO, matching SingleController.
- Apply the same exclusion semantics to GRPO, GDPO, and Reinforce++ estimators that accept the mask.
- Use one minimum rule: masked members do not vote; a group with too few valid members for the estimator produces no group-relative learning signal.
- Add focused mixed-valid, all-masked, too-few-valid, and execution-mode parity tests.
- Continue reading the existing nested mask during this phase.

## Wave 2 Gym-schema integration

After Gym #2608 lands:

- Prefer top-level `mask_sample`; retain the nested path as a bounded compatibility fallback.
- Preserve the mask through each data-plane path that demonstrably transforms or drops it.
- Preserve `failure_kind` and bounded `failure_reason` only where required for diagnostics; do not require universal queue/replay propagation before fixing estimator correctness.

## Deferred or excluded scope

- Retry, replacement, or resampling policy for no-result Gym failures; those must be resolved before estimator input under Gym #2135 and caller policy.
- Failure-kind metrics as a prerequisite to mask correctness.
- Exhaustive replay, slicing, and dynamic-sampling matrices without evidence those paths discard the mask.
- Gym transport, provider retry, or session cleanup.

## Dependencies

Wave 0 has no Gym dependency. Wave 2 depends on Gym #2608's stable top-level fields. Gym #3179 supplies stable names for optional diagnostics. This issue is required for an NeMo-RL release consuming masked Gym results, but does not block Gym #2750 completion.

## Acceptance criteria

- [ ] Direct synchronous GRPO, direct asynchronous GRPO, and SingleController pass equivalent valid masks to estimators.
- [ ] A masked reward never changes a valid sibling's baseline or normalized advantage.
- [ ] Groups with insufficient valid members produce no invalid group-relative signal.
- [ ] Mixed and all-masked behavior is consistent across supported estimators and execution modes.
- [ ] After Gym #2608, top-level `mask_sample` takes precedence over the bounded legacy fallback.
- [ ] Tested data-plane transformations preserve mask alignment.

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.