Skip to content

Allow zero loss_mask for remove_sample'd rollouts - #60

Merged
flukeskywalker merged 2 commits into
prodfrom
feat/validator-allow-removed-zero-mask
Sep 8, 2026
Merged

flukeskywalker merged 2 commits into
prodfrom
feat/validator-allow-removed-zero-mask

Conversation

@nightlessbaron

Copy link
Copy Markdown

validate_rollout_for_grpo_training_step rejects any sample with an all-zero loss_mask as a hard error. But rollout sample filters (mask_truncated / mask_token_truncated) legitimately zero a sample's loss_mask via remove_sample to drop it from the gradient while keeping its reward in the group baseline — and the token-mean reducer clamps the denominator, so an all-zero mask is safe.

Downgrade that case to a warning so masking doesn't crash train_actor.

@nightlessbaron
nightlessbaron force-pushed the feat/validator-allow-removed-zero-mask branch from 95526d0 to b98a2bf Compare July 1, 2026 02:56
@nightlessbaron
nightlessbaron marked this pull request as ready for review August 25, 2026 23:01
@nightlessbaron
nightlessbaron requested a review from a team August 25, 2026 23:01
@nightlessbaron
nightlessbaron force-pushed the feat/validator-allow-removed-zero-mask branch from b98a2bf to 346377d Compare August 25, 2026 23:01
Comment thread miles/rollout/filter_hub/rollout_filters.py
nightlessbaron and others added 2 commits September 8, 2026 09:42
… samples

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@flukeskywalker
flukeskywalker force-pushed the feat/validator-allow-removed-zero-mask branch from 7db4793 to 2e05752 Compare September 8, 2026 16:42
@flukeskywalker
flukeskywalker merged commit 6a7966e into prod Sep 8, 2026
4 checks passed
@flukeskywalker
flukeskywalker deleted the feat/validator-allow-removed-zero-mask branch September 8, 2026 16:45
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