perf(agentx): Refresh Qwen3.5 GB300 Pareto recipes - #2477
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 关于重新运行失败任务的文档 |
4360f90 to
f211f95
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30838893965 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| - config-keys: | ||
| - qwen3.5-fp4-gb300-dynamo-sglang-agentic-agg |
There was a problem hiding this comment.
🔴 Restoring the trailing two spaces on the blank separator line (perf-changelog.yaml:5376) that separates the prior pull/2420 entry from this PR's new entry — the diff replaces (two trailing spaces) with a plain empty line, stripping pre-existing whitespace.
Extended reasoning...
perf-changelog.yaml is treated as an append-only, byte-exact history file, and utils/validate_perf_changelog.py enforces that at CI time. The blank separator line between the previous entry (pr-link: .../pull/2420) and the new entry this PR appends previously contained two trailing spaces before the newline. This diff's hunk shows that exact line removed (- , i.e. two spaces) and replaced with a plain empty line (+), which strips the trailing whitespace from pre-existing, historical content rather than leaving it untouched.
I verified this at the byte level directly against the repository history:
git show e275f56:perf-changelog.yaml | sed -n "5376p" | cat -Arenders as$— two literal space characters followed by end-of-line — confirming the base file's separator line has two trailing spaces.git show 4360f90:perf-changelog.yaml | sed -n "5376p" | cat -A(this PR's HEAD) renders as$with no trailing spaces — confirming the two spaces were stripped by this PR.
This matters because validate_raw_change() in utils/validate_perf_changelog.py (lines 211-223) enforces byte-for-byte preservation of history when new entries are appended:
if additions:
if not head_raw.startswith(base_raw):
raise ChangelogValidationError(
"appended entries changed historical perf-changelog.yaml bytes; "
"restore the base file byte-for-byte and append at the end"
)base_raw (the file at the merge-base/previous state) ends in ...pull/2420\n \n — i.e., the byte immediately after the trailing \n following pull/2420 is a space character (the start of the two-space separator line). head_raw (this PR's file) instead has ...pull/2420\n\n- config-keys:... — a newline in that same position instead of a space. Because head_raw does not start with the exact byte sequence of base_raw, head_raw.startswith(base_raw) evaluates to False, and the validator raises "appended entries changed historical perf-changelog.yaml bytes; restore the base file byte-for-byte and append at the end".
This is exactly the failure mode AGENTS.md explicitly warns about: "Never delete or modify whitespace in perf-changelog.yaml - CI depends on exact whitespace (including trailing spaces on blank separator lines). Altering it breaks CI." Nothing else in the existing code prevents this — a plain-text/YAML edit that "looks" like a no-op whitespace cleanup silently trips a strict byte-comparison check that isn't visible from casually reading the diff.
The fix is mechanical: restore the two trailing spaces on perf-changelog.yaml line 5376 (the blank line right after the pull/2420 entry) so the file is byte-identical to history up to that point, with the new entry still appended after it.
| agentic-coding: | ||
| - dram-utilization: 0.80 | ||
| search-space: | ||
| - spec-decoding: "mtp" | ||
| - search-space: | ||
| - spec-decoding: mtp | ||
| conc-list: [1] | ||
| kv-offloading: dram | ||
| kv-offload-backend: { name: hicache } | ||
| prefill: | ||
| num-worker: 1 | ||
| tp: 4 | ||
| tp: 2 | ||
| ep: 1 | ||
| dp-attn: false | ||
| additional-settings: | ||
| - "CONFIG_FILE=recipes/sglang/qwen3.5/gb300-fp4/agentic/agg-gb300-tp4-c1-mtp-hicache-jid2191933.yaml" | ||
| - "CONFIG_FILE=recipes/sglang/qwen3.5/gb300-fp4/agentic/agg-gb300-tp2-c1-mtp-hicache-jid2530006.yaml" | ||
| decode: | ||
| num-worker: 0 | ||
| tp: 4 | ||
| num-worker: 1 | ||
| tp: 2 | ||
| ep: 1 | ||
| dp-attn: false | ||
| - spec-decoding: "mtp" | ||
| conc-list: [96] | ||
| kv-offloading: dram | ||
| kv-offload-backend: { name: hicache } | ||
| - spec-decoding: mtp | ||
| conc-list: [24] | ||
| prefill: | ||
| num-worker: 1 | ||
| tp: 4 | ||
| tp: 2 | ||
| ep: 1 | ||
| dp-attn: false | ||
| additional-settings: |
There was a problem hiding this comment.
🔴 All 7 refreshed entries in qwen3.5-fp4-gb300-dynamo-sglang-agentic-agg set decode.num-worker: 1 / decode.tp: 2 alongside prefill.num-worker: 1 / prefill.tp: 2, even though these are single-worker aggregated (disagg: false) recipes where one 2-GPU worker serves both prefill and decode. Because process_agentic_result.py only zeroes out the decode side when num_decode_gpu <= 0, this makes it compute num_gpus=4 and tp=4 for what is actually a 2-GPU TP2 deployment, halving the reported per-GPU throughput and mislabeling TP for all 7 new aggregate curve points (conc 1/24/32/40/48/52/64). Please set decode.num-worker back to 0 on all 7 entries, matching the prior version of this config and the sibling aggregated configs.
Extended reasoning...
The bug: In configs/nvidia-master.yaml's qwen3.5-fp4-gb300-dynamo-sglang-agentic-agg block (lines ~7107-7130), all 7 newly refreshed search-space entries set both prefill.num-worker: 1/tp: 2 and decode.num-worker: 1/tp: 2. This block is aggregated (disagg: false), and each entry's recipe yaml (e.g. agg-gb300-tp2-c1-mtp-hicache-jid2530006.yaml) confirms agg_workers: 1, gpus_per_agg: 2 — a single 2-GPU worker that performs both prefill and decode. There is no separate decode worker to count.\n\nWhy it matters / the code path: utils/agentic/aggregation/process_agentic_result.py computes GPU/TP accounting as:\n\nThe zeroing guard only fires when num_decode_gpu <= 0. With decode.num-worker: 1, num_decode_gpu = 1*2 = 2 > 0, so the guard is skipped: num_gpus becomes 2 + 2 = 4 instead of the real 2, and tp becomes 2 + 2 = 4 instead of the real 2. num_gpus is then the divisor used to compute per-GPU output/input/total throughput, so every one of these 7 points reports half its true per-GPU throughput, and is mislabeled as TP4 instead of TP2.\n\nWhy existing code doesn't prevent it: the guard only protects against decode workers being present when they shouldn't be if the config author remembers to zero num-worker; it can't detect "this num-worker field is wrong for an aggregated deployment" — that invariant has to be maintained in the YAML by the PR author, and this PR broke it for all 7 entries in this block.\n\nStep-by-step proof (conc=1 entry, jid2530006):\n1. Recipe yaml: resources.agg_workers: 1, resources.gpus_per_agg: 2 → real deployment is 1 worker, 2 GPUs total, TP2.\n2. nvidia-master.yaml sets prefill.num-worker: 1, prefill.tp: 2, decode.num-worker: 1, decode.tp: 2.\n3. These flow via PREFILL_NUM_WORKERS/DECODE_NUM_WORKERS env vars into process_agentic_result.py.\n4. num_prefill_gpu = 1*2 = 2; num_decode_gpu = 1*2 = 2. Since num_decode_gpu > 0, the zeroing branch does not run.\n5. num_gpus = 2 + 2 = 4 (should be 2); tp = 2 + 2 = 4 (should be 2).\n6. Per-GPU throughput = total_throughput / num_gpus = total_throughput / 4, when it should be total_throughput / 2 — exactly half the correct value. TP is also reported as 4 instead of 2.\n\nCorroboration: the prior version of this exact config used decode.num-worker: 0 (visible in the diff removing the old TP4 entries); sibling aggregated configs dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg and dsv4-fp4-gb300-dynamo-sglang-agentic-agg explicitly keep decode.num-worker: 0 with a comment explaining exactly this invariant. The disagg block in this same PR correctly uses decode.num-worker: 1 because there decode really is a separate worker — reinforcing that the agg block's use of 1 is simply wrong.\n\nFix: set decode.num-worker: 0 on all 7 aggregate entries (conc-list [1], [24], [32], [40], [48], [52], [64]) in qwen3.5-fp4-gb300-dynamo-sglang-agentic-agg, matching the pattern used everywhere else in this file for aggregated deployments.\n\nImpact if merged as-is: since this PR's entire purpose is to publish a refreshed, accurate 14-point Pareto frontier, merging with this bug means all 7 aggregate points on that frontier report exactly half their true per-GPU throughput and the wrong TP — corrupting the primary metric the PR exists to produce.
| - qwen3.5-fp4-gb300-dynamo-sglang-agentic-disagg | ||
| description: | ||
| - "Refresh the Qwen3.5 GB300 AgentX frontier after aiperf 381758a restored real conversation pacing; replace the stale nine-point sweep with a complete fourteen-point locally validated curve: disaggregate TP4/TP4 c8/c16/c32/c64/c96/c128, disaggregate TP2/TP2 c72, and aggregate TP2 c1/c24/c32/c40/c48/c52/c64." | ||
| - "The 30-minute local re-sweep covered low, mid, and high concurrency across aggregate TP2, disaggregate TP2/TP2, and disaggregate TP4/TP4. All 33 new-client steady-state samples completed with zero request errors and zero retract events; using the same historical output-token-throughput-per-user p50 metric as the old curve, the fourteen submitted points are the strict non-dominated set. The InferenceMAX sweep keeps the canonical 60-minute AgentX benchmark duration." | ||
| - "Align both configurations on the 2026-07-24 SGLang container, current aiperf be758d6, the merged Dynamo #12081 commit 5a638087, X-Dynamo-Session-ID affinity with the removed legacy conv-aware CLI path explicitly disabled, and NVIDIA/srt-slurm v1.0.36 while keeping the recipe and nvidia-master entries synchronized." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX |
There was a problem hiding this comment.
🔴 The new perf-changelog.yaml entry's pr-link is left as https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX (four X's) instead of the real PR number, 2477. This isn't just a cosmetic placeholder — utils/validate_perf_changelog.py's validate_added_pr_link only accepts the exact three-X placeholder (XXX / .../pull/XXX) or the real .../pull/{pr_number} link on a PR run, so pull/XXXX matches neither and the changelog-validation CI check will fail. Fix by replacing XXXX with 2477.
Extended reasoning...
What the bug is: The changelog entry appended at perf-changelog.yaml:5384 ends with:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXXThat is four literal X characters, not the real PR number (this PR is #2477). Every other entry in the file — including the three immediately above it (pull/2360, pull/2213, pull/2421, pull/2420) — carries a real, resolved PR number, confirming XXXX is an unfilled template placeholder left over from drafting rather than an intentional value.
The exact code path that fails: utils/validate_perf_changelog.py defines:
PR_LINK_PLACEHOLDERS = {
"XXX",
"https://github.com/SemiAnalysisAI/InferenceX/pull/XXX",
}and validate_added_pr_link(link, pr_number), called on a PR run with pr_number=2477, does:
expected = f"https://github.com/SemiAnalysisAI/InferenceX/pull/{pr_number}"
if link not in PR_LINK_PLACEHOLDERS and link != expected:
raise ChangelogValidationError(...)Note the placeholder set uses exactly three Xs (XXX), matching the documented template in AGENTS.md. The submitted value https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX has four Xs, so it is neither in PR_LINK_PLACEHOLDERS nor equal to expected (.../pull/2477). The function raises ChangelogValidationError, which fails the changelog-validation check.
Why existing tooling doesn't catch/prevent it upstream: The validator only checks the format of the link (exact placeholder string, or the exact expected PR URL) — it has no way to auto-correct a near-miss like an extra character, so a typo'd placeholder is treated as an arbitrary invalid string rather than "close enough" to the accepted placeholder.
Step-by-step proof:
- PR perf(agentx): Refresh Qwen3.5 GB300 Pareto recipes #2477 runs the changelog validator with
pr_number=2477. - The validator reads the new entry's
pr-link:"https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX". expected = "https://github.com/SemiAnalysisAI/InferenceX/pull/2477".- Check 1: is
linkinPR_LINK_PLACEHOLDERS({"XXX", ".../pull/XXX"})? No —XXXX(four X's) !=XXX(three X's) as a substring match on the full string. - Check 2: is
link == expected? No —XXXX!=2477. - Both checks fail →
ChangelogValidationErroris raised → the changelog-validation CI job fails.
Impact: This blocks the changelog-validation CI gate on this PR (not just a traceability nit) until fixed, though the fix is a one-character-format substitution.
Fix: Replace the placeholder value with the real PR number:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2477
Refresh the Qwen3.5-397B-A17B NVFP4 GB300 AgentX frontier with the fourteen-point locally-validated curve: aggregate TP2 (c1/c24/c32/c40/ c48/c52/c64), disaggregate TP2/TP2 (c72), and disaggregate TP4/TP4 (c8/c16/c32/c64/c96/c128), all MTP/NEXTN + hicache with X-Dynamo-Session-ID affinity.
f211f95 to
a020236
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30839233697 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30842453053 |
The 7 aggregated (disagg: false) agg entries run a single 2-GPU worker serving both prefill and decode. With decode.num-worker: 1, process_agentic_result.py counts num_gpus=4/tp=4 instead of 2/2 — halving reported per-GPU throughput and mislabeling TP4. Set decode num-worker back to 0 to match the sibling aggregated configs.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30858790538 |
…0-pareto-recipes # Conflicts: # perf-changelog.yaml
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30878899537 |
1 participant
Refresh the Qwen3.5-397B-A17B NVFP4 GB300 AgentX frontier with the fourteen-point locally-validated curve: aggregate TP2 (c1/c24/c32/c40/ c48/c52/c64), disaggregate TP2/TP2 (c72), and disaggregate TP4/TP4 (c8/c16/c32/c64/c96/c128), all MTP/NEXTN + hicache with X-Dynamo-Session-ID affinity.