Skip to content

Has patches, which is not first class user experience and not allowed in inferenceX #2674

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

Closed
ajith-sirra-amd wants to merge 7 commits into from
Closed

Has patches, which is not first class user experience and not allowed in inferenceX #2674

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da13aa6
[AMD] [AGENTX] Kimi-K3 Perf Tuning
AjithSirra Aug 19, 2026
8e77c39
[AMD] [AGENTX] Kimi-K3 Perf Tuning
AjithSirra Aug 19, 2026
e9ad188
[AMD] [AGENTX] Kimi Perf Tuning
AjithSirra Aug 25, 2026
3e34a17
Merge branch 'main' into amd/kimi-k3-agentic-perf-tuning-2.0
ajith-sirra-amd Aug 25, 2026
7ab516b
[AMD] [AGENTX] Kimi Perf Tuning
AjithSirra Aug 25, 2026
32089cf
Merge branch 'amd/kimi-k3-agentic-perf-tuning-2.0' of https://github.…
AjithSirra Aug 25, 2026
39adc30
[AMD] [AGENTX] Kimi Perf Tuning
AjithSirra Aug 25, 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
75 changes: 30 additions & 45 deletions benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,12 @@
set -euo pipefail
set -x

# Agentic trace replay benchmark for Kimi-K3 MXFP4 on MI355X / MI350X (gfx950)
# using vLLM.
#
# The server command is the AMD reference `vllm serve` for this model, i.e. the
# upstream vLLM recipe's amd block (vllm-project/recipes,

@cursor cursor Bot Aug 25, 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.

Unset EVAL_ONLY under set -u

Medium Severity

Removing the EVAL_ONLY="${EVAL_ONLY:-false}" default leaves [ "${EVAL_ONLY}" = "true" ] under set -u. When EVAL_ONLY is unset (common outside the GHA template), the script aborts with an unbound-variable error before replay or eval can run.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 39adc30. Configure here.

# https://recipes.vllm.ai/moonshotai/Kimi-K3) as run in practice:
#
# --trust-remote-code --moe-backend auto --tensor-parallel-size 8
# --load-format auto --gpu-memory-utilization 0.95 --mm-encoder-tp-mode data
# --max-num-seqs 128 --max-num-batched-tokens 4096 --enable-auto-tool-choice
# --tool-call-parser kimi_k3 --reasoning-parser kimi_k3
#
# with env VLLM_ROCM_USE_AITER=1 SAFETENSORS_FAST_GPU=1 AITER_SITUV2_A8W4=1
# AITER_BF16_FP8_MOE_BOUND=0 VLLM_USE_BREAKABLE_CUDAGRAPH=0.
#
# K3 is a 2.8T-parameter natively-multimodal MoE (896 routed experts, 16/token
# plus shared) on Kimi Delta Attention, gated MLA and Attention Residuals, with
# a 1M-token native context.
#
# TP=8 ONLY. The MXFP4 checkpoint is 1.561 TB decimal (1.420 TiB, 96
# safetensors), ~195 GB/GPU across 8 GPUs of the 288 GB part; TP=4 would need
# ~390 GB/GPU and cannot load. Upstream strategy_min_gpus agrees (single_node_tp
# and multi_node_tep both 8, DEP 16+), which is why there is no DP-attention arm.
#
# Required env vars:
# MODEL, TP, CONC, KV_OFFLOADING, TOTAL_CPU_DRAM_GB, RESULT_DIR, DURATION,
# EP_SIZE
#
# Perf-search knobs. Each defaults to the reference command's value, so an
# otherwise-unset run reproduces the reference exactly:
# GPU_MEM_UTIL 0.95 (reference)
# MAX_NUM_BATCHED_TOKENS 8192 (default)
# AITER_A8W4 1 (reference; 0 = aiter a16w4 MoE path)
# LANGUAGE_MODEL_ONLY true
# KV_CACHE_DTYPE fp8 (default for every arm; =auto for a bf16 A/B)
# KV_BLOCK_SIZE unset (unset -> vLLM sizes the page; 128 under fp8)
# MAX_MODEL_LEN 1M
# SPEC_DECODE true (this is the _mtp DSpark recipe; =false for a no-spec A/B)
# SPEC_NUM_TOKENS 2 (DSpark draft length; validated by the _mtp config)

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

wait_for_amd_gpu_clean

