[AMD] [AGENTX] Kimi Perf Tuning - #2795
ajith-sirra-amd wants to merge 18 commits into
Conversation
Signed-off-by: Sirra <asirra@amd.com>
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
Signed-off-by: Sirra <asirra@amd.com>
ajith-sirra-amd
added
AMD
agentx
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Signed-off-by: Sirra <asirra@amd.com>
…com/SemiAnalysisAI/InferenceX into amd/kimi-k3-agentic-perf-tuning-4.0
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33498326153 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if [ "$CONC" -le 4 ]; then | ||
| SPEC_NUM_TOKENS="${SPEC_NUM_TOKENS:-8}" | ||
| SPEC_ROWS=$(( SPEC_NUM_TOKENS + 1 )) | ||
| SPEC_ARGS=(--speculative-config "{\"model\":\"Inferact/Kimi-K3-DSpark\",\"num_speculative_tokens\":$SPEC_NUM_TOKENS,\"method\":\"dspark\",\"attention_backend\":\"TRITON_MLA\",\"kv_cache_dtype\":\"fp8\",\"draft_sample_method\":\"probabilistic\",\"rejection_sample_method\":\"synthetic\",\"synthetic_acceptance_length\":4.0}") | ||
| MAX_NUM_SEQS=1 | ||
| MAX_BATCHED_TOKENS=8192 | ||
| DCP_SIZE=1 | ||
| else | ||
| MAX_NUM_SEQS=$(( CONC + CONC / 4 )) | ||
| if [ "$MAX_NUM_SEQS" -gt 80 ]; then MAX_NUM_SEQS=80; fi | ||
| MAX_BATCHED_TOKENS=16384 | ||
| DCP_SIZE=8 | ||
| fi |
There was a problem hiding this comment.
🔴 For CONC>4 the else branch sets DCP_SIZE=8/MAX_BATCHED_TOKENS but never populates SPEC_ARGS, so the server starts with no --speculative-config at all. The second amd-master.yaml search-space arm (conc-list [8,16,32,48,52]) is labeled spec-decoding: mtp but every one of its concurrencies is >4, so this 'mtp' benchmark arm silently runs as a non-speculative baseline. Fix: give the else branch (CONC>4) its own SPEC_ARGS/SPEC_NUM_TOKENS just like the CONC<=4 branch so every conc value in an mtp arm actually enables DSpark speculative decoding.
Extended reasoning...
SPEC_ARGS is initialized to () and only populated inside if [ "$CONC" -le 4 ]; the else branch (line 77-82) only sets MAX_NUM_SEQS/MAX_BATCHED_TOKENS/DCP_SIZE, leaving SPEC_ARGS empty. VLLM_CMD later expands "${SPEC_ARGS[@]}" (line 131) which is a no-op, so vllm serve runs without --speculative-config. Old script's case statement gave nonzero SPEC_NUM_TOKENS for CONC in {1,2,4,8,10,12,14}; the new threshold only covers CONC<=4, silently dropping speculative decoding for CONC=8,16,32,48,52 even though amd-master.yaml's second search-space row explicitly sets spec-decoding: mtp for exactly those concurrencies. Results collected for that arm at conc>=8 will reflect baseline (no-spec) throughput/latency, not MTP-accelerated numbers, while being reported/labeled as the mtp benchmark.
Verification: normal. SPEC_ARGS=() is initialized at kimik3_fp4_mi355x_mtp.sh:68 and populated ONLY inside the if [ "$CONC" -le 4 ] branch (line 73). The else branch (lines 77-82, CONC>4) sets MAX_NUM_SEQS/MAX_BATCHED_TOKENS/DCP_SIZE but never touches SPEC_ARGS, so it stays empty. VLLM_CMD expands "${SPEC_ARGS[@]}" at line 131, which is a no-op when empty, so vllm serve starts with no… | normal.…
| SPEC_ARGS=() | ||
| SPEC_ROWS=1 | ||
| if [ "$CONC" -le 4 ]; then | ||
| SPEC_NUM_TOKENS="${SPEC_NUM_TOKENS:-8}" | ||
| SPEC_ROWS=$(( SPEC_NUM_TOKENS + 1 )) | ||
| SPEC_ARGS=(--speculative-config "{\"model\":\"Inferact/Kimi-K3-DSpark\",\"num_speculative_tokens\":$SPEC_NUM_TOKENS,\"method\":\"dspark\",\"attention_backend\":\"TRITON_MLA\",\"kv_cache_dtype\":\"fp8\",\"draft_sample_method\":\"probabilistic\",\"rejection_sample_method\":\"synthetic\",\"synthetic_acceptance_length\":4.0}") |
There was a problem hiding this comment.
🔴 The rejection_sample_method is now always "synthetic" for the spec-decode arm; the EVAL_ONLY branch that used "block" (real target-logit verification) was dropped, so running this script with EVAL_ONLY=true now silently commits synthetic-accepted drafts instead of verifying against the target model, corrupting the accuracy score. Fix: restore the EVAL_ONLY check so eval runs use rejection_sample_method="block" and only non-eval runs use synthetic acceptance, as the sibling kimik3_fp4_b300_vllm_mtp.sh (which explicitly documents this exact hazard) still does.
Extended reasoning...
Old script: if [ "${EVAL_ONLY:-false}" = "true" ] selected rejection_sample_method "block", else "synthetic". New script (lines 70-73) builds SPEC_ARGS unconditionally with "rejection_sample_method":"synthetic","synthetic_acceptance_length":4.0 regardless of EVAL_ONLY. Later (line 144) if [ "${EVAL_ONLY:-false}" = "true" ]; then run_eval still runs the accuracy eval against a server started with synthetic acceptance, so every draft token is accepted without checking target logits, making eval scores meaningless/inflated for CONC<=4 (the only branch where SPEC_ARGS is set) — exactly the bug the b300 sibling script's comment warns against.
Verification: normal. The diff removes the EVAL_ONLY guard that previously selected rejection_sample_method "block" for eval runs. New lines 70-73 unconditionally build SPEC_ARGS with "rejection_sample_method":"synthetic","synthetic_acceptance_length":4.0 (the CONC<=4 arm), and line 144 still runs run_eval --port "$PORT" when EVAL_ONLY=true. So an EVAL_ONLY run at CONC<=4 now serves with…
| OFFLOAD_ARGS=() | ||
| if agentic_kv_offload_enabled; then | ||
| CPU_BYTES_PER_RANK=$(( TOTAL_CPU_DRAM_GB * 1000 * 1000 * 1000 / TOTAL_RANKS )) | ||
| OFFLOAD_ARGS=(--kv-transfer-config "{\"kv_connector\":\"SimpleCPUOffloadConnector\",\"kv_role\":\"kv_both\",\"kv_connector_extra_config\":{\"cpu_bytes_to_use_per_rank\":$CPU_BYTES_PER_RANK,\"lazy_offload\":false}}") |
There was a problem hiding this comment.
🟡 (optional) KV_OFFLOAD_BACKEND is no longer validated or dispatched on: the old case statement (vllm-simple vs lmcache vs error) is gone, so any non-empty, non-"none" value silently produces the same SimpleCPUOffloadConnector config, ignoring the requested backend. Fix: re-add a check that KV_OFFLOAD_BACKEND equals the supported value(s) for this script (e.g. "vllm-simple") and error otherwise, instead of unconditionally building OFFLOAD_ARGS whenever agentic_kv_offload_enabled is true.
Extended reasoning...
benchmark_lib.sh's sourced validation (lines 88-109) only checks KV_OFFLOADING is none/dram and KV_OFFLOAD_BACKEND is non-empty when dram; it never checks the backend name. Previously this script's own case/esac (removed) rejected unsupported names like a typo or "lmcache" (which this script no longer implements) with an explicit error. Now, if KV_OFFLOAD_BACKEND=lmcache or any other non-empty string is passed, agentic_kv_offload_enabled() (line 97) returns true and the script silently builds the vllm-simple SimpleCPUOffloadConnector config anyway, running a benchmark under a different KV-offload mechanism than what was requested/labeled, with no error.
Verification: nit. Factually accurate: the diff removes the case "${KV_OFFLOAD_BACKEND:-}" in vllm-simple)...lmcache)...*) echo "Error: unsupported KV_OFFLOAD_BACKEND"...esac dispatch. The replacement at kimik3_fp4_mi355x_mtp.sh:96-100 unconditionally builds SimpleCPUOffloadConnector whenever agentic_kv_offload_enabled is true and never reads KV_OFFLOAD_BACKEND. The sourced validation in… | Severity:…
| SPEC_ROWS=1 | ||
| if [ "$CONC" -le 4 ]; then | ||
| SPEC_NUM_TOKENS="${SPEC_NUM_TOKENS:-8}" | ||
| SPEC_ROWS=$(( SPEC_NUM_TOKENS + 1 )) | ||
| SPEC_ARGS=(--speculative-config "{\"model\":\"Inferact/Kimi-K3-DSpark\",\"num_speculative_tokens\":$SPEC_NUM_TOKENS,\"method\":\"dspark\",\"attention_backend\":\"TRITON_MLA\",\"kv_cache_dtype\":\"fp8\",\"draft_sample_method\":\"probabilistic\",\"rejection_sample_method\":\"synthetic\",\"synthetic_acceptance_length\":4.0}") | ||
| MAX_NUM_SEQS=1 | ||
| MAX_BATCHED_TOKENS=8192 | ||
| DCP_SIZE=1 | ||
| else | ||
| MAX_NUM_SEQS=$(( CONC + CONC / 4 )) | ||
| if [ "$MAX_NUM_SEQS" -gt 80 ]; then MAX_NUM_SEQS=80; fi | ||
| MAX_BATCHED_TOKENS=16384 | ||
| DCP_SIZE=8 | ||
| fi | ||
| export DCP_SIZE |
There was a problem hiding this comment.
🟡 (optional) DCP_SIZE is now unconditionally derived from CONC (1 or 8) instead of honoring the DCP_SIZE env var the workflow sets from configs/amd-master.yaml's dcp-size field, so that field silently has no effect (base script used DCP_SIZE="${DCP_SIZE:-8}", respecting the passed-in value). Fix: default from the incoming ${DCP_SIZE:-...} env var per branch instead of hardcoding, so config edits to dcp-size actually change server behavior instead of being silently discarded.
Extended reasoning...
benchmark-tmpl.yml sets env DCP_SIZE from inputs.dcp-size, itself populated from amd-master.yaml's per-arm dcp-size field via generate_sweep_configs.py (lines ~322/957/1052). Before this diff the script did DCP_SIZE="${DCP_SIZE:-8}", so that config value drove --decode-context-parallel-size. Now lines 74-82 always set DCP_SIZE=1 (CONC<=4) or DCP_SIZE=8 (CONC>4) regardless of the env var, and line 83 exports the overwritten value. Currently both yaml arms happen to match, masking the bug, but any future arm that sets a different dcp-size (e.g. testing dcp-size=4 at high CONC) will silently run with 8 instead, with no warning, and result metadata (via benchmark_lib.sh's "dcp_size": ${DCP_SIZE:-1}) will misleadingly report the actually-used value as if it were intentional.
Verification: nit. The factual claim is correct: the new script overwrites DCP_SIZE unconditionally rather than honoring the env var. In the diff, the CONC branch sets DCP_SIZE=1 and the else branch sets DCP_SIZE=8, followed by export DCP_SIZE; there is no ${DCP_SIZE:-...} fallback anywhere, unlike the base's DCP_SIZE="${DCP_SIZE:-8}". The env var really is plumbed from config:… | nit. The env-var…
Signed-off-by: Sirra <asirra@amd.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33498871333 |
Signed-off-by: Sirra <asirra@amd.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33499561779 |
Signed-off-by: Sirra <asirra@amd.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33583361234 |
…com/SemiAnalysisAI/InferenceX into amd/kimi-k3-agentic-perf-tuning-4.0
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec99717. Configure here.
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
This branch has not been deployed
2 participants
Kimi Perf Tuning
Note
Medium Risk
Benchmark-only changes, but they alter published agentic performance geometry (offload backend, conc/DCP/MTP split, and container pin), so regression or incomparability with prior Kimi-K3 submissions is possible.
Overview
Retunes Kimi-K3 MXFP4 agentic MTP on MI355X by rewriting
kimik3_fp4_mi355x_mtp.shand updatingkimik3-fp4-mi355x-vllm-agentic-mtpinamd-master.yaml.The launch script now splits geometry by concurrency: low conc (1/2/4) uses DCP=1 + DSpark MTP with a golden synthetic acceptance-length table (default 8 draft tokens) for comparable throughput runs; higher conc uses DCP=8 without speculative decode because MTP’s
TRITON_MLAdraft path conflicts with DCP. It bumps ROCm/AITER env flags, switches toFULL_DECODE_ONLYcudagraphs with tunedmax-num-seqs/ batched-token limits,fastsafetensors, and--no-async-scheduling, and drops the LMCache sidecar in favor ofSimpleCPUOffloadConnectorwhen DRAM offload is enabled.The benchmark matrix pins a new vLLM ROCm nightly image, raises dram-utilization to 0.80, and narrows the search space to c1 GPU-resident plus c32/c72/c76 with DCP 8 and
vllm-simpleoffload (replacing the prior LMCache multi-point sweep).perf-changelog.yamlrecords the image change.Reviewed by Cursor Bugbot for commit 042b7fb. Bugbot is set up for automated code reviews on this repo. Configure here.