Repository navigation
[ROCm] EAGLE: carry real top_k into the draft proposal - #55
Merged
Merged
Conversation
sample_draft_proposal decides greedy-vs-sample from temperature alone, but
SamplingParams rewrites temperature 0 to temperature=1.0 with top_k=1, so a
greedy request is indistinguishable from a T=1 one there. It samples a sharp
but non-degenerate distribution and proposes a non-argmax token often enough
to break the draft chain, costing accept length.
Pass the per-request top_ks through and let a greedy row propose its argmax.
The CUDA graph runner has to carry top_ks in a device buffer the same way it
already carries temperatures: its synthetic SamplingBatchInfo used a host-side
placeholder, so without this the correction never sees a real top_k and only
about a fifth of the loss comes back.
TOP_K_ALL rather than -1 for the buffer fill: -1 is not a top_k this pipeline
ever carries, and it reads as top_k <= 1, i.e. greedy, for the padded rows.
GLM-5.2-MXFP4, MI355X, TP4/EP4, EAGLE steps=5 topk=1 draft=6,
GSM8K 200q 5-shot temp=0 conc=8:
baseline before after
accept len 3.864 3.117 3.857
output tok/s 653.3 569.0 653.1
GSM8K accuracy 0.929 - 0.936
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JohnQinAMD
force-pushed
the
fix/eagle-draft-greedy-topk
branch
from
September 11, 2026 17:40
360fe29 to
72ea484
Compare
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
sample_draft_proposaldecides greedy-vs-sample from temperature alone.SamplingParamsrewritestemperature=0totemperature=1.0, top_k=1, so a greedy request is indistinguishable from a genuineT=1one at that point. The draft then samples a sharp-but-not-degenerate distribution and proposes a non-argmax token often enough to break the chain, costing accept length.Fix
Pass the per-request
top_ksthrough and let a greedy row (top_k <= 1) propose its argmax.This stays unbiased:
eagle_samplerenormalises the target by the same per-rowtop_ksbefore the accept test, so a greedy row'spis one-hot —Xequal to the target argmax accepts, anything else rejects and the residual it resamples from ispitself. Both arms commit the target argmax.The CUDA graph runner has to carry
top_ksin a device buffer the way it already carriestemperatures. Its syntheticSamplingBatchInfoused a host-side placeholder, so without the producer half the correction never sees a realtop_kand only about a fifth of the loss comes back. The buffer is filled withTOP_K_ALLrather than-1:-1is not atop_kthis pipeline ever carries, and it would read astop_k <= 1, i.e. greedy, for the padded rows.Usage
No flag. The fix is inside the EAGLE draft proposal, so it applies to any server already running EAGLE with rejection sampling, which is where the regression lives:
It only moves anything for greedy requests (
temperature=0, whichSamplingParamsrewrites totemperature=1.0, top_k=1). Requests that genuinely sample are unaffected: theirtop_kisTOP_K_ALL, the greedy branch does not fire, and the proposal is the same draw as before.Verify from
accept lenin the server's decode log under a greedy workload — it should sit near the no-speculation baseline rather than about 20% below it:Note that the CUDA graph half is what makes this visible. With
--disable-cuda-graphthe realtop_ksalready reached the proposal, so a server run that way shows the fixed accept length with or without this patch.Results
GLM-5.2-MXFP4, MI355X, TP4/EP4, EAGLE
steps=5 topk=1 draft=6,--speculative-use-rejection-sampling, measured against this repo'smain.GSM8K 200q 5-shot
temperature=0, concurrency 8 — accept length is the metric this fix moves:main, rejection sampling offmain, rejection sampling onTurning rejection sampling on costs 18% of accept length; this restores it to the level the argmax path already had.
bench_serving, random 1024/512, 12 prompts, concurrency 1. Read mean TPOT, not median: speculative decoding makes the per-token distribution bimodal, and the median sits inside the accepted-step mode where this fix does not show.main, rejection sampling offmain, rejection sampling onThree repeats of that benchmark on one unchanged server give 310.2 / 312.9 / 309.7 tok/s and mean TPOT 3.02 / 3.00 / 3.03, so the noise floor is about 0.5% and these deltas sit well outside it.
The GSM8K accuracy column does not: three repeats on one unchanged server give 0.940 / 0.945 / 0.920, which spans every number in the table above. Nothing here is an accuracy claim in either direction — the point is that none of the arms produce invalid output.
With
--speculative-use-rejection-samplingoff, which is the default,sample_draft_proposalnever runs and this patch changes nothing.