du -h /dev/shm

Check warning on line 9 in benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh

View check run for this annotation

Claude / Claude Code Review

Leftover debug command du -h /dev/shm 

Line 9 adds a bare `du -h /dev/shm` with no redirection, comment, or `|| true` guard, unlike this script's other diagnostics (`rocm-smi || true` / `amd-smi || true`). It looks like a leftover debug probe from bring-up (checking /dev/shm capacity for the DRAM KV-offload path) and just adds noise to the benchmark log; under `set -euo pipefail` it could also abort the run if `du` ever returns non-zero.

@claude claude Bot Aug 19, 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.

🟡 Line 9 adds a bare du -h /dev/shm with no redirection, comment, or || true guard, unlike this script's other diagnostics (rocm-smi || true / amd-smi || true). It looks like a leftover debug probe from bring-up (checking /dev/shm capacity for the DRAM KV-offload path) and just adds noise to the benchmark log; under set -euo pipefail it could also abort the run if du ever returns non-zero.

Extended reasoning...

This PR replaces the script's old documentation header with a bare du -h /dev/shm call at line 9, right after wait_for_amd_gpu_clean. The command's output is never captured, tested, or referenced anywhere downstream — it exists purely as a side effect on stdout.

The rest of the script is careful about diagnostics that aren't load-bearing: rocm-smi || true and amd-smi || true (further down in the same file) are both explicitly guarded with || true so a non-zero exit doesn't tank the run, and their purpose (dumping GPU state to the log for debugging) is self-evident from context. du -h /dev/shm has neither the guard nor any comment explaining why it's there. That asymmetry, combined with it being the only such command in the entire benchmarks/ tree (per grep), strongly suggests it's a leftover from local bring-up/debugging of the DRAM KV-offload arm rather than an intentional, permanent diagnostic.

Concretely, the code path is: script starts with set -euo pipefail, sources benchmark_lib.sh, calls wait_for_amd_gpu_clean, then immediately runs du -h /dev/shm unconditionally before check_env_vars even runs. Every invocation of this benchmark — across all 10 concurrency points in the new conc-list — will emit this du output into the log, adding noise with no functional purpose.

There's also a latent robustness gap: because the line has no || true, if du ever exits non-zero (e.g., a transient permission issue or an unreadable entry under /dev/shm), set -e will abort the whole benchmark run before the server is even started. In practice these runs execute as root in the enroot container, so du should always succeed against its own tmpfs — meaning this is a latent risk rather than an observed failure — but it stands out precisely because the pattern used everywhere else in the file (|| true) exists to prevent exactly this class of failure.

To fix: either delete the line entirely (simplest, since nothing consumes its output), or if the intent is to keep an /dev/shm capacity check for debugging DRAM KV-offload sizing, add a comment explaining why and guard it the same way as the other diagnostics, e.g. du -sh /dev/shm || true # sanity-check host shm usage for KV offload.

Step-by-step proof: (1) checkout the script, (2) run it (or just bash -x it up to line 9) with any valid env vars set, (3) observe du -h /dev/shm executes and prints usage for every subdirectory under /dev/shm to stdout/log before check_env_vars is even reached — confirming it fires unconditionally and its output goes nowhere but the log. (4) To see the abort risk, chmod 000 a subdirectory under /dev/shm as a non-root user and rerun — du returns non-zero, and since there's no || true, set -e immediately kills the script, whereas the guarded rocm-smi/amd-smi calls later in the same file would survive an equivalent failure.


check_env_vars MODEL TP CONC KV_OFFLOADING TOTAL_CPU_DRAM_GB RESULT_DIR DURATION EP_SIZE

if [[ -n "${SLURM_JOB_ID:-}" ]]; then
Expand Down Expand Up @@ -81,7 +43,12 @@
resolve_trace_source
install_agentic_deps


# ---- Reference env block ----------------------------------------------------
# Keep ALL of these. Commenting them out does not avoid the AITER FMHA crash:
# that crash is gated on VLLM_ROCM_USE_AITER alone (AiterFlashAttnPrefillBackend
# .is_available() consults only rocm_aiter_ops.is_enabled()), so disabling the
# others just loses the MoE kernels while keeping the failure.
export VLLM_ROCM_AITER_MLA_ASM_PADDING=asm
export VLLM_ROCM_USE_AITER=1
export SAFETENSORS_FAST_GPU=1
Expand Down Expand Up @@ -146,9 +113,6 @@
esac
fi

