[AMD][AgentX] Add Kimi-K2.7 MI355X vLLM agentic MTP#2286
Conversation
…orkflow Signed-off-by: haic0 <haic0@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.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 关于重新运行失败任务的文档 |
haic0
added
AMD
full-sweep-enabled
agentx
|
Claude finished @haic0's task in 7m 34s —— View job Review — PR #2286
No blocking issues found; left 2 🟡 inline comments: the Checks that passed: changelog entry is appended at the end of the file and paired with the master-config change; |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - "Add Kimi-K2.7-Code-MXFP4 agentic-coding MTP config on MI355X via vLLM v0.24.0, using EAGLE3 speculative decoding with lightseekorg/kimi-k2.6-eagle3.1-mla" | ||
| - "Mirror the best-config agentic sweep: GPU-only none at conc [4,8] and dram/LMCache at conc [16], duration 3600s, dram-utilization 0.80" | ||
| - "Recipe benchmarks/single_node/agentic/kimik2.7_fp4_mi355x_mtp.sh reuses the Kimi-K2.7 agentic recipe and adds --speculative-config for EAGLE3" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER |
There was a problem hiding this comment.
🟡 WARNING: pr-link still contains PLACEHOLDER instead of the actual PR number.
Why it matters: Every other changelog entry carries a real PR link — this is the permanent provenance record for the perf change, and a dead link breaks traceability for anyone auditing the sweep history.
Fix:
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER | |
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2286 |
There was a problem hiding this comment.
The pr-link at the end of perf-changelog.yaml still reads 'https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER' — unchanged since this comment was posted. Please update it to the canonical link for this PR (#2286) or the XXX placeholder convention used by every other entry in the file.
| # ---- Run benchmark ---------------------------------------------------------- | ||
| build_replay_cmd "$RESULT_DIR" | ||
|
|
||
| run_agentic_replay_and_write_outputs "$RESULT_DIR" |
There was a problem hiding this comment.
🟡 WARNING: The EVAL_ONLY branch present in the sibling recipe (kimik2.5_fp4_mi355x.sh) was dropped — this script unconditionally runs the throughput replay.
Why it matters: Agentic evals are opt-in via --evals-only/--all-evals in generate_sweep_configs.py (mark_eval_entries, MIN_EVAL_CONC = 16), and the LMCache row at conc 16 makes this config eval-eligible. If an eval sweep ever targets this config, the job will set EVAL_ONLY=true but this script will silently run the full 3600s throughput replay instead of run_eval, burning a node-hour and producing no eval artifact.
Fix: Mirror the sibling's tail:
| # ---- Run benchmark ---------------------------------------------------------- | |
| build_replay_cmd "$RESULT_DIR" | |
| run_agentic_replay_and_write_outputs "$RESULT_DIR" | |
| # ---- Run benchmark ---------------------------------------------------------- | |
| if [ "${EVAL_ONLY:-false}" = "true" ]; then | |
| run_eval --port "$PORT" | |
| else | |
| build_replay_cmd "$RESULT_DIR" | |
| run_agentic_replay_and_write_outputs "$RESULT_DIR" | |
| fi |
(If MTP evals are deliberately unsupported here — as in kimik2.5_fp4_b300_mtp.sh — an explicit exit 1 on EVAL_ONLY=true would also work; just don't fall through to the replay.)
| - "Add Kimi-K2.7-Code-MXFP4 agentic-coding MTP config on MI355X via vLLM v0.24.0, using EAGLE3 speculative decoding with lightseekorg/kimi-k2.6-eagle3.1-mla" | ||
| - "Mirror the best-config agentic sweep: GPU-only none at conc [4,8] and dram/LMCache at conc [16], duration 3600s, dram-utilization 0.80" | ||
| - "Recipe benchmarks/single_node/agentic/kimik2.7_fp4_mi355x_mtp.sh reuses the Kimi-K2.7 agentic recipe and adds --speculative-config for EAGLE3" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER |
There was a problem hiding this comment.
🔴 The new perf-changelog.yaml entry (line 4976) uses pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER, but the changelog tooling only recognizes the canonical link for this PR (.../pull/2286) or the literal placeholders XXX / .../pull/XXX. This won't fail PR-time CI, but it will abort the standard merge_with_reuse.sh merge flow when it tries to canonicalize the appended entry — please change it to XXX (or 2286) to match every other entry in the file.
Extended reasoning...
perf-changelog.yaml:4976 sets pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER for the newly appended kimik2.7-fp4-mi355x-vllm-agentic-mtp entry. utils/validate_perf_changelog.py defines the only two accepted forms for a link in an appended entry:
CANONICAL_PR_LINK = re.compile(r"https://github\.com/SemiAnalysisAI/InferenceX/pull/\d+")
PR_LINK_PLACEHOLDERS = {"XXX", "https://github.com/SemiAnalysisAI/InferenceX/pull/XXX"}validate_added_pr_link() requires the link to either match CANONICAL_PR_LINK (a real PR number, i.e. .../pull/2286 for this PR) or be one of PR_LINK_PLACEHOLDERS. The string PLACEHOLDER is neither — it doesn't match \d+ and it isn't the literal token XXX — so this is the only entry in the entire file that uses an unrecognized, non-canonical value. Every other entry (including the two immediately preceding it) uses either a real PR number or XXX.
Where this actually bites: it does not fail this PR's own CI. run-sweep.yml's "Validate perf-changelog matrix" step runs validate_perf_changelog.py's main(), which only calls validate_matrix_compatible_change → process_changelog.py, and process_changelog.py's ChangelogEntry model (in utils/matrix_logic/validation.py) treats pr-link as an untyped/unvalidated alias field — it never calls validate_added_pr_link. That's consistent with the PR's own test-plan output passing.
The enforcement instead happens at merge time, via the repo's standard merge path. utils/merge_with_reuse.sh (lines 168 and 181) invokes prepare_perf_changelog_merge.py, whose canonicalize_appended_links() calls compare_entries(base, head, pr_number=2286) → validate_added_pr_link(link, 2286) for each newly appended entry, before any placeholder-to-real-number rewriting happens. For this entry that call raises:
ChangelogValidationError: new PR entry must use '.../pull/2286' or an XXX placeholder; found '.../pull/PLACEHOLDER'
Step-by-step proof:
- PR [AMD][AgentX] Add Kimi-K2.7 MI355X vLLM agentic MTP #2286 merges via
merge_with_reuse.sh, which is the supported/standard reuse-merge tool for this repo. - The script runs
prepare_perf_changelog_merge.py canonicalize --pr-number 2286 ...to rewrite anyXXX/.../pull/XXXplaceholders in the diff to.../pull/2286. canonicalize_appended_links()first validates each appended entry'spr-linkviacompare_entries()→validate_added_pr_link(".../pull/PLACEHOLDER", 2286)."https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER"is not inPR_LINK_PLACEHOLDERSand does not equal the expected".../pull/2286", so the function raisesChangelogValidationError.merge_with_reuse.shpropagates the failure and the merge aborts — the changelog entry is never canonicalized to point at PR [AMD][AgentX] Add Kimi-K2.7 MI355X vLLM agentic MTP #2286.
Had this entry used the conventional XXX placeholder instead, the merge tool would have auto-rewritten it to https://github.com/SemiAnalysisAI/InferenceX/pull/2286 and proceeded normally, exactly as the two prior entries (pull/2279, pull/2280) presumably were.
Fix: change line 4976 to either pr-link: XXX (or .../pull/XXX) so the merge tool auto-fills the real number, or set it directly to https://github.com/SemiAnalysisAI/InferenceX/pull/2286.
| --port "$PORT" \ | ||
| --prometheus-host 127.0.0.1 \ | ||
| --prometheus-port "$VLLM_ROUTER_METRICS_PORT" \ | ||
| --request-timeout-secs 14400 \ | ||
| --disable-retries > "$ROUTER_LOG" 2>&1 & | ||
| ROUTER_PID=$! | ||
| echo "Router PID: $ROUTER_PID" | ||
| wait_for_server_ready --port "$PORT" --server-log "$ROUTER_LOG" --server-pid "$ROUTER_PID" |
There was a problem hiding this comment.
🔴 kimik2.7_fp4_mi355x_mtp.sh unconditionally runs the throughput trace replay (build_replay_cmd + run_agentic_replay_and_write_outputs) at the end of the script and never checks EVAL_ONLY. Every other agentic recipe -- including kimik2.5_fp4_mi355x.sh, which this PR mirrors -- branches on EVAL_ONLY to call run_eval instead when doing an accuracy-only pass. Add the same if/else branch so eval-only dispatches for this config run the accuracy eval instead of silently running the full replay.
Extended reasoning...
The bug: kimik2.7_fp4_mi355x_mtp.sh ends with an unconditional call to build_replay_cmd "$RESULT_DIR" followed by run_agentic_replay_and_write_outputs "$RESULT_DIR" (lines 375-382). There is no reference to EVAL_ONLY" anywhere in the 388-line file. Contrast this with the script it explicitly claims to mirror, kimik2.5_fp4_mi355x.sh` (lines 115-120):
if [ "${EVAL_ONLY}" = "true" ]; then
run_eval --port "$PORT"
else
build_replay_cmd "$RESULT_DIR"
run_agentic_replay_and_write_outputs "$RESULT_DIR"
fiEvery other agentic-coding recipe in benchmarks/single_node/agentic/ (dsv4_fp4_mi355x_vllm.sh, qwen3.5_fp8_mi355x.sh, dsv4_fp4_b300_vllm.sh, dsv4_fp4_b200_sglang.sh, etc. -- roughly 20+ scripts) has this identical branch. The new recipe simply drops it.
Why this matters / is live, not dead code: EVAL_ONLY is a real mechanism wired end-to-end, not a vestigial flag. .github/workflows/benchmark-tmpl.yml sets EVAL_ONLY: ${{ inputs.eval-only }} before invoking the launch script, and when running in eval-only mode it checks for results*.json afterward, failing the job if none is produced. On the config-generation side, utils/matrix_logic/generate_sweep_configs.py's mark_eval_entries/mark_all_eval_entries explicitly marks agentic-coding entries (the include_agentic path) for eval dispatch when --evals-only/--all-evals is passed -- the exact same mechanism that would apply to this new kimik2.7-fp4-mi355x-vllm-agentic-mtp config, since it declares an agentic-coding scenario like every sibling.
Concrete failure walkthrough:
- A sweep run is dispatched with eval-only mode enabled (either via the workflow input or via
generate_sweep_configs.py --evals-only/--all-evals, which marks agentic entries including this new one for eval dispatch). EVAL_ONLY=true" is exported into the environment beforekimik2.7_fp4_mi355x_mtp.sh` runs.- The script starts the vLLM server exactly as normal, then reaches the tail of the file. Since there is no
if [ "${EVAL_ONLY}" = "true" ]check, it ignores the flag entirely and unconditionally callsbuild_replay_cmd+run_agentic_replay_and_write_outputs, i.e. runs the full ~3600s throughput trace replay instead ofrun_eval --port "$PORT". - This produces the wrong artifact type for an eval-only run (throughput results instead of the lm-eval accuracy
results*.json), so the workflow's post-run check forresults*.jsonfails the job outright -- or, if the replay path happens to also emit a similarly-named file, it silently reports throughput numbers where accuracy numbers were expected.
Fix: Wrap the tail in the same branch used by every sibling recipe:
if [ "${EVAL_ONLY}" = "true" ]; then
run_eval --port "$PORT"
else
build_replay_cmd "$RESULT_DIR"
run_agentic_replay_and_write_outputs "$RESULT_DIR"
fiThis is a copy-paste omission from an otherwise very thorough recipe (this script is 388 lines and clones most of the sibling's logic faithfully), but it breaks the supported eval-only flow for this specific config -- the whole point of which is to validate accuracy, not throughput.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29731629508 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29731629508 |
Restore upstream perf-changelog entries so the Kimi MTP branch remains append-only against current main. Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| echo "Starting vllm server..." | ||
| export PYTHONNOUSERSITE=1 | ||
| export VLLM_SPEC_CONFIG="${VLLM_SPEC_CONFIG:-{\"model\": \"lightseekorg/kimi-k2.6-eagle3.1-mla\", \"method\": \"eagle3\", \"num_speculative_tokens\": 3}}" |
There was a problem hiding this comment.
@haic0 u gotta add
synthetic_acceptance_length https://github.com/SemiAnalysisAI/InferenceX/blob/main/golden_al_distribution/README.md
There was a problem hiding this comment.
@functionstackx Thanks, i will refine it by using synthetic AL.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29884535715 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29884586906 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29884586906 |
Route eval-only runs through run_eval so run-sweep agentic eval jobs produce SWE-bench artifacts. Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve perf-changelog against latest main so run-sweep can validate the Kimi MTP entry as append-only. Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29913348936 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29913348936 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29963838104 |
Add Kimi tool-call and reasoning parsers with auto tool choice so agentic eval requests using tool_choice=auto are accepted by vLLM. Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve perf-changelog against latest main so run-sweep validates the Kimi MTP entry as append-only. Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29963838104 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29975243963 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29975243963 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29997671653 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29997671653 |
seungrokj
added
the
evals-only
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30038590047 |
Signed-off-by: haic0 <haichzha@gbt350-odcdh5-wbb3.png-odc.dcgpu> Co-authored-by: Cursor <cursoragent@cursor.com>
…nto haic-kimi2.7-agentic-mtp-main
Co-authored-by: Cursor <cursoragent@cursor.com>
haic0
added
evals-only
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30084651690 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30084749892 |
haic0
removed
the
evals-only
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30084919887 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30084919887 |
3 participants
Summary
kimik2.7_fp4_mi355x_mtp.sh, mirroring the existing Kimi-K2.7 MI355X agentic vLLM recipe with EAGLE3 speculative decoding vialightseekorg/kimi-k2.6-eagle3.1-mla.kimik2.7-fp4-mi355x-vllm-agentic-mtptoconfigs/amd-master.yamlwith GPU-only rows at conc[4, 8]and LMCache row at conc[16].run-sweep.ymlchangelog flow dispatches the new agentic MTP matrix.Test plan
bash -n benchmarks/single_node/agentic/kimik2.7_fp4_mi355x_mtp.shpython3 utils/matrix_logic/generate_sweep_configs.py test-config --config-files configs/amd-master.yaml --runner-config configs/runners.yaml --scenario-type agentic-coding --config-keys kimik2.7-fp4-mi355x-vllm-agentic-mtp --no-evalspython3 utils/process_changelog.py --changelog-file perf-changelog.yaml --base-ref origin/main --head-ref HEADpytest utils/matrix_logic/test_validation.py utils/matrix_logic/test_generate_sweep_configs.py -q