fix(loss): return fp32 empty log-probs from the chunked path - #2827
Closed
yueming-yuan wants to merge 1 commit into
Closed
yueming-yuan wants to merge 1 commit into
yueming-yuan wants to merge 1 commit into
Conversation
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy and
maocheng23
as code owners
August 31, 2026 05:20
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
yueming-yuan
force-pushed
the
yueming/fix-empty-logprob-dtype
branch
from
August 31, 2026 05:21
0d3cfbc to
abe8274
Compare
yueming-yuan
force-pushed
the
yueming/fix-empty-logprob-dtype
branch
from
August 31, 2026 05:22
abe8274 to
75cbdb5
Compare
Collaborator
Author
|
Folded into #2818. |
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.
Summary
With model-precision logits (#2764/#2818), a CP rank whose contiguous half holds no response logits returns its empty log-probs as
logits.new_zeros((0,))— in bf16/fp16 — while every other rank returns fp32 CE outputs. The dtype leaks throughallgather_cp_redistribute(zeros(dtype=value.dtype)) into the cross-rankdist.nn.all_reduce, and the byte-size mismatch crashes the collective:Before #2764 the logits reaching this code were always fp32 on the Megatron backend, so the empty tensor's dtype could not diverge. Reachable with allgather-CP + bf16 whenever one rank's half contains only prompt tokens (long prompts).
The fix pins the empty log-probs/entropy of the chunked (non-true-on-policy) path to fp32, matching the dtype the fused CE produces on non-empty ranks. The true-on-policy path is untouched: its non-empty outputs stay in model precision, so its empty tensor already matches.
Tests
Covered by the 2-rank allgather-CP consistency tests in #2826 (stacked on this PR): the
contiguous_empty_rankcase crashes on this exact path without the fix and passes with it.