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 11m 49s —— View job PR Review — 1 blocking, 1 warning
Summary: 🔴 The launcher's vLLM checkpoint override ( Two notes for the author: (1) merging intentionally replaces the existing dsv4 B200 vLLM AgentX production curve (TP8 conc 1–14 + DEP8 conc 32–196) with three validation points on a personal fork image — flagging so maintainers confirm that's the intent; (2) the Simple NVMe config passes the same |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34394041977 |
Reuse the DeepSeek-V4 TP8 AgentX serving recipe, add explicit NVMe and tiered modes, and scope node-local cache cleanup to the allocation.
956b289 to
fd2239f
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34394405958 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if [[ "$FRAMEWORK" == "vllm" ]]; then | ||
| export MODEL_PATH="/scratch/models/DeepSeek-V4-Pro" | ||
| fi |
There was a problem hiding this comment.
🔴 BLOCKING: This checkpoint swap is not scoped to the agentic validation recipe — it changes the weights for every dsv4-fp4 vLLM job on this launcher, including the existing production dsv4-fp4-b200-vllm fixed-seq-len config (configs/nvidia-master.yaml:947), which also has framework: vllm, runner: cluster:b200-nscale, and routes through this compat launcher for all single-node jobs (launch_b200-nscale-slurm.sh:39-41).
Why it matters: benchmarks/single_node/fixed_seq_len/dsv4_fp4_b200_vllm.sh:43-45 explicitly assumes the pinned checkpoint is the ModelOpt NVFP4 conversion ("The pinned checkpoint uses ModelOpt NVFP4 expert weights ... use FlashInfer CuTeDSL instead"). Silently serving /scratch/models/DeepSeek-V4-Pro to that recipe will break or change its existing benchmark curve, and there is no perf-changelog.yaml entry for dsv4-fp4-b200-vllm — this violates the repo invariant that every benchmark-affecting change be covered by a changelog entry for the affected config keys.
Fix: Scope the override to the agentic recipe. SCENARIO_TYPE (and IS_AGENTIC) are exported by benchmark-tmpl.yml and empty for fixed-seq-len jobs:
| if [[ "$FRAMEWORK" == "vllm" ]]; then | |
| export MODEL_PATH="/scratch/models/DeepSeek-V4-Pro" | |
| fi | |
| if [[ "$FRAMEWORK" == "vllm" && "${SCENARIO_TYPE:-}" == "agentic-coding" ]]; then | |
| export MODEL_PATH="/scratch/models/DeepSeek-V4-Pro" | |
| fi |
(Alternatively, keep the unscoped change and add a perf-changelog.yaml entry for dsv4-fp4-b200-vllm plus fix its MoE-backend assumption — but that widens this validation PR considerably.)
| local rc=$? | ||
| if [[ -n "$NVME_HOST_DIR" ]]; then | ||
| timeout --kill-after=10s 60s srun --jobid="$JOB_ID" \ | ||
| rm -rf -- "$NVME_HOST_DIR" || { | ||
| echo "NVMe cleanup failed: $NVME_HOST_DIR on job $JOB_ID" >&2 | ||
| rc=1 | ||
| } | ||
| fi | ||
| scancel "$JOB_ID" || true | ||
| exit "$rc" |
There was a problem hiding this comment.
🟡 WARNING: Cleanup depends on the allocation still being alive. If the job dies before the EXIT trap's srun --jobid runs — SALLOC_TIME_LIMIT expiry (a hung benchmark at 480 min), preemption, or a node failure — srun fails against the dead job ID and /scratch/inferencex-kv-$JOB_ID is stranded permanently on node-local NVMe. Nothing ever reclaims it: each retry uses a new job ID and this PR deliberately deletes only the current job's directory, so repeated timeouts can accumulate up to ~1 TB of cache files per stranded run on a compute node the login node can't reach.
Fix: Keep the per-job delete, but before creating the new directory, sweep stale siblings whose owning job no longer exists (preserves the "never touch a live job's files" property):
if [[ "${KV_OFFLOADING:-none}" == *nvme* ]]; then
NVME_HOST_DIR="/scratch/inferencex-kv-$JOB_ID"
srun --jobid="$JOB_ID" bash -c '
for d in /scratch/inferencex-kv-*; do
[[ -d "$d" ]] || continue
jid="${d##*-}"
squeue -h -j "$jid" > /dev/null 2>&1 || rm -rf -- "$d"
done
' || true
srun --jobid="$JOB_ID" mkdir -m 700 "$NVME_HOST_DIR" || exit 1
| """ | ||
| kv_offloading = benchmark.get(Fields.KV_OFFLOADING.value, "none") | ||
| if kv_offloading != "dram": | ||
| if kv_offloading not in ("dram", ["dram", "nvme"]): |
There was a problem hiding this comment.
📊 Line Count Report
- Total Lines: 1573
- Base Lines: 1570
- Change: 📈 +3 lines
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34394952535 |
| cleanup_offload_cache() { | ||
| local rc=$? | ||
| if [[ -n "$NVME_HOST_DIR" ]]; then | ||
| timeout --kill-after=10s 60s srun --jobid="$JOB_ID" \ | ||
| rm -rf -- "$NVME_HOST_DIR" || { | ||
| echo "NVMe cleanup failed: $NVME_HOST_DIR on job $JOB_ID" >&2 | ||
| rc=1 | ||
| } | ||
| fi | ||
| scancel "$JOB_ID" || true | ||
| exit "$rc" | ||
| } |
There was a problem hiding this comment.
🟡 (optional) The new EXIT-trap cleanup_offload_cache() overwrites rc=1 whenever the bounded NVMe rm -rf fails/times out, so a benchmark that already succeeded (rc=0) gets reported as a failed CI step just because scratch cleanup hit a transient Slurm/filesystem hiccup. Fix: isolate cleanup failures from the run's own exit status — log the cleanup failure but exit with the original $? captured before cleanup, only setting a nonzero code if the run itself failed.
Extended reasoning...
Main bench srun (line 585-591) finishes with rc=0 -> script falls off the end -> EXIT trap fires, local rc=$? captures 0 first. If the subsequent timeout --kill-after=10s 60s srun --jobid="$JOB_ID" rm -rf -- "$NVME_HOST_DIR" (545-549) fails for any reason (Slurm control-plane hiccup, node already draining, srun step rejected) it unconditionally sets rc=1, and exit "$rc" at line 552 then reports failure to the CI workflow even though the benchmark itself succeeded and produced valid results. This path is entirely new (no cleanup/trap existed on this branch before the diff), so it is not pre-existing; before the diff nothing could turn a successful run into a failed step this way.
Verification: normal. In the new EXIT trap at runners/launch_b200-nscale-compat.sh (added this diff), cleanup_offload_cache() does local rc=$? (capturing the main bench srun's exit code), then when NVME_HOST_DIR is set runs timeout --kill-after=10s 60s srun --jobid="$JOB_ID" rm -rf -- "$NVME_HOST_DIR" and on any nonzero result unconditionally sets rc=1, followed by exit "$rc". So a benchmark…
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34395212219 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34400931163 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34404334784 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34412940233 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34420875287 |
1 participant
Summary
Validate the sliding-window/hybrid attribution update in cquil11/vllm#2, using the existing DeepSeek-V4-Pro B200 TP8/MTP AgentX recipe.
GPU memory utilization is 0.85. This image loads ~105 GiB of weights per GPU, so 0.30–0.50 cannot fit this recipe. Canonical AgentX warmup/duration remain unchanged. B300 is excluded.
Uses
full-sweep-enabledwithno-evals: trueon the new changelog entries,supported by merged #2935.
Only the three throughput points run; this does not provide model-quality eval
evidence. CPU threading remains at the runtime default.
Frontend exception to the historical recipe: use the Python frontend
(
VLLM_USE_RUST_FRONTEND=0) where the fork implements Prometheus source metrics.The Rust frontend has a separate metrics implementation and currently ignores
the added attribution field. This validates the same engine/SWA/offloading path,
but is not a direct frontend-performance comparison with the historical run.
Implementation
kv-offloading: nvmeand[dram, nvme], with explicit runtime backend validation. Other recipes continue accepting only their supported modes.sha256:3e5daa6e289046ca99e690a8a91a46a9ffb5a3c213dac6deb9afb06a6ef22188. Exact upstream wheele41011129bd54c9e8e12b645ea9b964027d2363fplus fork PR headb0dc209c14451e9a4e63f1db148ed492d881b765, merged upstream #55712 SWA coverage fix, and open upstream #50014 unread-promotion protection. Exact validation source is separate from the unchanged metrics PR. No CUDA rebuild. FlashInfer Python, cubin, and CUDA 13 JIT-cache packages all use 0.6.18.Validation
git diff --check: passed.0.28.1rc1.dev369+ge41011129, torch2.13.0+cu130, FlashInfer0.6.18.external(105,020 tokens in those buckets). Whole-export totals include a warmup boundary bucket.6a822f912extends the bounded cleanup deadline to five minutes. Shell syntax and 116 focused changelog tests passed. Replacement official run 34420875287 is all green, retaining the image, three points, and canonical duration. DRAM: 1,117 valid requests; Simple NVMe: 2,075; tiered: 2,067. Zero request errors; one drain-deadline cancellation each for NVMe/tiered. TTFT/ITL coverage gates passed. All three AIPerf exports have exactly five sources, conserved legacy/source totals, and agreement across all nine JSON/CSV counter series. Complete profiling buckets show 190,624 host tokens (DRAM), 15,882,752 disk tokens (Simple), and 2,213,744 host plus 14,092,624 disk tokens (tiered). Raw artifacts: DRAM, NVMe, tiered. Slow cold checkpoint loading was accelerated with read-only file prefetch before profiling; server commands and thread settings were unchanged. P2P validation remains tracked separately in Validate current vLLM P2P cache sources on GB300 1P/1D #2938.Prepared with AI assistance; this is a validation PR, not a claim of completed GPU verification.
Note
Medium Risk
Touches Slurm launcher teardown, new matrix offload modes, and benchmark recipe/runtime flags; scope is mostly B200 validation but shared validation helpers affect other agentic sweeps.
Overview
Adds NVMe and tiered DRAM+NVMe agentic KV offload modes end-to-end: matrix validation/generation accepts
nvmeand['dram','nvme'], maps tier lists to runtime values likedram+nvme, and keeps DRAM budgeting for DRAM/tiered (not pure NVMe). Sharedbenchmark_lib.shchecks offload mode against per-recipe allowed backends.The DeepSeek-V4 B200 vLLM MTP recipe is retargeted to a cache-source validation sweep: pinned custom image, Python frontend (
VLLM_USE_RUST_FRONTEND=0), lower GPU memory utilization, and three points—native DRAM, Simple disk NVMe, and native tiered DRAM+NVMe—viaOffloadingConnector/SimpleCPUOffloadConnectorconfigs. Nscale B200 launcher mounts a job-scoped NVMe path, extends cleanup to 5 minutes before releasing Slurm, fixes vLLM DSV4 checkpoint selection, and hardens allocation/import failures.Docs note NVMe cleanup timeouts;
perf-changelog.yamlrecords the validation iterations (image, host cache size, upstream backports).Reviewed by Cursor Bugbot for commit 6a822f9. Bugbot is set up for automated code reviews on this repo. Configure here.