softmax: accumulate the f16 row sum in f32 - #2586
Open
czoli1976 wants to merge 1 commit into
Open
Conversation
The f16 softmax summed the exponentials into an f16 accumulator, so once a row grew long enough for the running sum to outgrow its own terms the additions rounded away and the row was normalised by too small a divisor. Accumulate in f32 and narrow once, for both the softmax and log-softmax paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 4, 2026
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.
softmax_inner_slice_f16summed the exponentials into an f16 accumulator. Once a row is long enough that the running sum outgrows its own terms, the additions round away entirely and the row gets normalised by too small a divisor — so the output does not sum to 1.The regression test added here fails on main with
row sums to 1.2903354, expected 1.0at row length 4096: attention weights inflated by 29%.How bad, and where
Relative error of the f16 accumulator against the same sum in f32, over normally-distributed logits:
The mechanism is easiest to see in the flat case: with all logits equal, every term after the max subtraction is 1.0, so the sum should be
row_len. f16 spacing at 2048 is 2.0, so from there onsum + 1.0 == sumand the total sticks at 2048 — 50% low for a 4096-long row.This is the default path rather than an opt-in one.
SoftmaxExp::default()isLibc, andScaledMaskedSoftmax::evalconstructsSoftmaxExp::Libcdirectly, so every f16 attention softmax reaches it. Error grows with sequence length, which is the direction long-context inference is going.The fix
Accumulate in f32 and narrow once at the end, for both the softmax and log-softmax branches. Per-element values are untouched — the exponentials are still computed exactly as before and still stored as f16 — so the only thing that changes is the summation, and short rows are unaffected.
It should also be marginally cheaper:
f16 + f16widens to f32 internally in thehalfcrate anyway, so this drops a narrowing per element.Known, not fixed here
FastCompacthas the same flaw one level down:HSoftMaxL2inlinalg/src/generic/reduce.rsaccumulates withlet mut sum = f16::zero()and reduces withreduce_two(a: f16, b: f16). Fixing that means changing the map-reduce accumulator type for f16, which touches the frame and the AVX-512 kernel alongside the generic one — a bigger change than this, andFastCompactis opt-in. Happy to follow up if you want it in the same series. This PR routes theFastCompactresult through f32 for the reciprocal, but cannot repair a sum that was already lost inside the kernel.Tests
tract-core 271, test-f16 2378, test-unit-core 816, tract-linalg 4414 — green.
cargo fmt --allclean;cargo clippyclean (the two remaining warnings are pre-existing on main).🍍