[AMD] [AGENTX] Kimi Perf Tuning - #2795
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 |
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.
Reviewed by Cursor Bugbot for commit 842eef3. Configure here.
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>
2 participants
Kimi Perf Tuning
Note
Medium Risk
Changes published benchmark topology and serving flags for a large-model agentic recipe; mis-tuning could skew official perf curves or break long-context runs, but there is no application auth or data-path logic.
Overview
Retunes the Kimi-K3 MXFP4 agentic MTP path on MI355X by rewriting
kimik3_fp4_mi355x_mtp.shand narrowing the matrix inamd-master.yaml.The launch script drops the long inline recipe docs, LMCache offload plumbing, and the hard TP=8 guard. It adds AiPerf warmup flags and a refreshed ROCm/vLLM env block (explicit AITER MLA/MoE toggles, K3 KDA safe stages, longer execute/ready timeouts,
fastsafetensorsload). Serving is now concurrency-split: at CONC ≤ 4 it enables DSpark spec (default 8 draft tokens, synthetic acceptance in replay) with DCP off andmax-num-seqs=1; above that it turns spec off, sets DCP=8 with ROCM_AITER_MLA, scales batch limits, and uses a CUDAGraph capture ladder from size 1. KV offload, when enabled, is SimpleCPUOffloadConnector only (per-rank bytes useTOTAL_RANKS). The vLLM CLI also adds--no-async-scheduling,--enable-prompt-tokens-details, and fixed 0.88 GPU memory util.kimik3-fp4-mi355x-vllm-agentic-mtpbumps the pinned vLLM ROCm nightly image and replaces the prior multi-concurrency sweep (including LMCache DRAM arms) with a single search point: TP8, DCP8, conc 52, GPU-resident KV.perf-changelog.yamlrecords the refresh for that config key.Reviewed by Cursor Bugbot for commit fbe084e. Bugbot is set up for automated code reviews on this repo. Configure here.