# ---- LLM server ------------------------------------------------------------
bash "$(dirname "$0")/apply_k3_container_patches.sh"

# ---- Parallelism ------------------------------------------------------------
EP_ARGS=()
if [ "$EP_SIZE" -gt 1 ]; then
Expand All @@ -171,9 +135,23 @@
)
fi

ASYNC_SCHED_ARGS=()
if [ "${ASYNC_SCHEDULING:-0}" != "1" ]; then
ASYNC_SCHED_ARGS=(--no-async-scheduling)
fi

MLA_PREFILL_BACKEND="${MLA_PREFILL_BACKEND:-ROCM_AITER_FA}"
MLA_PREFILL_ARGS=()
if [ -n "$MLA_PREFILL_BACKEND" ]; then
MLA_PREFILL_ARGS=(
--attention-config
"{\"mla_prefill_backend\":\"$MLA_PREFILL_BACKEND\"}"
)
fi

# ---- HIP graph ------------------------------------------------------------
MAX_NUM_SEQS=20
MAX_CUDAGRAPH_CAPTURE_SIZE=60
MAX_NUM_SEQS="${MAX_NUM_SEQS:-$(( CONC * 2 ))}"
MAX_CUDAGRAPH_CAPTURE_SIZE="${MAX_CUDAGRAPH_CAPTURE_SIZE:-$(( MAX_NUM_SEQS * 3 ))}"
CUDAGRAPH_CAPTURE_SIZES="$(seq -s, 1 "$MAX_CUDAGRAPH_CAPTURE_SIZE")"
COMPILATION_CONFIG_ARGS=(--compilation-config "{\"mode\":3,\"cudagraph_mode\":\"FULL_AND_PIECEWISE\",\"max_cudagraph_capture_size\":$MAX_CUDAGRAPH_CAPTURE_SIZE,\"custom_ops\":[\"+fused_rms_norm_gated\"],\"cudagraph_capture_sizes\":[$CUDAGRAPH_CAPTURE_SIZES]}")

Expand Down Expand Up @@ -202,6 +180,13 @@
--max-model-len 1048576
--enable-prefix-caching
--kv-cache-dtype "fp8"
--max-num-batched-tokens 8192
--decode-context-parallel-size 8
--dcp-comm-backend a2a
--attention-backend ROCM_AITER_MLA
--cp-kv-cache-interleave-size 1
"${ASYNC_SCHED_ARGS[@]}"
"${MLA_PREFILL_ARGS[@]}"
"${COMPILATION_CONFIG_ARGS[@]}"
"${SPEC_ARGS[@]}"
"${OFFLOAD_ARGS[@]}"
Expand Down
5 changes: 2 additions & 3 deletions configs/amd-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@
- "DECODE_MTP_SIZE=2"

kimik3-fp4-mi355x-vllm-agentic-mtp:
image: vllm/vllm-openai-rocm:nightly-cb8104839c141609d99f1254459ef3a4f1bd4263
image: vllm/vllm-openai-rocm:nightly
model: moonshotai/Kimi-K3
model-prefix: kimik3
runner: cluster:mi355x-amds
Expand All @@ -631,14 +631,13 @@
framework: vllm
multinode: false
scenarios:
agentic-coding:
- dram-utilization: 0.50
search-space:
- { tp: 8, kv-offloading: none, conc-list: [1, 4, 8] , spec-decoding: mtp}
- { tp: 8, ep: 1, kv-offloading: dram, kv-offload-backend: { name: vllm-simple }, conc-list: [10], spec-decoding: mtp }
- { tp: 8, ep: 1, kv-offloading: dram, kv-offload-backend: { name: vllm-simple }, conc-list: [1, 4, 8, 10, 12, 16, 20, 24, 28, 32], spec-decoding: mtp }

dsr1-fp4-mi355x-sglang-disagg:
image: lmsysorg/sglang-rocm:v0.5.12-rocm720-mi35x-20260519

Check failure on line 640 in configs/amd-master.yaml

View check run for this annotation

Claude / Claude Code Review

Missing perf-changelog.yaml entry for this recipe change 

