Repository navigation
Fix NaN from the small-batch CUDA sum forward when a child is -inf - #60
Merged
Merged
Conversation
…was -inf The small-batch CUDA sum forward (batch < 64, autotuned against Triton) runs an online logsumexp over each node's children with the running max starting at -inf. When the first children a warp reads are -inf, `__expf(m - nmx)` is exp(-inf - (-inf)) = NaN, and the running sum stays NaN even after finite children follow. With 10% of the children at -inf, every output was NaN; an HMM with exact-zero emission entries returned NaN for every row of `pc(x)` at batch 1-32, while batch 64 (Triton) was correct. The other CUDA sum kernels and the BlockScale small-batch forward already handle -inf; this one did not. The running max now starts at -FLT_MAX. Every update is then well defined: a -inf child adds p * exp(-inf) = 0, the first finite child scales the empty sum by exp(-FLT_MAX - x) = 0, and a node whose children are all -inf gets log(0) = -inf. On finite inputs the results are bit-identical to before, and the speed is unchanged (within 1% for every config, batch 1-48, hidden 256-2048). A guard on the new max in the loop was also correct but measured 10-110% slower. The new test calls the kernel directly with every config and compares it with an fp64 reference on inputs with leading -inf runs, scattered -inf and an all -inf column; it fails on the old kernel. The existing CUDA-vs-Triton test passes whenever the autotuner picks Triton, so it could not catch this. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.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
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
The small-batch CUDA sum forward (
layer/kernels/c/smallbatch_forward_sum.cu, used below batch 64 when the autotuner picks it over Triton) returned NaN whenever a child value was −inf. This PR fixes it with no speed cost.The bug
The kernel computes each node's logsumexp online, with the running max starting at −inf:
If the first children a warp reads are −inf, the running sum becomes NaN and stays NaN, even after finite children follow. The final merge across warps has the same problem when a whole split is −inf.
Measured on
main(a85d063):pc(x)Any PC whose product values can be −inf is affected at small batch: zero-probability evidence, zero parameters, Indicator / Literal leaves. The other CUDA sum kernels and the BlockScale small-batch forward already handle −inf; this one never did.
The fix
The running max starts at
-FLT_MAXinstead of −inf. Every update is then well defined:p * exp(-inf) = 0;exp(-FLT_MAX - x) = 0;log(0) = -inf.Verification
Old vs fixed kernel, each built as its own extension and called with the exact arguments the sum layer passes (hidden size 256 / 1024 / 2048, batch 1–48, all four SPLIT configs):
New test:
test_small_batch_forward_cuda_neg_inf_childrencalls the kernel directly with every config, so it doesn't depend on which kernel the autotuner picks, and compares against fp64. It fails on the old kernel. The existingtest_small_batch_forward_cuda_matches_tritonpasses whenever the autotuner picks Triton, so it could not catch this.Full suite: 1249 passed, 30 skipped on one GPU. The 4 extra skips compared with a 4-GPU run are the tests that need at least 2 GPUs.
🤖 Generated with Claude Code