Skip to content

[AMD] Add Qwen3.5 FP8 MI355X SGLang disaggregated benchmark #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
functionstackx merged 5 commits into from
May 28, 2026
Merged

[AMD] Add Qwen3.5 FP8 MI355X SGLang disaggregated benchmark #1570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fb9c48
Add Qwen3.5 FP8 MI355X SGLang disaggregated benchmark (PR-1).
May 27, 2026
a5944df
Add perf-changelog entry for qwen3.5-fp8-mi355x-sglang-disagg.
May 27, 2026
8a9baff
Fix misleading 8k1k comment in qwen3.5-fp8-mi355x-sglang-disagg config.
May 27, 2026
99fc3fb
qwen3.5-fp8-mi355x-sglang-disagg: 8k1k row uses dp-attn=false
functionstackx May 28, 2026
224d444
fix: add FRAMEWORK to check_env_vars in qwen3.5 sglang-disagg script
functionstackx May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .github/configs/amd-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,70 @@ qwen3.5-fp8-mi355x-atom-mtp:
- { tp: 4, ep: 1, conc-start: 4, conc-end: 256, spec-decoding: mtp }
- { tp: 8, ep: 1, conc-start: 4, conc-end: 256, spec-decoding: mtp }

Comment thread
ChangLiu0709 marked this conversation as resolved.
qwen3.5-fp8-mi355x-sglang-disagg:

@chunfangamd chunfangamd May 27, 2026

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.

Chang, we will need to rework this PR to keep the git commit history

image: lmsysorg/sglang-rocm:v0.5.11-rocm700-mi35x-20260511
model: Qwen/Qwen3.5-397B-A17B-FP8
model-prefix: qwen3.5
runner: mi355x-disagg
precision: fp8
framework: sglang-disagg
multinode: true
disagg: true
scenarios:
fixed-seq-len:
- isl: 1024
osl: 1024
search-space:
# Matches qwen3.5-fp8-mi355x-sglang TP8/EP1 low-concurrency sweep
- spec-decoding: "none"
conc-list: [ 8, 16, 32, 64, 128, 256, 512 ]
prefill:
num-worker: 1
tp: 8
ep: 1
dp-attn: false
additional-settings:
- "PREFILL_NODES=1"
decode:
num-worker: 1
tp: 8
ep: 1
dp-attn: false
additional-settings:
- "DECODE_NODES=1"
- "DECODE_MTP_SIZE=0"

- isl: 8192
osl: 1024
search-space:
# 1P+1D TP8/EP1 low-concurrency sweep.
# dp-attn intentionally false (matches the 1k1k row): with
# --enable-dp-attention + --moe-a2a-backend mori, sglang auto-promotes
# moe_ep_size=tp_size=8, but is_deepep_class_backend() excludes MoRI,
# so num_shared_slots stays at the global value (1) and the
# (num_experts - num_shared_slots) % moe_ep_size assertion in
# fused_moe_triton/layer.py fires for Qwen3.5 (512 routed + 1 shared).
# Track upstream sglang for a fix; flip back to dp-attn=true once
# MoRI is added to is_deepep_class_backend() or shared-slot
# accounting is reconciled.
- spec-decoding: "none"
conc-list: [ 8, 16, 32, 64, 128, 256, 512 ]
prefill:
num-worker: 1
tp: 8
ep: 1
Comment thread
ChangLiu0709 marked this conversation as resolved.
dp-attn: false
additional-settings:
- "PREFILL_NODES=1"
decode:
num-worker: 1
tp: 8
ep: 1
dp-attn: false
additional-settings:
- "DECODE_NODES=1"
- "DECODE_MTP_SIZE=0"

qwen3.5-fp4-mi355x-sglang:
image: lmsysorg/sglang:v0.5.12-rocm720-mi35x
model: amd/Qwen3.5-397B-A17B-MXFP4
Expand Down
31 changes: 31 additions & 0 deletions benchmarks/multi_node/amd_utils/models.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,37 @@ DeepSeek-R1-0528:
chunked_prefill_size: 262144
cuda_graph_bs_range: "1-128"