AGENTS.md's non-negotiable invariants require a new perf-changelog.yaml entry (appended at the tail) for every recipe addition or modification that can affect benchmark performance. This PR rewrites the kimik3_fp4_mi355x_mtp.sh launch script (new DCP/MLA-prefill-backend flags, new MAX_NUM_SEQS/cudagraph formula, image bump to floating :nightly) and expands the conc-list in configs/amd-master.yaml, but perf-changelog.yaml is untouched. Please append an entry documenting these changes before mergi
Comment on lines 634 to 669

@claude claude Bot Aug 19, 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.

🔴 AGENTS.md's non-negotiable invariants require a new perf-changelog.yaml entry (appended at the tail) for every recipe addition or modification that can affect benchmark performance. This PR rewrites the kimik3_fp4_mi355x_mtp.sh launch script (new DCP/MLA-prefill-backend flags, new MAX_NUM_SEQS/cudagraph formula, image bump to floating :nightly) and expands the conc-list in configs/amd-master.yaml, but perf-changelog.yaml is untouched. Please append an entry documenting these changes before merging.

Extended reasoning...

AGENTS.md:21 states as a non-negotiable benchmark invariant: "Every change that can affect benchmark performance and every recipe addition or modification requires a new perf-changelog.yaml entry. The file is append-only and byte-sensitive... append only at the tail." This is explicitly called out as non-negotiable, not a style suggestion, and CLAUDE.md points to AGENTS.md as the canonical source of project instructions.

The PR's diff touches exactly two files — benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh and configs/amd-master.yaml — and does not include perf-changelog.yaml. Yet the change is squarely inside the invariant's scope: it rewrites the vLLM server launch flags for the kimik3-fp4-mi355x-vllm-agentic-mtp recipe (adds --decode-context-parallel-size 8, --dcp-comm-backend a2a, --attention-backend ROCM_AITER_MLA, --cp-kv-cache-interleave-size 1, a new MLA prefill-backend attention-config, and async-scheduling args), replaces the fixed MAX_NUM_SEQS=20 / MAX_CUDAGRAPH_CAPTURE_SIZE=60 constants with a formula derived from CONC, bumps the served image from a pinned digest (nightly-cb8104839c141609d99f1254459ef3a4f1bd4263) to the floating :nightly tag, and collapses/expands the configs/amd-master.yaml search-space from two arms (conc-list: [1,4,8] and conc-list: [10]) into one arm with a ten-point conc-list: [1,4,8,10,12,16,20,24,28,32]. The PR title itself is "Kimi-K3 Perf Tuning 2.0" and its description says the changes "compound into a substantial throughput improvement" — this is definitionally a perf-affecting recipe modification, the exact category the invariant targets.

Nothing in the current tooling would silently let this slip through if the changelog gate runs: the repo carries utils/validate_perf_changelog.py plus changelog gate tests specifically to enforce append-only, byte-sensitive changelog entries per recipe change. A missing tail entry for this PR (there is no entry keyed to PR #2674 or kimik3-fp4-mi355x-vllm-agentic-mtp describing these new flags) would fail that gate, not just look incomplete to a human reviewer.

To fix: append a new entry at the tail of perf-changelog.yaml describing the kimik3-fp4-mi355x-vllm-agentic-mtp config-key change — new DCP/MLA-prefill-backend/async-scheduling flags, the new MAX_NUM_SEQS/MAX_CUDAGRAPH_CAPTURE_SIZE formula, the image bump, and the expanded conc-list — along with a link to this PR, following the same style as the neighboring entries for similar recipe rewrites (e.g. the #2576 GLM-5.2 entry, which documents comparable knob changes).

Step-by-step proof: (1) AGENTS.md:21 requires a changelog entry for "every recipe addition or modification" affecting perf. (2) git diff/PR diff shows only kimik3_fp4_mi355x_mtp.sh and configs/amd-master.yaml changed — no perf-changelog.yaml hunk exists in the diff. (3) The two changed files unambiguously constitute a recipe modification: new server flags, new sizing formula, new image tag, new search-space. (4) Searching perf-changelog.yaml's existing content for kimik3-fp4-mi355x-vllm-agentic-mtp or PR #2674 finds no matching entry — the file's tail predates this PR. (5) Therefore the invariant is violated as written, independent of any subjective judgment call.

model: amd/DeepSeek-R1-0528-MXFP4-v2
model-prefix: dsr1
runner: mi355x-disagg
Expand Down