Refresh DeepSeek-V4 B200 SGLang AgentX MTP - #2578
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 关于重新运行失败任务的文档 |
7c7a5b4 to
721ec8d
Compare
cquil11
changed the title
|
Claude finished @cquil11's task in 8m 25s —— View job PR Review
SummaryOne 🔴 BLOCKING issue (inline comment posted): the new Everything else checks out:
|
721ec8d to
8b73f94
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31621468427 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - config-keys: | ||
| - dsv4-fp4-b200-sglang-agentic-hicache-mtp | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "Refresh the purged B200 DeepSeek-V4-Pro SGLang AgentX resident, DP-attention, and HiCache grid on SGLang v0.5.17." | ||
| - "Use native EAGLE MTP (3 steps, top-k 1, 4 draft tokens) and golden synthetic acceptance length 2.49 for throughput; eval retains real verification." | ||
| - "Follow the official SGLang DeepSeek-V4 Blackwell recipe and require nonempty SGLang server metrics." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2578 |
There was a problem hiding this comment.
🔴 BLOCKING: New perf-changelog.yaml entry is inserted mid-file instead of appended to the end.
Why it matters: The file is read chronologically (oldest at top, newest at bottom — it starts at PR #95 and ends at PR #2562's entry on line 5906). This entry was inserted at line 5441, right after the PR #2508 entry, with ~450 lines of existing history below it. Beyond breaking the ordering convention, utils/validate_perf_changelog.py enforces append-only byte-for-byte history (head_raw.startswith(base_raw) in validate_raw_change), so CI changelog validation will fail with "appended entries changed historical perf-changelog.yaml bytes; restore the base file byte-for-byte and append at the end".
Fix: Delete this entry from here and append it verbatim after the last entry in the file (currently the qwen3.5-fp4-mi355x-sglang-agentic-mtp entry for PR #2562, ending at line 5906), separated by exactly one empty line.
| if [ "${#METRICS_ARGS[@]}" -gt 0 ]; then | ||
| capture_cache_metrics | ||
| trap capture_cache_metrics EXIT | ||
| fi |
There was a problem hiding this comment.
🟡 The EXIT trap guard if [ "${#METRICS_ARGS[@]}" -gt 0 ] at line 259 is always true, since METRICS_ARGS is set unconditionally at line 103 to a fixed 2-element array and is never emptied or reassigned on any path. It reads as though metrics capture is optional (mirroring the genuinely-conditional CACHE_ARGS pattern above it), but it isn't — either drop the guard or make METRICS_ARGS actually conditional.
Extended reasoning...
The bug: METRICS_ARGS is declared once, unconditionally, at line 103:
METRICS_ARGS=(--enable-metrics --enable-cache-report)It is never reassigned, appended to, or emptied anywhere else in the script — a grep for METRICS_ARGS across the file turns up exactly three hits: the assignment at line 103, its expansion into SGLANG_CMD at line ~204, and the guard at line 259:
if [ "${#METRICS_ARGS[@]}" -gt 0 ]; then
capture_cache_metrics
trap capture_cache_metrics EXIT
fiSince the array always has exactly 2 elements, ${#METRICS_ARGS[@]} is always 2, and the condition is always true. The if block is dead weight — capture_cache_metrics and the EXIT trap install unconditionally regardless of what this guard says.
Why it's misleading rather than merely redundant: the script has one real precedent for this shape a few lines earlier — CACHE_ARGS. That array starts empty (CACHE_ARGS=()) and is populated only inside the require_agentic_kv_offload_backend hicache branch, so a length check on it is a legitimate runtime conditional. METRICS_ARGS copies that visual pattern (an array-length guard right before use) without the underlying conditionality that makes the pattern meaningful. A reader skimming the script would reasonably assume metrics/cache-report capture is optional in some configuration, when in fact it is always on.
Step-by-step proof:
- Line 103 executes unconditionally on every invocation of the script:
METRICS_ARGS=(--enable-metrics --enable-cache-report). - No branch (HiCache on/off, DP-attention on/off, eval-only or not) touches
METRICS_ARGSagain before line 259. - At line 259,
${#METRICS_ARGS[@]}evaluates to2in every possible run. [ 2 -gt 0 ]is always true, socapture_cache_metricsis always invoked immediately and thetrap capture_cache_metrics EXITis always installed.- Therefore no execution path skips this block — the guard has no observable effect on behavior.
Impact: none functionally — this doesn't change program behavior since the branch is always taken anyway. It's purely a readability/maintainability nit: a future editor could plausibly try to make metrics capture conditional by clearing METRICS_ARGS somewhere, not realizing the guard already silently assumed that possibility without it ever occurring.
Fix: either (a) drop the if and call capture_cache_metrics/install the trap unconditionally, since metrics are always enabled, or (b) if optional metrics capture was actually intended, make METRICS_ARGS conditionally empty (e.g., only set it under a flag) so the guard reflects real behavior.
| # processing use the isolated environment when InferenceX is mounted at /ix. | ||
| SGLANG_PYTHON="$(command -v python3)" | ||
| if [[ "$INFMAX_CONTAINER_WORKSPACE" != /workspace ]]; then | ||
| AGENTIC_VENV="${AGENTIC_VENV:-/tmp/inferencex-agentic-venv}" | ||
| "$SGLANG_PYTHON" -m venv "$AGENTIC_VENV" | ||
| export PATH="$AGENTIC_VENV/bin:$PATH" | ||
| fi | ||
| install_agentic_deps |
There was a problem hiding this comment.
🟡 In eval-only mode, the empty AGENTIC_VENV created at dsv4_fp4_b200_sglang_mtp.sh:52-56 shadows python3 before transformers is installed into it, so get_native_max_context_length's bare-python3 probe (benchmark_lib.sh:908) fails silently and falls back to a hardcoded 16384 instead of DeepSeek-V4-Pro's real native context. For this specific recipe the fallback is harmless in practice (the single-node agentic eval runs GSM8K, whose prompts/outputs fit well inside 16384), but the detection is still silently wrong and worth fixing, e.g. by creating the venv with --system-site-packages so the native-context probe still sees the system transformers.
Extended reasoning...
The mechanism is real. When INFMAX_CONTAINER_WORKSPACE != /workspace (the documented normal case for this B200 image, since it mounts InferenceX at /ix), the script runs "$SGLANG_PYTHON" -m venv "$AGENTIC_VENV" with no --system-site-packages, then prepends $AGENTIC_VENV/bin to PATH. This venv starts completely empty. install_agentic_deps (benchmark_lib.sh:1826) builds a separate AIPERF_VENV via uv --python "$AIPERF_PYTHON" and never touches AGENTIC_VENV, so the new venv stays empty until _install_lm_eval_deps eventually runs python3 -m pip install ... lm-eval[api] into it.
In EVAL_ONLY=true mode, run_eval (benchmark_lib.sh:1689-1690) calls compute_eval_context_length before _install_lm_eval_deps runs (that install happens later, inside run_lm_eval, at line 1008). compute_eval_context_length calls get_native_max_context_length (line 908), which shells out to a bare python3 -c '... from transformers import AutoConfig ...'. Since AGENTIC_VENV/bin is first on PATH and empty at this point, the import raises, the except swallows it, and the probe prints 0. Because MAX_MODEL_LEN is unconditionally unset for agentic callers (benchmark_lib.sh:77) and this recipe never calls setup_eval_context, compute_eval_context_length sees benchmark_ctx=0 and native_max=0 and falls through to eval_ctx=${MAX_MODEL_LEN:-16384}=16384 (line 946), emitting a WARN to stderr along the way.
Step-by-step proof:
INFMAX_CONTAINER_WORKSPACEresolves to/ix(not/workspace) for this image, so the script hits theifbranch and createsAGENTIC_VENVempty, thenexport PATH="$AGENTIC_VENV/bin:$PATH".EVAL_ONLY=true→ the script callsrun_eval --port "$PORT".run_evalseesEVAL_MAX_MODEL_LENunset → callscompute_eval_context_length "$MODEL" "${MAX_MODEL_LEN:-0}"withMAX_MODEL_LENunset (→0).compute_eval_context_lengthcallsget_native_max_context_length, which runs barepython3 -c '...AutoConfig...'.python3now resolves inside the emptyAGENTIC_VENV,transformersisn't installed there yet, import fails, function prints0.- Both
benchmark_ctxandnative_maxare0→ fallback branch:eval_ctx=16384,EVAL_MAX_MODEL_LEN=16384. - Later,
run_lm_evalpassesmax_length=16384and computesmax_output_tokens=16384-4096=12288forlm_eval --model_args/--gen_kwargs. - Without the venv (the
/workspacecase), barepython3resolves to the image's system Python, which already hastransformersinstalled, so the probe would have returned DeepSeek-V4-Pro's real (larger) native context instead of0.
Where the original framing overreaches (addressing the refutation). One verifier objection is correct and should be acknowledged: AGENTIC_VENV is not simply redundant dead weight. _install_lm_eval_deps (benchmark_lib.sh:870-887) and the lm_eval invocation itself (line 1029) use bare python3 -m pip install --break-system-packages ... / python3 -m lm_eval. Without something ahead of the system Python on PATH, those bare-python3 calls would install/upgrade lm-eval[api] (and transitively transformers) directly into the SGLang server's system interpreter — exactly the corruption the script's own comment says it's trying to avoid. So the venv does serve a real isolation purpose for the eval path; it isn't redundant with AIPERF_VENV.
The refutation's second point is also correct and matters for grading severity: this config has multinode: false, and per utils/matrix_logic/generate_sweep_configs.py:289-291/405-406, single-node agentic entries run their eval through the GSM8K lm-eval path, not SWE-bench. GSM8K prompts and chain-of-thought generations are far shorter than 16384 tokens, and max_output_tokens still comes out to 12288 either way, so for this recipe's actual eval point there is no truncation or observable score impact — the fallback happens to be harmless here.
Why it's still worth flagging. The detection is genuinely broken (a documented-intent violation per benchmark_lib.sh:70-71, "agentic replays must use the model's native context limit"), it fails silently (only a stderr WARN, no exit), and it's not obviously scoped to "harmless for GSM8K" from the call site — the same pattern will silently misconfigure any future agentic recipe/eval-task combination where the actual context matters, and it's easy to overlook since nothing crashes. A simple, low-risk fix: create AGENTIC_VENV with --system-site-packages (so the bare-python3 native-context probe still resolves transformers from the image's system install while pip installs continue to prefer the venv's own site-packages), or resolve the native-context probe via $SGLANG_PYTHON explicitly instead of a bare python3.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31621811964 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31646630175 |
…00-sglang-agentx # Conflicts: # perf-changelog.yaml
1 participant
Summary
sglang:server metrics.This supersedes the B200 portion of #2145.
Production data audit
The current production database has no aggregate B200 DeepSeek-V4 SGLang AgentX MTP results, so this refresh does not duplicate an up-to-date submission.
Validation