Qwen3.5-397B-A17B-FP8:
base_flags: "--decode-log-interval 1000 --log-level warning --watchdog-timeout 3600 --load-balance-method round_robin --kv-cache-dtype fp8_e4m3 --attention-backend aiter --disaggregation-transfer-backend mori --moe-dense-tp-size 1"
mtp_flags: ""
dp_flags: "--moe-a2a-backend mori --enable-dp-attention --enable-dp-lm-head"
prefill:
mem_fraction_static: 0.8
disable_radix_cache: true
dp:
max_running_requests: 24
chunked_prefill_size: "MORI_MAX_DISPATCH_TOKENS_PREFILL * PREFILL_TP_SIZE"
cuda_graph_bs: "1 2 3"
no_dp:
max_running_requests: 128
chunked_prefill_size: 262144
cuda_graph_bs_range: "1-128"
decode:
mem_fraction_static: 0.85
prefill_round_robin_balance: true
dp:
max_running_requests: 4096
chunked_prefill_size: "MORI_MAX_DISPATCH_TOKENS_DECODE * DECODE_TP_SIZE"
cuda_graph_bs_range: "1-160"
ep_only:
max_running_requests: 256
chunked_prefill_size: 262144
cuda_graph_bs_range: "1-256"
no_dp:
max_running_requests: 128
chunked_prefill_size: 262144
cuda_graph_bs_range: "1-128"

DeepSeek-R1-0528-MXFP4-Preview:
base_flags: "--decode-log-interval 1000 --log-level warning --watchdog-timeout 3600 --ep-dispatch-algorithm fake --load-balance-method round_robin --kv-cache-dtype fp8_e4m3 --attention-backend aiter --disaggregation-transfer-backend mori"
mtp_flags: "--speculative-algorithm NEXTN --speculative-eagle-topk 1"
Expand Down
84 changes: 84 additions & 0 deletions benchmarks/multi_node/qwen3.5_fp8_mi355x_sglang-disagg.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#!/usr/bin/env bash

source "$(dirname "$0")/../benchmark_lib.sh"

check_env_vars \
CONC_LIST \
ISL \
OSL \
IMAGE \
SPEC_DECODING \
MODEL_PATH \
PREFILL_NUM_WORKERS \
PREFILL_TP \
PREFILL_EP \
PREFILL_DP_ATTN \
DECODE_NUM_WORKERS \
DECODE_TP \
DECODE_EP \
DECODE_DP_ATTN \
PREFILL_NODES \
DECODE_NODES \
RANDOM_RANGE_RATIO \
FRAMEWORK

if [[ -n "$SLURM_JOB_ID" ]]; then
echo "JOB $SLURM_JOB_ID running on $SLURMD_NODENAME"
fi

set -x

# Use upstreamed multi_node scripts (no external clone needed)
cd "$GITHUB_WORKSPACE/benchmarks/multi_node/amd_utils" || exit 1

# Set up SGL launch script-specific environment variables
export TIME_LIMIT="08:00:00"
export MODEL_PATH=$MODEL_PATH
export MODEL_NAME=$MODEL_NAME
export CONTAINER_IMAGE=$IMAGE

if [[ "${PREFILL_EP:-1}" -eq 1 ]]; then
export PREFILL_ENABLE_EP=false
else
export PREFILL_ENABLE_EP=true
fi

if [[ "$PREFILL_DP_ATTN" == "true" ]]; then
export PREFILL_ENABLE_DP=true
else
export PREFILL_ENABLE_DP=false
fi

if [[ "${DECODE_EP:-1}" -eq 1 ]]; then
export DECODE_ENABLE_EP=false
else
export DECODE_ENABLE_EP=true
fi

if [[ "$DECODE_DP_ATTN" == "true" ]]; then
export DECODE_ENABLE_DP=true
else
export DECODE_ENABLE_DP=false
fi

# Launch jobs based on ISL/OSL
# Replace ' ' in CONC_LIST with 'x' such that the concurrency list is represented
# by a list of numbers delimited by 'x'. This is because of how the underlying launch script
# expects the concurrencies.
JOB_ID=$(bash ./submit.sh $PREFILL_NODES \
$PREFILL_NUM_WORKERS \
$DECODE_NODES \
$DECODE_NUM_WORKERS \
$ISL $OSL "${CONC_LIST// /x}" inf \
${PREFILL_ENABLE_EP} ${PREFILL_ENABLE_DP} \
${DECODE_ENABLE_EP} ${DECODE_ENABLE_DP} \
${PREFILL_TP} ${DECODE_TP} \
${RANDOM_RANGE_RATIO} \
${NODE_LIST:-})

if [[ $? -ne 0 ]]; then
echo "Failed to submit job" >&2
exit 1
fi

echo "$JOB_ID"
Comment thread
cursor[bot] marked this conversation as resolved.
10 changes: 10 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3171,3 +3171,13 @@
description:
- "Validates measured-power aggregation pipeline (PR #1558) on both NVIDIA (H200) and AMD (MI355X) hardware — different SMI tools (nvidia-smi vs amd-smi), different CSV schemas (power.draw [W] vs socket_power), same aggregator. No config change. Entry intentionally kept past merge so run-sweep produces canonical agg JSONs with avg_power_w + joules_per_output_token on main for both vendors, seeding the dashboard's day-zero data."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1558

