Skip to content

[AMD][Fix] AgentX HIP TPOT regression when SGLANG_SIMULATE_ACC_LEN is set - #40598

Merged
ch-wan merged 1 commit into
sgl-project:mainfrom
yichiche:fix/hip-skip-rej-samp-simulate-acc
Sep 22, 2026
Merged

ch-wan merged 1 commit into
sgl-project:mainfrom
yichiche:fix/hip-skip-rej-samp-simulate-acc

Conversation

@yichiche

@yichiche yichiche commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

This PR fixes an AgentX TPOT regression on HIP after #37134.

Agent mode (InferenceMax AgentX) sets SGLANG_SIMULATE_ACC_LEN=3.39 for Qwen3.5 (match-expected / real-draft-token). The server still runs a full EAGLE verify, then overwrites accept length. #37134 auto-enables --speculative-use-rejection-sampling on HIP, so every decode step also runs the Triton chain sampler; that result is then thrown away. Extra verify work shows up as ~14% worse AgentX TPOT (TP4 conc=4) vs pre-#37134 / a 0915 baseline.

Fixed-length MTP does not set SGLANG_SIMULATE_ACC_LEN, and the client is greedy (temperature=0), so verify stays on argmax and does not hit this regression.

Skip the HIP auto-enable when SGLANG_SIMULATE_ACC_LEN > 0. Real eval / serving (env unset or < 0) still auto-enables as in #37134.

Modifications

  • _should_auto_enable_hip_rejection_sampling now takes simulate_acc_len and returns false when simulate_acc_len > 0.
  • _handle_eagle_family passes envs.SGLANG_SIMULATE_ACC_LEN into that gate.
  • Explicit --speculative-use-rejection-sampling is unchanged (the helper already no-ops when the flag is already on).
  • Unit test test_simulate_acc_len_stays_off in test/registered/unit/spec/test_eagle_gate_routing.py.

Accuracy Tests

No new EVAL_ONLY / quality run. The EVAL_ONLY=true AgentX path does not set SGLANG_SIMULATE_ACC_LEN, so HIP still auto-enables rejection sampling as in #37134. This change only affects the simulated-accept throughput path, whose accept length is overwritten after verify. The new unit test covers the auto-enable predicate for simulate_acc_len=3.39 vs -1.0.

Benchmarking and Profiling

InferenceX arm qwen3.5-fp4-mi355x-sglang-agentic-mtp, recipe benchmarks/single_node/agentic/qwen3.5_fp4_mi355x_sglang_mtp.sh, TP4, conc=4, DURATION=1200. Same container (rocm/sgl-dev:v0.5.19-rocm720-mi35x-20260915), weights, GPUs, and recipe; only the SGLang SHA / this patch changes.

SGLang TPOT mean (ms) Interactivity mean (tok/s/user) total tok/s requests ok
832ec39cc (pre-#37134-era good) 3.1 317.9 21163.7 350/357
8d08dfdab (main, #37134 in tree) 3.6 277.5 19124.3 334/341
8d08dfdab + this patch 3.2 315.6 21158.2 350/357

First-bad commit for the AgentX drop was 7eedd57ab (#37134). Parent 2f7d04da5 was still good.

Checklist

Review Process

  1. Ping Merge Oncalls to start the PR flow. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • /tag-run-ci-label, /rerun-failed-ci, /tag-and-rerun-ci
  4. After green CI and required approvals, ask Merge Oncalls to merge.

CI States

Latest PR Test (Base): 🚫 Run #35624210597
Latest PR Test (Extra): ✅ Run #35676026555
Latest PR Test (AMD ROCm 10): ⏳ Run #35624210574

@yichiche yichiche added the run-ci CI: run the baseline test suite on this PR label Sep 21, 2026
@yichiche yichiche changed the title [Fix] Skip HIP EAGLE rejection-sampling auto-enable when SGLANG_SIMULATE_ACC_LEN is on [AMD][Fix] AgentX HIP TPOT regression when SGLANG_SIMULATE_ACC_LEN is set Sep 21, 2026
@1am9trash 1am9trash added the amd label Sep 22, 2026

@1am9trash 1am9trash left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Skip sampling with fixed SGLANG_SIMULATE_ACC_LEN case (agentic workload).
Only impact on is_hip() code path.

@ch-wan
ch-wan merged commit bc30fa1 into sgl-project:main Sep 22, 2026
214 of 274 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

amd run-ci CI: run the baseline test suite on this PR speculative-decoding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants