chore(agentx): clean up outdated non-MTP recipes - #2531
Conversation
|
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 关于重新运行失败任务的文档 |
|
Claude finished @cquil11's task in 4m 2s —— View job Review of PR #2531
SummaryOne blocking issue, posted inline: both master configs were modified (8 config keys removed) but Everything else checks out:
Note: I could not run |
1b3b4c1 to
5ec74f9
Compare
Delete only the DeepSeek-V4 and GLM-5.2 AgentX configs and scripts tied to purged source runs. Preserve MTP, fixed-sequence, and unrelated platform coverage. [skip-sweep]
5ec74f9 to
26b5c87
Compare
| - { tp: 8, ep: 8, conc-start: 128, conc-end: 256 } | ||
| - { tp: 8, ep: 8, dp-attn: true, conc-start: 64, conc-end: 1024 } | ||
|
|
||
| dsv4-fp4-b200-vllm-agentic: | ||
| image: vllm/vllm-openai:nightly-dev-x86_64-cu13.0.1-904e4ec | ||
| model: deepseek-ai/DeepSeek-V4-Pro | ||
| model-prefix: dsv4 | ||
| runner: cluster:b200-dgxc | ||
| precision: fp4 | ||
| framework: vllm | ||
| multinode: false | ||
| scenarios: | ||
| agentic-coding: | ||
| - dram-utilization: 0.80 | ||
| search-space: | ||
| # Pure TP at low concurrency. | ||
| - { tp: 8, kv-offloading: none, conc-list: [1, 2, 4, 6, 8] } | ||
| - { tp: 8, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, conc-list: [8, 12, 16] } | ||
| - { tp: 8, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.1" }, conc-list: [8, 12, 16] } | ||
| # DEP | ||
| - { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, conc-list: [8, 16, 24, 32, 40, 48, 56, 64, 68, 72, 80], router: { name: vllm-router, version: "0.1.14" } } | ||
| - { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [12, 20, 28, 36, 44, 52, 60, 68, 76], router: { name: vllm-router, version: "0.1.14" } } | ||
| - { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.1" }, conc-list: [12, 20, 28, 36, 44, 52, 60, 68, 76], router: { name: vllm-router, version: "0.1.14" } } | ||
|
|
||
| dsv4-fp4-b200-vllm-agentic-mtp: | ||
| image: vllm/vllm-openai:nightly-dev-x86_64-cu13.0.1-904e4ec | ||
| model: deepseek-ai/DeepSeek-V4-Pro |
There was a problem hiding this comment.
🟡 MODELS.md still says the DeepSeek-V4-Pro and GLM-5.2 agentic non-MTP retirement is 'not yet enacted' and that 'the non-MTP arm still runs' (lines 35, 142, 144), but this PR is exactly that enactment — it deletes the non-MTP scripts/configs. Please update the Deprecation Notice status and the two model-matrix rows to reflect that the retirement is now enacted, and consider archiving the removed scripts/configs under deprecated/ per the #2493/#2527 convention this doc credits, instead of deleting them outright.
Extended reasoning...
MODELS.md's Deprecation Notice explicitly tracks the DeepSeek-V4-Pro and GLM-5.2 agentic non-MTP retirements as a pending, not-yet-enacted item:
- Line 15: "The speculative-decoding A/B retirements in the second table are not yet enacted."
- Line 29/32: the A/B table lists
DeepSeek-V4-Pro ... Agentic coding, non-MTPandGLM-5.2 ... Agentic coding, non-MTPas the deprecated arms. - Line 35: "Status: not yet enacted. Every non-spec-decode agentic arm above still runs. ... would drop all AMD and all SGLang agentic coverage for DeepSeek-V4-Pro and Qwen3.5 ... This round runs once the replacement arms exist."
- Lines 142/144: the model support matrix rows for GLM-5.2 and DeepSeek-V4-Pro both say "Agentic coding (MTP-only pending — the non-MTP arm still runs; see the Deprecation Notice)".
This PR is precisely the enactment the doc describes as pending. It deletes dsv4_fp4_{b200,b300}_{sglang,vllm}.sh, dsv4_fp4_mi355x_{sglang,vllm}.sh, dsv4_fp4_mi355x_sglang-disagg.sh, and glm5.2_fp4_b300_sglang.sh, and removes the corresponding master-config keys from both configs/amd-master.yaml and configs/nvidia-master.yaml (dsv4-fp4-b200-vllm-agentic, dsv4-fp4-b300-vllm-agentic, dsv4-fp4-mi355x-vllm-agentic, dsv4-fp4-mi355x-sglang-agentic-hicache, dsv4-fp4-mi355x-sglang-disagg-agentic-hicache, dsv4-fp4-b200-sglang-agentic-hicache, dsv4-fp4-b300-sglang-agentic-hicache, glm5.2-fp4-b300-sglang-agentic). Yet MODELS.md is not in the PR's 18 changed files.
The code path that triggers the contradiction is simple: after this PR merges, a reader of MODELS.md sees "not yet enacted" / "the non-MTP arm still runs" for DeepSeek-V4-Pro and GLM-5.2, while the actual master configs and scripts confirm the non-MTP arm no longer exists at all. Nothing in the existing repo tooling (schema validation, pytest utils/matrix_logic/, etc.) cross-checks MODELS.md prose against the master YAMLs, so there is no mechanism that would have caught this before merge — it is a purely textual/documentation gap that only a human diff-read (or this review) surfaces.
Impact is limited to documentation accuracy: no benchmark, CI, or runtime behavior is affected, since MODELS.md is not consumed by any script in this repo (confirmed by its role as a maintainer-facing tracking doc, per the file's own description). But it is the canonical ledger the team uses to reason about what is and isn't safe to remove next, and the precedent PRs (#2493, #2527) both updated this exact section as part of their own enactment — so a maintainer reviewing this PR would very likely expect the same here, and future contributors reading MODELS.md would be misled into thinking the non-MTP arm still exists on AMD/SGLang for these two models.
Concrete proof, step by step:
- Before this PR:
configs/nvidia-master.yamlcontainsdsv4-fp4-b200-vllm-agentic(non-MTP) anddsv4-fp4-b200-vllm-agentic-mtp(MTP) side by side;MODELS.md:35says the non-MTP arm "still runs." - This PR's diff (
configs/nvidia-master.yamllines 945-950 in the pre-image) deletes thedsv4-fp4-b200-vllm-agenticblock entirely, leaving onlydsv4-fp4-b200-vllm-agentic-mtp. - The same pattern repeats for
dsv4-fp4-b300-vllm-agentic, the threedsv4-fp4-mi355x-*-agentic*AMD entries, the twodsv4-fp4-*-sglang-agentic-hicacheNVIDIA entries, andglm5.2-fp4-b300-sglang-agentic. - After merge, grepping the repo for
dsv4-fp4-b200-vllm-agentic:(non-MTP) returns nothing, whileMODELS.md:35and:144still assert that arm "still runs." - This is a direct, verifiable factual contradiction between the doc and the repo state it purports to describe.
Fix: update the Deprecation Notice's status line (currently "not yet enacted") to reflect that the DeepSeek-V4-Pro and GLM-5.2 rows of the A/B table are now enacted (Qwen3.5, MiniMax-M3, and Kimi-K3 rows presumably remain pending), and update lines 142/144 to drop the "non-MTP arm still runs" caveat for these two models. Following the archival convention from #2493/#2527 — moving the deleted scripts/configs into sibling deprecated/ directories and configs/deprecated/*.yaml instead of deleting them outright — would also preserve the audit trail those precedent PRs established, though that's a secondary nice-to-have relative to the doc-accuracy fix.
Summary
The GLM-5.2 MI325X and H200 Dynamo-SGLang e2e/exploration cases in app PR #694 have no corresponding live script or master-config entry on upstream
main, so no additional upstream files exist to remove for those attempts.Source-run mapping
28911223583/3,28955639528/3,29376853679/1,29413860950/3,29445892486/2,29486959583/2,29706766201/5, and29706772949/3mapped to the seven removed DSV4 scripts.29651235293/1and29682242847/1both mapped tobenchmarks/single_node/agentic/glm5.2_fp4_b300_sglang.sh.BENCH_SCRIPT,BENCHMARK_SCRIPT, or explicit benchmark command rather than inferred from its run title.Companion InferenceX-app purge PRs
29651235293/1.29682242847/1.Why / impact
Reason:
Non MTP which we aren't focusing on for AgentX as well as outdated AgentX harness.The companion app PRs purge the existing database attempts and prevent re-ingest. This upstream cleanup prevents future sweeps from regenerating the same obsolete source configurations.
Verification
python -m pytest utils/matrix_logic/ -q— 224 passed.bash -npassed for every modified surviving shell script.git diff --checkpassed.