- config-keys:
- qwen3.5-fp8-mi355x-sglang-disagg
description:
- "Add Qwen3.5-397B-A17B-FP8 MI355X SGLang disaggregated prefill-decode benchmark"
- "Image: lmsysorg/sglang-rocm:v0.5.11-rocm700-mi35x-20260511"
- "1P+1D TP8/EP1 smoke sweep for 1k1k and 8k1k (conc 8-512); MoRI transfer backend"
- "Add models.yaml server flags and multinode launch script qwen3.5_fp8_mi355x_sglang-disagg.sh"
- "8k1k row uses dp-attn=false (matches 1k1k): with --enable-dp-attention + --moe-a2a-backend mori, sglang auto-promotes moe_ep_size=tp_size=8, but is_deepep_class_backend() excludes MoRI, so num_shared_slots stays at the global value (1) and the (num_experts - num_shared_slots) % moe_ep_size assertion in fused_moe_triton/layer.py fires for Qwen3.5 (512 routed + 1 shared). Track upstream sglang; flip back to dp-attn=true once MoRI is added to is_deepep_class_backend() or shared-slot accounting is reconciled."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1570

@cursor cursor Bot May 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Changelog entry indentation inconsistent with existing entries

Medium Severity

The new perf-changelog.yaml entry starts at 0-space indentation (- config-keys:) while all surrounding entries use 2-space indentation ( - config-keys:). In YAML, list items at different indentation levels belong to different structural contexts. This means process_changelog.py may not parse the new entry as part of the same changelog list, potentially causing the CI sweep to skip the qwen3.5-fp8-mi355x-sglang-disagg config entirely.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 224d444. Configure here.

7 changes: 6 additions & 1 deletion runners/launch_mi355x-amds.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ if [[ "$IS_MULTINODE" == "true" ]]; then
# Ensure root-owned files are cleaned up even on early exit to prevent
# EACCES errors when the next GH Actions job checks out on this runner.
# Always preserve slurm logs as CI artifacts for debugging.
# KEEP_LOGS=1 disables the trap entirely (local-debug knob).
cleanup_and_save_logs() {
if [[ -n "${GITHUB_ACTIONS:-}" && -n "${JOB_ID:-}" ]]; then
local art_dir="$GITHUB_WORKSPACE/benchmark_artifacts"
Expand All @@ -69,7 +70,11 @@ if [[ "$IS_MULTINODE" == "true" ]]; then
fi
sudo rm -rf "$BENCHMARK_LOGS_DIR" 2>/dev/null || true
}
trap cleanup_and_save_logs EXIT
if [[ "${KEEP_LOGS:-0}" == "1" ]]; then
trap '' EXIT
else
trap cleanup_and_save_logs EXIT
fi
Comment on lines +73 to +77

@claude claude Bot May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 The new KEEP_LOGS=1 path only disarms the EXIT trap (lines 56-60), but line 178's unconditional sudo rm -rf "$BENCHMARK_LOGS_DIR" 2>/dev/null || true (and the $BENCHMARK_LOGS_DIR/logs rm at line 167) still fires on the happy path. So a successful run with KEEP_LOGS=1 and no GITHUB_ACTIONS set wipes slurm_job-${JOB_ID}.{out,err} anyway — exactly the local-debugging use case the PR description claims to support. Fix: gate the rms at lines 167 and 178 on "${KEEP_LOGS:-0}" != "1" as well.

Extended reasoning...

What the bug is

The PR documents KEEP_LOGS=1 as the local-debugging knob that "skips EXIT trap log cleanup" so users can inspect Slurm logs under benchmark_logs/ after a run. The implementation (runners/launch_mi355x-amds.sh:56-60) replaces the EXIT trap with trap '' EXIT when KEEP_LOGS=1:

if [[ "${KEEP_LOGS:-0}" == "1" ]]; then
    trap '' EXIT
else
    trap 'sudo rm -rf "$BENCHMARK_LOGS_DIR" 2>/dev/null || true' EXIT
fi

That correctly preserves logs when the script exits early (e.g. before slurm finishes). But the script ALSO has two inline sudo rm -rf calls at the bottom of the IS_MULTINODE=true branch that are unrelated to the trap:

  • Line 167: sudo rm -rf "$BENCHMARK_LOGS_DIR/logs" 2>/dev/null || true
  • Line 178: sudo rm -rf "$BENCHMARK_LOGS_DIR" 2>/dev/null || true

