Repository navigation
[ROCm] EAGLE: reject non-finite draft probabilities, not just NaN - #57
Closed
JohnQinAMD wants to merge 1 commit into
Closed
JohnQinAMD wants to merge 1 commit into
JohnQinAMD wants to merge 1 commit into
Conversation
The residual-sampling guard in speculative_sampling_classic_kernel tested
q for NaN with `q_val == q_val`. Infinities pass that test, so a -inf in
draft_probs reaches `p - q` as +inf, norm_sum becomes infinite, and the
cumulative search `total_cumsum > coin * norm_sum` is never true. The
kernel then takes its degenerate fallback.
That fallback is VOCAB_SIZE - 1, and VOCAB_SIZE is the padded tensor width
(154880 for GLM-5.2), not the tokenizer's 154856. The emitted id is an
untrained padding embedding: it decodes to an empty string, so nothing is
visible in the output, but it enters the context and the model degenerates
from there.
Test q against the probability range instead. A comparison with NaN is
false, so the range test rejects NaN along with the infinities and with
negatives.
GLM-5.2-MXFP4, MI355X, TP4/EP4, EAGLE steps=5 topk=1 draft=6,
rejection sampling on, temperature=1.0 top_p=0.95:
before after
fallback emissions 70/1050 0/3437
stray token 0 per 1200 25 to 81 0
GSM8K 200q 5-shot at temperature=0 is unchanged (0.935 / 0.000 invalid /
accept len 3.03), as expected: a greedy batch takes the argmax path and
never reaches this kernel.
The source of the non-finite draft probabilities is a separate defect and
is not addressed here; this kernel has to tolerate them either way, which
is what the NaN guard already intended.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #58, which carries this commit unchanged plus the root cause it defends against: Closing in favour of the combined PR — the two halves are not independently useful. This one alone leaves the draft distribution corrupt (just no longer visible, since the guard zeroes those entries), and the root-cause fix alone leaves a sampler that can still emit |
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.
Problem
speculative_sampling_classic_kernelguards the draft probabilityqagainst NaN withq_val == q_val. Infinities satisfy that test, so a-infindraft_probsreachesp - qas+inf,norm_sumbecomes infinite, and the cumulative searchtotal_cumsum > coin * norm_sumis never true. The kernel then takes its degenerate fallback.That fallback is
VOCAB_SIZE - 1, whereVOCAB_SIZEis the padded tensor width — 154880 for GLM-5.2, against the tokenizer's 154856. The id it emits is an untrained padding embedding. It decodes to an empty string, so nothing is visible in the output, but it is committed to the context and the model degenerates from there into repeated</think>/<tool_call>fragments and stray!(token 0).This only fires where verify actually samples. A greedy batch takes the argmax path and never reaches the kernel, which is why
temperature=0workloads are unaffected.Fix
Test
qagainst the probability range rather than for NaN. A comparison with NaN is false, so the range test rejects NaN along with the infinities and with negatives.Usage
No flag. The kernel is reached when EAGLE verify samples, which on ROCm means
--speculative-use-rejection-sampling:Then send a request with
temperature > 0. Before this patch the reply carries stray!characters at a percent-level rate; after it, none.Results
GLM-5.2-MXFP4, MI355X, TP4/EP4, EAGLE
steps=5 topk=1 draft=6, rejection sampling on,temperature=1.0 top_p=0.95:GSM8K 200q 5-shot at
temperature=0: 0.935 accuracy, 0.000 invalid, accept len 3.03 — unchanged, as expected for a path this patch cannot reach.Not addressed here
Two things this patch deliberately leaves alone.
Where the non-finite draft probabilities come from.
draft_probscarries-infentries, consistently at draft step index 2, 5 to 59 of them per batch. They are a CUDA-graph artefact, not a softmax output:--disable-cuda-graphdraft_probs!per 1200 generated tokensTwo further properties of those entries: each run of them ends on a 64-aligned column, and the run's position climbs monotonically with the decode step at roughly the accept length. A wave64 tile tail whose position tracks the sequence is the shape of an attention-mask fill, so the draft step's softmax output buffer appears to overlap something in the graph's memory pool that writes
-inf. Pinning down the exact pair needs allocation-interval instrumentation of that pool, which is out of scope here.This kernel has to tolerate the values either way, which is what the NaN guard already intended.
The fallback itself.
final_token = VOCAB_SIZE - 1is still an invalid id whenever the vocabulary dimension is padded, even though the guard above makes it unreachable in the case observed. Falling back toargmax(p)would need the pass-1 loop to carry a running max, which is more than this fix should take on.🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ❌ Run #34638460549
Latest PR Test (Extra): ❌ Run #34638460202
Latest PR Test (AMD ROCm 10): ❌ Run #34638460442