fix: normalize rewards by explicit sample groups#2204
Open
morluto wants to merge 2 commits into
Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RolloutManager._post_process_rewardsinferred GRPO reward groups from the total sample count. When a custom rollout returns uneven numbers of samples per prompt, the fallback reshaped the reward vector to one row and centered rewards across the entire batch.For groups of sizes 4, 3, and 4, a prompt whose three rewards are all
5.0received advantages of-1.090909instead of zero.Changes
group_indexand restore results in input order.group_indexis missing; reject mixed or uneven unidentified groups instead of silently using a global baseline.Validation
\n-
uvx --from pre-commit pre-commit run --all-files --show-diff-on-failure --color=alwaysPYTHONPATH=. python3 tests/test_reward_utils.py(5 passed)ruff check slime/ray/rollout.py slime/rollout/reward_utils.py slime/rollout/_fanout_test_helpers.py tests/test_reward_utils.py tests/test_qwen2.5_0.5B_fanout_short.pypython3 -m py_compile slime/ray/rollout.py slime/rollout/reward_utils.py slime/rollout/_fanout_test_helpers.py tests/test_reward_utils.py tests/test_qwen2.5_0.5B_fanout_short.pypython3 .github/workflows/generate_github_workflows.pytests/test_qwen2.5_0.5B_fanout_short.pywas not run locally because it requires four GPUs; it remains registered in the GPU CI matrix.