Port GLM-5.1 TileRT fixed-sequence benchmarks to srt-slurm - #2755
Port GLM-5.1 TileRT fixed-sequence benchmarks to srt-slurm#2755cquil11 wants to merge 8 commits into
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 关于重新运行失败任务的文档 |
d30e15c to
e459bd4
Compare
b9ed5d7 to
82d5e44
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 82d5e44. Configure here.
| --trust-remote-code \ | ||
| --save-result \ | ||
| --result-dir "$result_dir" \ | ||
| --result-filename "results_concurrency_${concurrency}_gpus_16_ctx_8_gen_8.json" |
There was a problem hiding this comment.
Benchmark omits warmup requests
Medium Severity
The custom benchmark_serving.py invocation never passes --num-warmups. The previous TileRT path always warmed up with twice the concurrency via run_benchmark_serving, while the script default is 0. At the preserved conc-1 points that folds cold-start requests into TTFT/TPOT, so the ported 1k/1k and 8k/1k numbers are not comparable to the old launcher.
Reviewed by Cursor Bugbot for commit 82d5e44. Configure here.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the missing MODEL_PATH default in the new glm5.1+tilert branch (runners/launch_b200-nscale-slurm.sh:52) — the recipe hardcodes model.path: "hf:zai-org/GLM-5.1-FP8" directly (same pattern already used for glm5.2 in launch_h200-dgxc-slurm.sh), so the model resolves via that literal HF ref regardless of the resulting empty model_paths alias entry, and this does not look like a functional bug.
Extended reasoning...
Three confirmed findings (routing-bypass gap for glm5.1-fp8-b200-tilert-agentic, the perf-changelog.yaml mid-file blank-line removal, and the double #-insertion in enroot_uri_for_image for the pre-formatted DCGM_EXPORTER_ENROOT_REF) are being posted as inline comments and are sufficient reason to withhold approval. I additionally verified the ruled-out candidate that the new glm5.1+tilert branch never sets MODEL_PATH (unlike the dsv4/kimik2.6/kimik3 branches above it): the new recipe YAML hardcodes model.path: "hf:zai-org/GLM-5.1-FP8", matching the existing precedent in launch_h200-dgxc-slurm.sh where glm5.2's model.path falls back to the same hf:zai-org/... literal, so the empty model_paths alias entry this produces in the generated srtslurm.yaml appears to be dead/unused rather than a functional defect. Given the confirmed findings already flag real routing and byte-append-only violations, a human should still look at this PR.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
runners/launch_b200-nscale-slurm.sh:59— New glm5.1+fp8+tilert compat-launcher bypass matches by model-prefix/precision/framework/spec-decoding only, with no IS_AGENTIC guard (unlike the existing kimik3 precedent at line 122), so it also captures the untouched glm5.1-fp8-b200-tilert-agentic config.Extended reasoning...
glm5.1-fp8-b200-tilert-agentic (configs/nvidia-master.yaml, same model-prefix/precision/framework/spec-decoding=mtp, no CONFIG_FILE additional-setting) now falls through into the native srt-slurm path instead of run_compat_launcher. It hits the generic 'CONFIG_FILE is not set' check further down and exits with an error, breaking the agentic-coding sweep that worked before this PR via the legacy compat launcher.
Verification: normal — the new compat-launcher bypass is scenario-blind and regresses the untouched agentic config. runners/launch_b200-nscale-slurm.sh:57-60 (PR) adds: [[ $MODEL_PREFIX != "glm5.1" || $PRECISION != "fp8" || $FRAMEWORK != "tilert" || $SPEC_DECODING != "mtp" ]] This bracket matches purely on model-prefix/precision/framework/spec-decoding, unlike the kimik3 precedent that gates on…
-
🔴
runners/launch_b200-nscale-slurm.sh:199— import_squash() now always runs enroot_uri_for_image() on image_ref (line 199), but DCGM_EXPORTER_ENROOT_REF passed in at line 239 is already pre-formatted with a registry '#' separator (nvcr.io#nvidia/...), so enroot_uri_for_image re-splits it and inserts a second '#'.Extended reasoning...
For image_ref='nvcr.io#nvidia/k8s/dcgm-exporter:...', first_component='${image_ref%%/*}' evaluates to 'nvcr.io#nvidia' (contains '.', matches the registry-qualified branch), producing 'docker://nvcr.io#nvidia#k8s/dcgm-exporter:...' — a malformed enroot URI with a duplicate '#'. Any USES_DCGM_POWER=1 run (dcgm-power-enabled fixed-sequence DSV4/Kimi-K2.6 lanes) will fail the DCGM exporter
enroot import, whereas before this diff the same DCGM_EXPORTER_ENROOT_REF was passed straight toenroot import -o ... docker://$image_refand worked. Fix: enroot_uri_for_image must detect an already-'#'-qualified ref (or the DCGM caller must pass the raw image and let enroot_uri_for_image do the qualification) rather than reprocessing a pre-formatted URI.Verification: normal — regression on the DCGM-power path introduced by this diff. On base (commit 00e4d79 lines 176/190)
import_squashdidenroot import -o "$squash_file" "docker://$image_ref", so the pre-formatted DCGM ref built at line 237DCGM_EXPORTER_ENROOT_REF="${DCGM_EXPORTER_IMAGE/nvcr.io\//nvcr.io#}"=nvcr.io#nvidia/k8s/dcgm-exporter:4.6.0-4.8.3-distrolessbecame the correct URI… -
🟡
perf-changelog.yaml:6493— Diff removes the pre-existing blank separator line between the PR #2688 and qwen3.8next entries while appending the new PR #2755 entry.Extended reasoning...
perf-changelog.yaml is documented in AGENTS.md as append-only and byte-sensitive ('Preserve all existing bytes and separator whitespace, and append only at the tail'); this diff also mutates bytes mid-file, violating that invariant even though the new entry itself is correctly tail-appended.
Verification: nit. The diff hunk
@@ -6490,7 +6490,6 @@removes a blank separator line (the-empty line) between the pre-existing PR #2688 entry (pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2688) and theqwen3.8next-fp8-h200-sglang-agentic-mtpentry, while the new PR #2755 entry is separately tail-appended. AGENTS.md line 21 states: "The file is append-only and byte-sensitive. Preserve…
|
Closing because the project scope is now limited to porting active, nondeprecated AMD multi-node configurations to srt-slurm. This PR is outside that scope. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33087514017 |
1 participant
Summary
benchmark_serving.pydirectly throughbenchmark.type: customValidation
bash -n runners/launch_b200-nscale-slurm.shsrtctl dry-runresolves the two-node TileRT topology and custom benchmarkutils/matrix_logic: 255 tests passRuntime sweep is intentionally deferred until the active AgentX TileRT validation proves model download, conversion, prefill, decode, and router health on the same shared implementation.
Note
Low Risk
Benchmark orchestration and Slurm recipe wiring only; no changes to inference serving logic or security-sensitive paths.
Overview
Moves B200 GLM-5.1 FP8 TileRT fixed-sequence 1P1D MTP sweeps off the manual
glm5.1_fp8_b200_tilert-disagg.shlauncher and onto native srt-slurm via a new recipe atdisagg-1p1d-tp8-mtp.yaml(vLLM prefill, TileRT decode, TileRT router, NIXL, MTP).The 1k/1k and 8k/1k concurrency-1 matrix entries in
nvidia-master.yamlnow setCONFIG_FILEto that recipe. Benchmarking runs InferenceXbenchmark_serving.pythroughbenchmark.type: custominstead of the old submit path.launch_b200-nscale-slurm.shclones the pinned TileRT srt-slurm fork for allglm5.1+tilertjobs (not only agentic), copies the fullb200-fp8recipe tree, and ensures HF hub + TileRT weights mounts for TileRT even when agentic mode is off.perf-changelog.yamldocuments the migration forglm5.1-fp8-b200-tilert.Reviewed by Cursor Bugbot for commit 82d5e44. Bugbot is set up for automated code reviews on this repo. Configure here.