Log GRPO policy_loss at the optimizer-window scale for every loss type - #7042
Open
behroozazarkhalili wants to merge 1 commit into
Open
Log GRPO policy_loss at the optimizer-window scale for every loss type#7042behroozazarkhalili wants to merge 1 commit into
behroozazarkhalili wants to merge 1 commit into
Conversation
…ss type The grpo, sapo, bnpo, dr_grpo and luspo branches capture policy_loss before dividing by the gradient accumulation normalizer, so the logged value is the optimizer-window loss. The cispo, dapo and vespo branch folds that normalizer into its token-count denominator before the capture, so its policy_loss was one micro-batch contribution, half the value of the other branches at gradient_accumulation_steps=2. The capture now multiplies back by the accumulation factor in train mode; the returned loss is unchanged. The regression test recomputes each of the three objectives and the entropy term from the model's own log probabilities and checks both the returned micro-batch loss and the logged policy_loss. On the previous code every row reports exactly half the expected value. Fixes #7011
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Fixes #7011.
What
policy_losswas logged at two scales depending onloss_type. Thegrpo,sapo,bnpo,dr_grpoandluspobranches capture it before the gradient accumulation rescale, so the logged number is the optimizer-window loss. Thecispo,dapo,vespobranch foldscurrent_gradient_accumulation_steps / steps_per_generationinto its token-count normalizer before the capture, so it logged one micro-batch contribution instead. The capture in that branch now multiplies back by the accumulation factor in train mode (1.0 in eval). The returned loss, and therefore training, is unchanged.Test
test_policy_loss_uses_optimizer_window_scale, parametrized overcispo,dapo,vespo, builds a 3-sample batch by hand, recomputes the per-token objective and the entropy term from_get_per_token_logps_and_entropies, and checks both the returned loss and the loggedpolicy_lossatgradient_accumulation_steps=2, steps_per_generation=4.Verification
On a compute node (job 58031566):
grpo_trainer.py: 3 failed, each logged value exactly half the expected one;steps_per_generationas the factor: 3 failed;Scope
policy_lossis appended only when the entropy bonus is enabled, so the issue's reproduction as written (noentropy_coef) raisesIndexErroron this revision; the test setsentropy_coef=0.2.gspo_tokencarries the same normalizer without apolicy_losscapture; Re-sync gspo_token _compute_loss with GRPOTrainer #6856 resyncs its_compute_lossfrom this file and will pick the corrected block up. GMPO and the replay-buffer trainer have no affected branch.Note
Low Risk
Observability-only fix for logged metrics; training loss computation and gradients are unchanged.
Overview
Fixes inconsistent
policy_lossmetrics forcispo,dapo, andvespo: those branches folded gradient accumulation andsteps_per_generationinto the loss normalizer before logging, so the metric reflected a micro-batch slice while other loss types logged the full optimizer-window value.GRPOTrainer._compute_lossnow multiplies the detachedpolicy_lossbycurrent_gradient_accumulation_stepsin train mode (1.0 in eval). The scalar loss returned for backprop is unchanged.Adds
test_policy_loss_uses_optimizer_window_scale, parametrized over the three loss types, to assert the returned loss and loggedpolicy_lossmatch hand-computed expectations withgradient_accumulation_steps=2andsteps_per_generation=4.Reviewed by Cursor Bugbot for commit 27e6594. Bugbot is set up for automated code reviews on this repo. Configure here.