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=35405462530 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=35405462530 |
Adds colocated 1P1D Qwen3.5-397B-A17B FP8 AgentX recipes for B300 at concurrency 16, 24 and 32. Each uses one TP4 prefill worker and one TP4 decode worker, all eight allocated GPUs, and 72 GB HiCache per prefill rank. Existing aggregate curves remain intact.
The recipes pin SGLang
nightly-dev-cu13-20260918-20518d85and Dynamo805a77f053d807b0d8def5d27f674a6df0ed839e, with NEXTN and colocated Mooncake intra-node NVLink.apply_srt_recipesupplies repository golden acceptance for throughput; accuracy uses real verification. Native DCGM telemetry covers all eight GPUs, with explicit formal measurement-window fields and strict producer, coverage and final power validation. Slurm failures and missing power fail the launcher after diagnostic artifacts are collected.The initial full sweep qualified C32 at 20,629.16 total tok/s/GPU and 200.581 p90 interactivity, versus the nearby published aggregate point at 18,270.73 and 199.203: 12.91% more throughput with slightly better interactivity. Its full-hour profile completed 5,464 requests with zero errors/cancellations; submission metadata, duration coverage, native power, actual CI power validation and an independent strict power check passed. Output throughput was 184.47 tok/s/GPU and TTFT p90 was 1.171 seconds. All six exploratory full GSM8K evaluations passed on 1,319 examples each; retained C16/C24/C32 strict scores were 97.95%/98.03%/97.57%.
C16/C24 retain promising short-run measurements but their initial full throughput jobs failed before measurement during intermittent prefill-rank-3 loading on gpu11. The recipes temporarily exclude that node and allow four hours for canonical warmup, the unchanged one-hour profile and artifact processing. A later eval on gpu11 succeeded; no hardware defect is claimed. No shared files, node settings or other users' jobs were changed.
The final scope excludes 2P1D variants and the higher-concurrency candidates: their short-run gains did not establish canonical qualification. C48's complete full-length raw profile was dominated and finalization timed out; C96 exceeded the allocation during profiling after 77.7 minutes of warmup. C64's full-length raw profile also lost substantially (5,101.05 tok/s/GPU at 18.851 p90 interactivity, with 79/1,737 drain cancellations). Native power publication also failed because its maximum sample gap was 3.824 seconds; no validation threshold was relaxed. The job correctly failed its required power gate. Exact serving settings matched their earlier short runs, so no accidental recipe change was found. These negative results are retained as diagnostics, not admitted to the curve.
Final validation is running in the fresh complete three-point official sweep on
48a9c308d, withfull-sweep-enabledandall-evals, using canonical 3,600-second profiles and ten warmup requests per lane. The initial incomplete sweep is not reused. Local checks passed: three native recipe renderings, exact three throughput/three eval generation, one consolidated changelog entry with byte-identical main history, 46 focused launcher/power tests, and 1,732 tests across the full local suite plus the pinned SRT connector suite (one skipped).CI passed lint and 1,798 tests (one skipped) on the final head. The six GPU jobs are reserved in priority positions 1–6; full performance/power/eval qualification remains pending.
AI model disclosure: OpenAI Codex; the runtime identifies the GPT-6 family but does not expose a verifiable exact model/version. Delegated agents handled B200/B300 testing and bounded source, placement, cache and telemetry audits.