Conversation
|
Thanks for the contribution!
中文感谢你的贡献!
|
|
Claude finished @cquil11's task in 3m 46s —— View job Review of PR #3268
LGTM - no blocking issues found. What I checked:
Non-blocking notes:
Not verified in this run (permissions): I could not execute |
There was a problem hiding this comment.
I reviewed this PR — it's a mechanical, YAML-only addition (one new srt-slurm recipe, a matching master-config entry, and an append-only changelog line), and I did not find a confirmed bug. That said, a human should look before this starter is used: the new recipe sets watchdog-timeout: 3600 on both the prefill and decode roles, while every other qwen3.5 srt-slurm recipe in this repo (all gb200/gb300, fp4/fp8, agg/disagg variants) uses watchdog-timeout: 1000000. This same recipe also sets SGLANG_DISAGGREGATION_BOOTSTRAP_TIMEOUT/WAITING_TIMEOUT to 100000s specifically to tolerate a slow disaggregated handshake — a watchdog nearly 28x shorter than that budget could kill a healthy worker mid-bootstrap. Worth confirming this is intentional (e.g. copied from the upstream SGLang cookbook default) rather than an unintentional deviation from the repo's established value.
Extended reasoning...
Overview
This PR adds a single new srt-slurm disaggregated benchmark recipe (benchmarks/multi_node/srt-slurm-recipes/qwen3.5/sglang/b300-fp8/agentx/disagg-1p1d-tp4-tp4-mtp.yaml) for Qwen3.5-397B-A17B FP8 on B300 GPUs (1 prefill + 1 decode worker, TP4/TP4, NEXTN MTP speculative decoding, mooncake KV transfer), a matching entry in configs/nvidia-master.yaml, and an append-only perf-changelog.yaml entry. No source code is touched — it's config/recipe data only.
Security risks
None. There is no code execution path introduced beyond existing srt-slurm/benchmark tooling; the changes are declarative YAML consumed by existing, unmodified infrastructure.
Level of scrutiny
Low-to-moderate is appropriate: this is a mechanical, additive config change following established repo conventions (recipe path/naming, additional-settings: CONFIG_FILE=..., image/container consistency between the recipe and the master config, append-only changelog). I independently verified via git diff and repo-wide grep that: (1) model.container in the recipe matches the image field in the new master-config entry; (2) the changelog entry is appended strictly at the tail with no prior bytes touched; (3) the additional-settings/CONFIG_FILE=recipes/... path convention matches dozens of existing entries (e.g. the dsr1/trtllm recipes), so the recipes/ prefix (vs. the actual benchmarks/multi_node/srt-slurm-recipes/ path) is an established alias, not a bug; (4) the master config's agentic-coding entries elsewhere in the file also omit trace-source despite CONFIGS.md documenting it as required — this is a pre-existing doc/implementation mismatch across the whole file, not something newly introduced by this PR, so I did not weigh it as a PR-specific issue.
Other factors
The one substantive anomaly I found on my own reading, beyond what was already flagged as candidates, is the watchdog-timeout: 3600 value used on both prefill and decode roles in the new recipe. I grepped every watchdog-timeout occurrence across all qwen3.5 srt-slurm recipes in the repo (dozens of GB200/GB300, fp4/fp8, aggregated and disaggregated variants) and this new file is the only one using 3600; every other file uses 1000000. The same new recipe separately sets SGLANG_DISAGGREGATION_BOOTSTRAP_TIMEOUT and SGLANG_DISAGGREGATION_WAITING_TIMEOUT to 100000 seconds, apparently to tolerate a slow disaggregated bootstrap/handshake — but a 3600s watchdog would fire well before that window elapses, plausibly killing a healthy-but-slow worker. This is exactly the kind of thing the bug-hunting run's two candidates pointed at; the run's own notes say those were dismissed as duplicates "not independently ruled out," and my own check corroborates the underlying concern rather than refuting it, so I'm surfacing it for a human to confirm intentionality (the PR description itself calls this recipe an unqualified "barebones draft," which raises rather than lowers the value of a second look here).
|
View unofficial run (performance): https://inferencex.semianalysis.com/inference?unofficialRun=35428532759 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=35428532759 |
|
/use 35428532759 |
Adds three B300 Qwen3.5-397B-A17B FP8 AgentX recipes at concurrency 16, 24 and 32. Each colocates one TP4 prefill worker and one TP4 decode worker on a single node, uses all eight allocated GPUs, and gives each prefill rank 72 GB of HiCache. Existing aggregate curves remain intact.
The complete official sweep passed all three throughput jobs, all three fresh accuracy jobs and the accuracy collector on source head
48a9c308d. Each profile used the canonical 3,600 seconds, ten warmup requests per lane, 393 traces and seed 42. Exact CI artifacts were reconciled with native request, power and evaluation data.All rates use eight active/eight allocated GPUs. Logical total throughput includes cached input tokens; interactivity is the reciprocal of p90 full-response ITL. All three points are mutually nondominated and nondominated against the current published B300 FP8 frontier, refreshed read-only at 14:05 UTC on September 19. C16 strictly improves the published aggregate TP4 C8 point by 17.67% throughput and 2.92% interactivity. C32 improves the aggregate TP4 C12 point by 13.19% throughput and 3.42% interactivity. C24 adds an intermediate measured tradeoff. Seven aggregate controls on the same nightly also did not dominate these points; those short controls are screening evidence, not matched full-hour speedup estimates.
C16/C24/C32 completed 3,251/4,616/5,477 requests with zero request errors. C24 had two drain-deadline cancellations (0.043%); the others had none. Submission validity and duration coverage passed. Required native telemetry and both actual CI and independent strict power validation passed across all eight GPUs. All six Slurm allocations completed with exit code 0:0. Fresh GSM8K used real verification, all 1,319 test examples and five-shot prompting, with no sample limit; every score passed the repository's 94% threshold. The final collector contains exactly the three expected evaluation rows.
The recipes pin SGLang
nightly-dev-cu13-20260918-20518d85, Dynamo805a77f053d807b0d8def5d27f674a6df0ed839e, and the FP8 checkpoint revision. They use NEXTN, repository golden acceptance for throughput, prefill-only HiCache, supported Mamba cache flags and Mooncake intra-node NVLink. The launcher validates terminal Slurm accounting and required power after preserving diagnostics; eval-only jobs skip throughput power validation. Node-local model verification replaces login-host preflight.The selected scope excludes 2P1D and higher-concurrency experiments: their short-run gains did not establish canonical qualification. C48/C64 full-length measurements lost to the frontier; C64 additionally failed the strict power-gap gate. C96 exhausted its original allocation after long canonical warmup, so it was not admitted. No validation threshold was relaxed. Four-hour limits now cover startup, canonical warmup, the unchanged one-hour profile and artifact processing. gpu11 is temporarily excluded after three pre-measurement loading failures; its cause remains unproven, and a later eval on that node succeeded. No other users' jobs, shared files or node settings were changed. Server counter-decrease warnings are retained separately and are not interpreted as proven worker restarts.
Final CI passed lint and 1,800 tests (one skipped) on synchronized head
48902958a. The reuse/changelog sweep passed and correctly skipped new GPU work using authorization pinned to the complete source sweep. Native renderings and the exact three-throughput/three-eval matrix passed, with exactly one PR changelog entry and byte-preserved main history. Main synchronization preserves every recipe/serving/client setting; its shared power-window contract resolver produces identical windows for these three explicit caller contracts, verified by focused behavior tests. PR is conflict-free and ready for review; it has not been merged.AI model disclosure: OpenAI Codex; the runtime identifies the GPT-6 family but does not expose a verifiable exact model/version. Delegated agents handled bounded testing and source, placement, cache, telemetry and completion audits.