Repository navigation
[ROCm] Fix EAGLE spec-decode verify silently sampling greedy on HIP - #2
Merged
Merged
Conversation
The sgl_kernel sampling-verify ops (tree_speculative_sampling_target_only, top_k/top_p_renorm_prob) are CUDA/MUSA-only, so HIP fell through eagle_sample's gate to argmax(greedy), silently ignoring temperature/top_p and degenerating into repetition loops at temp>0. Route HIP through the pure-Triton chain sampler (reject_sampling.py) when rejection sampling is active and the batch isn't all-greedy, with a Triton pivot-search top-p renorm (_renorm_top_k_top_p_hip) replacing the CUDA-only renorm ops. The gate is factored into a pure helper (_verify_uses_greedy) that reduces to the original predicate byte-for-byte on every non-HIP platform; CUDA-only imports are guarded so HIP never ImportErrors. top_p>=1.0 is an exact no-op to stay bit-exact with CUDA's top_p_renorm_prob(p, 1.0)==p. Tests: gate-routing truth table (CPU) and renorm-vs-nucleus numerical equivalence (GPU). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without this, the ROCm sampling fix only helps HIP users who pass --speculative-use-rejection-sampling; everyone else still silently gets argmax verify. Default the flag on in _handle_eagle_family when running on HIP with an EAGLE/EAGLE3 topk=1 config whose options are compatible with rejection sampling (default accept thresholds, non-deterministic), unless the user opted in explicitly. The conditions mirror the validation below so the flip never turns a previously-working config into a raise, and CUDA behavior is untouched. Setting the flag makes the draft worker emit the target-vocab proposal (draft_probs) that the Triton chain sampler consumes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
xiaobochen-amd
approved these changes
Aug 27, 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.
Motivation
On ROCm/HIP, EAGLE speculative decoding always commits
argmax(greedy) in verify. It ignorestemperatureandtop_p:_is_hipis in that list because the sampling-verify ops(
tree_speculative_sampling_target_only,top_k_renorm_prob,top_p_renorm_prob) are only importedunder
is_cuda()/is_musa(). So any EAGLE spec-decode run attemperature>0on ROCm decodesgreedy. On long reasoning output that turns into repetition loops. This is not model specific.
Modifications
eagle_utils.py— move the check into a small_verify_uses_greedy(...)helper, which returnsexactly the old answer on every non-HIP platform. Add a Triton top-p renorm (pivot binary search)
and a HIP wrapper to stand in for the CUDA-only sgl_kernel ops, and guard the CUDA-only imports.
HIP then goes through the pure-Triton
chain_speculative_sampling_triton.arg_groups/speculative_hook.py— turn onspeculative_use_rejection_samplingby default on HIPfor EAGLE/EAGLE3 with topk==1, default accept thresholds, and non-deterministic mode, so users do
not have to know about a flag. These conditions are a strict subset of the checks that raise just
below, so turning it on can never break a config that used to work.
Accuracy Tests
Both sides were run on this branch with the same settings. Only the patch differs. GLM-5.2-FP8,
8x MI355X, NEXTN spec decode (
num_steps=1, eagle_topk=1, draft_tokens=2), GPQA-Diamond 198questions,
temperature=1.0 / top_p=0.95,max_tokens=98304.+20.2 points, about 5.2 sigma. The cause is not model quality. On the questions where the old
code finishes, it scores 0.9333, which matches the patched build. It loses because 39.4% of its
answers never finish: greedy verify at
temperature>0falls into repetition loops and burns thewhole budget without giving an answer. You can also see it in wall-clock time (about 2x) and in a
first attempt that died because requests went past the default 1800s per-request timeout.
A probe on a non-greedy batch confirms which branch runs: old code takes
GREEDY(argmax), this PRtakes
SAMPLING. The same run shows thespeculative_hook.pyhalf working, with the server loggingenabling speculative_use_rejection_sampling by defaultand no error.Speed Tests and Profiling
This is a correctness fix; the sampling path costs the same. Throughput under NEXTN spec decode on GSM8K n=200: 740-771 tok/s before, 764 tok/s after.
GSM8K cannot show this bug because it runs greedy, so argmax is already the right answer there.
It is only a no-regression check: 0.945-0.960 old vs 0.940 patched at n=200, throughput 740-771 vs
764 tok/s.
Known gaps (pre-existing, not regressions)
On HIP, EAGLE topk==1 with
--enable-deterministic-inferenceor non-default accept thresholds isleft alone on purpose, because rejection sampling really is incompatible there. HIP tree drafting
(topk>1) still uses greedy, because the Triton chain sampler only handles chains.
_renorm_top_k_top_p_hipdecides whether to run the top-k sort withbool((top_ks < vocab).any()),which is a device sync on every verify step. Passing
sampling_info.need_top_k_samplingandneed_top_p_sampling(what the CUDA branch already uses) would remove it. That measured well under0.5% of a decode step, so I left it out to keep this PR a pure bug fix whose numbers match the code.
How to reproduce
Image
lmsysorg/sglang-rocm:v0.5.17-rocm720-mi35x-20260817,/data/huggingface/GLM-5.2-FP8. UseFP8, not MXFP4: on that image
quark.pyimports a name itsoverrides.pydoes not have, so theserver dies while loading weights.
--limit 198takes one permutation per question (process_docsusesn_repeats=2). You needHF_TOKEN, the dataset is gated. lm-eval has to fall back toreasoning_contentwhencontentisempty, or every graded answer from a thinking model is an empty string.
Two things to watch when probing the branch. Gate the probe on
not sampling_info.is_all_greedy:server warmup sends a greedy request, so a plain one-shot probe tells you nothing. And do not use
output diversity as a test: the old code still returns 8 out of 8 different completions at
temperature=1.0, because the bonus token goes through the normal sampler.AI assistance
This PR was developed with AI assistance (Claude). The AI helped with the rebase and conflict
resolution, wrote the unit tests, and ran the measurements reported above. All code and results were
reviewed and verified by the author on real hardware; the commits carry a
Co-authored-bytrailer.Checklist
CI States
Latest PR Test (Base):⚠️ Run #33050712665⚠️ Run #33050712255
Latest PR Test (Extra):