fix: request float32 logits from the fused JSD projection GEMM - #1433
Open
KyleMylonakisProtopia wants to merge 7 commits into
Open
KyleMylonakisProtopia wants to merge 7 commits into
KyleMylonakisProtopia wants to merge 7 commits into
Conversation
`fused_linear_jsd_forward` documents that it computes the logits in FP32, but the projection matmul ran in the input dtype and only cast afterwards, so the logits were already rounded to 8 (bf16) or 11 (fp16) mantissa bits before the cast executed. cuBLAS accumulates a bf16 GEMM in FP32 internally; the kernel discarded those bits on the store and then cast the rounded result back up. The JSD scalar hides this (<=0.04% error) because it is dominated by large terms, but `dx` is a difference of nearly-equal distributions, so the rounding cancels catastrophically: gradients were off by 4-23% in bf16. Ask the GEMM for an FP32 output via `torch.mm(..., out_dtype=torch.float32)`. This is numerically equivalent to upcasting both operands and running an FP32 GEMM -- a bf16xbf16 or fp16xfp16 product is exactly representable in FP32 -- but keeps the low-precision tensor cores and needs no FP32 copy of the head. Gradient error drops to 0.04-0.71%, and end-to-end fwd+bwd is 1.03-1.07x faster and memory-neutral. Platforms without `out_dtype` support keep the previous expression, mirroring the existing `accum_dtype` grad-weight guard. Add a regression test comparing against an FP32-projection oracle. The existing `TorchLMHeadJSD` cannot detect this: it performs the same inert cast and so matches the buggy kernel exactly, and its `torch.rand` operands cluster the logits too tightly to expose the rounding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 3, 2026
Contributor
Author
|
Any chance of getting this in? It would fix a significant loss of precision and be faster than the current implementation for virtually no change in memory overhead? Seems like a no-brainer. |
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
fused_linear_jsd_forwarddocuments that it computes the logits in FP32:It does not. The matmul runs in the input dtype, so the logits are rounded to 8 (bf16) or 11 (fp16)
mantissa bits before the cast executes — the cast changes the container, not the contents. cuBLAS
already accumulates a bf16 GEMM in FP32; the kernel discards those bits on the store, then casts the
rounded result back up.
The JSD scalar hides this (≤0.04% error) because it is dominated by large terms.
dxdoes not: it isa difference of nearly-equal distributions, so the rounding cancels catastrophically and the
gradients are off by 4–23% in bf16.
LigerFusedLinearJSDis the fused path for bf16 distillation,so this lands on distillation training runs.
This asks the projection GEMM for an FP32 output instead of casting after the fact.
Closes #1432
Why
out_dtyperather than upcasting the operandstorch.mm(x, w.t(), out_dtype=torch.float32)is numerically equivalent to casting both operands toFP32 and running an FP32 GEMM: a bf16×bf16 or fp16×fp16 product is exactly representable in FP32
(8+8 and 11+11 mantissa bits both fit in 24) and the accumulation is FP32 either way. Unlike an
explicit upcast it keeps the low-precision tensor cores and needs no FP32 copy of the head.
Isolated projection GEMM,
BT=2048, H=4096, V=128256:(x @ w.t()).to(fp32)(before)torch.mm(..., out_dtype=fp32)(this PR)Accuracy
Relative error of
max|Δ|for the gradients against a reference differing from the kernel only inprojecting in FP32.
BT=256, H=1024, V=32000,randninputs, weight scaled to the stated logit std:grad_inputbefore → aftergrad_weightbefore → afterPerformance
End-to-end fwd+bwd through
LigerFusedLinearJSD, bf16, mean of 10 iterations after 3 warmups:accum_dtypeNoneNoneNonefloat32float32float32Faster, and memory-neutral end-to-end. (The isolated GEMM saves the bf16 logits buffer, but the
caching allocator absorbs it at the kernel level — so no memory win is claimed here.)
This changes bf16/fp16 numerics by default
Deliberately, and with no new flag: there is no trade-off to expose, since the FP32 projection is
more accurate, faster, and memory-neutral. Unlike
accum_dtype— which is opt-in because FP32accumulation costs a real
V x H x 4buffer — this costs nothing, so gating it would leave everyuser on the inaccurate path. The kernel's own comment already promises this behavior.
Compatibility:
test_correctness,test_correctness_with_ignore_index,test_correctness_functional,test_correctness_all_ignoredand
test_ampcases, which compare against the bf16-projectionTorchLMHeadJSDoracle.out_dtypeis honored undertorch.autocast(bfloat16)(verified) and compiles undertorch.compile(fullgraph=True)(verified).out_dtypesupport (torch < 2.8, non-CUDA, sm < 80) keep today's expression, sothere is no regression and no new OOM risk there. This mirrors how the
accum_dtypegrad-weightpath in this same function already guards its
torch.addmm(..., out_dtype=...)fast path.grad_input, theaccum_dtypegrad-weight path,fused_linear_jsd_backward, thedispatch("jsd_loss_and_grad", ...)inner kernel and the autograd signature are all untouched, sothe cuTile / CuTeDSL JSD backends are unaffected.
The existing
_ADDMM_SUPPORTS_OUT_DTYPEgate is renamed_MM_SUPPORTS_OUT_DTYPEsince it now guardsboth
torch.mmandtorch.addmm— same torch 2.8 feature, same value. Happy to drop the rename ifyou would rather keep the diff to the projection alone.
What this does not fix: AMP with a float32 head
The guard requires the operands to share a dtype, so it deliberately skips the autocast case.
torch.amp.custom_fwdis applied withoutcast_inputs, so autocast stays enabled inside the forwardand harmonizes a bf16 hidden state with an fp32
lm_headitself. That path has the same magnitude oferror (
grad_input13.18%,grad_weight11.40% at the shapes above) and this PR does not improve it.That is a scope decision, not an oversight: unlike the same-dtype case there is no free fix. Rounding
the fp32 head ourselves once per chunk duplicates autocast's weight cache and still loses the head's
mantissa (13.18% → 3.23%); refusing the downcast for an fp32 GEMM gets 13.18% → 0.50% but overrides
the low-precision matmul the user explicitly asked autocast for and gives up tensor cores. Both are
trade-offs worth their own discussion, so they are recorded in #1432 rather than folded in here.
(Also worth knowing:
aten::mm.dtypeis not on autocast's promotion list, so passingout_dtypeinside an autocast region raises rather than promoting.)
test_ampis unaffected and still passes.Testing
New regression test
test_logits_projection_is_not_rounded_to_input_dtype, parametrized overdtype x beta. It compares against an FP32-projection oracle rather than the existingTorchLMHeadJSD, which cannot detect this defect because it performs the same inert cast (a bf16nn.Linearfollowed by.to(torch.float32)) and so matches the buggy kernel exactly. The existingcases also draw both operands from
torch.rand, clustering the logits at mean ≈ 127 / std ≈ 5;randnat a realistic spread is what exposes the rounding.Verified as a real regression test: with the kernel change reverted, all 5 active parametrizations
fail at 1.25%–22.90%; with it, all pass under 1%.
TorchLMHeadJSDis intentionally left alone — itstill passes and legitimately documents parity with an eager bf16
lm_head.The
float16, beta=0.5combination is explicitly skipped with its reason: there the referencegradient peaks at ~4e-6, below float16's smallest normal (6.1e-5), so quantization of the output
buffer dominates and the projection cannot be isolated. The FP32 oracle itself misses the 1% bar
there, so asserting it would be testing float16's exponent range, not this change.
make checkstyleis clean.make test:main(control, same machine)The +5 passed is exactly this PR's new parametrizations. The 2 failures are pre-existing on
main: I ran the same suite on an unmodified checkout and it fails the identical two tests,test/transformers/test_grpo_loss.py::test_grpo_loss_vs_trl[2-128-1000-0.04-{False,True}-luspo-sequence-1.5],with byte-identical NaN gradient tensors. They also pass in isolation and at file scope (104 passed,
17 skipped), so they are order-dependent within the full run.
test_grpo_loss.pyandchunked_loss/grpo_loss.pycontain no reference to JSD orfused_linear_jsd. Happy to file thatseparately if it is not already tracked.
Every JSD-adjacent suite is green on this branch:
Tolerances in
test/ops/test_fused_linear_jsd.pyare deliberately left alone: that fileparametrizes over every registered backend, whose inner JSD kernels differ.