Neither is gated on KEEP_LOGS. On the happy path, both fire after the result-collection phase, so KEEP_LOGS=1 does not actually preserve logs after a successful run — only after an early/failed exit. The PR's own test-plan item ("re-run with KEEP_LOGS=1 and confirm Slurm logs are retained under benchmark_logs/") fails.

Step-by-step proof

Walk through the local-debug case: KEEP_LOGS=1, no GITHUB_ACTIONS set, IS_MULTINODE=true, successful run.

  1. Line 56-60: KEEP_LOGS=1 → trap '' EXIT disarms the EXIT trap. ✓
  2. Lines 62-86: Slurm job submitted, log file $BENCHMARK_LOGS_DIR/slurm_job-${JOB_ID}.out (and .err) gets populated as the job runs. ✓
  3. Lines 88-158: Job completes, result JSONs are copied to $GITHUB_WORKSPACE — but the slurm_job-*.{out,err} files themselves are still in $BENCHMARK_LOGS_DIR. ✓
  4. Line 167: sudo rm -rf "$BENCHMARK_LOGS_DIR/logs" — removes the per-job subdir. Unconditional.
  5. Lines 170-175: if [[ -n "${GITHUB_ACTIONS:-}" ]] block is skipped because we're running locally — so the cp of slurm_job-*.{out,err} to $ARTIFACT_DIR never happens.
  6. Line 178: sudo rm -rf "$BENCHMARK_LOGS_DIR" 2>/dev/null || true — wipes the entire directory, including the slurm out/err files the user wanted to inspect. This is the bug.
  7. Script exits cleanly. EXIT trap is no-op (good — but irrelevant, the damage is already done by step 6).

User looks under benchmark_logs/ — empty. Feature does not work as advertised in its primary use case.

Why existing code doesn't prevent it

The trap disarmament only matters for paths where control exits before reaching line 178 (e.g. the exit 1 at line 81 when the job dies before creating the log file). On any successful run, control always reaches line 178 because there's no early return. The KEEP_LOGS check was added in only one place when it needed to be added in three.

The GH Actions branch at lines 170-175 partially masks the issue in CI: when GITHUB_ACTIONS is set, the slurm out/err files are first copied to $ARTIFACT_DIR ($GITHUB_WORKSPACE/benchmark_artifacts) before the final rm wipes $BENCHMARK_LOGS_DIR. So CI still gets logs uploaded as artifacts — but local users (the entire target audience for KEEP_LOGS=1) get nothing.

Impact

Documented user-facing feature (KEEP_LOGS=1 for local debugging) does not work on the happy path. The PR's own test plan checkbox cannot be satisfied as written. Severity is normal because the feature is new in this PR and ships broken — not a pre-existing issue.

Fix

Gate both inline rms on KEEP_LOGS, mirroring the trap setup:

if [[ "${KEEP_LOGS:-0}" != "1" ]]; then
    sudo rm -rf "$BENCHMARK_LOGS_DIR/logs" 2>/dev/null || true
fi

…and similarly at line 178. (Or factor into a helper.) The line-167 rm is arguably also safe to skip on KEEP_LOGS=1 since $BENCHMARK_LOGS_DIR/logs is the per-job subdir with the in-container result tree, which is exactly what a local debugger would want to inspect alongside the slurm out/err files.

Comment thread
cursor[bot] marked this conversation as resolved.
Comment on lines +73 to +77

@claude claude Bot May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 BLOCKING: KEEP_LOGS=1 only disarms the EXIT trap (line 73-74) but the inline sudo rm -rf "$BENCHMARK_LOGS_DIR/logs" at line 191 still fires unconditionally on the happy path, wiping the per-job result tree that a local debugger would want to inspect.

Why it matters: The feature's stated purpose (retain logs under benchmark_logs/ for local debugging) doesn't work for successful runs — only for early/failed exits. The PR's own test plan item ("re-run with KEEP_LOGS=1 and confirm Slurm logs are retained") cannot be satisfied.

Fix: Gate line 191 on KEEP_LOGS as well:

Suggested change
if [[ "${KEEP_LOGS:-0}" == "1" ]]; then
trap '' EXIT
else
trap cleanup_and_save_logs EXIT
fi
+ if [[ "${KEEP_LOGS:-0}" != "1" ]]; then
+ sudo rm -rf "$BENCHMARK_LOGS_DIR/logs" 2>/dev/null || true
+ fi


SCRIPT_NAME="${EXP_NAME%%_*}_${PRECISION}_mi355x_${FRAMEWORK}.sh"
if [[ "$FRAMEWORK" == "sglang-disagg" ]] || [[ "$FRAMEWORK" == "vllm-disagg" ]]; then
Expand Down