fix: surface NaN GRPO/RLOO steps in log_history - #6772
Open
tonycoder-hub wants to merge 2 commits into
Open
Conversation
A step whose loss is NaN or Inf is still backpropagated, but `Trainer` replaces it in the reported running loss with the average of the previously logged losses (`logging_nan_inf_filter`, enabled by default). A run that is corrupting its weights therefore reports a clean loss curve, in `log_history` and in any tracker fed from it. Record whether each forward pass produced a finite loss and log the fraction as `frac_non_finite_loss`, so the filtered step leaves a trace. RLOO carries the same change, since it shares the metric machinery and the same silent behavior. Co-authored-by: Tony Coder <407243179@qq.com>
Co-authored-by: Tony Coder <407243179@qq.com>
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
2 participants
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.
What does this PR do?
Trainer logging_nan_inf_filter replaces a NaN loss with the previous average, so a corrupted GRPO step looks clean in log_history. This logs frac_non_finite_loss on GRPO and RLOO so a non-finite step is visible.
Fixes #6702
Before submitting
AI writing disclosure
Who can review?
Anyone in the community is free to review the PR once the tests have passed.
GRPO tests: 157 passed. RLOO tests: 70 passed.
Note
Low Risk
Small observability-only change in compute_loss with no change to loss computation or optimization logic; low risk aside from minor distributed logging overhead.
Overview
GRPO and RLOO now record
frac_non_finite_lossin training/eval metrics so corrupted steps are visible inlog_historyeven when Hugging FaceTrainer’s defaultlogging_nan_inf_filterreplaces NaN/Inf losses with a smoothed finite value in the reportedloss.Each
compute_losscall flags non-finite losses (gathered across processes), appends to the existing metrics pipeline, and still returns the raw loss for backward. GRPO covers both the standard and Liger-kernel paths.Docs for GRPO and RLOO logged metrics describe the new field; regression tests poison the first step with NaN and assert
frac_non_finite_loss > 0on that step only whilelossstays finite (fixes #6702).Reviewed by Cursor Bugbot for commit 4972662. Bugbot is set up for automated code reviews on this repo. Configure here.