Skip to content

Add DSV4 B200 disaggregated Dynamo SGLang STP configuration / 新增 DSV4 B200 分离式 Dynamo SGLang STP 配置 - #2560

Open
RohitNagraj wants to merge 2 commits into
mainfrom
dsv4-fp4-b200-dynamo-sglang-nscale
Open

Add DSV4 B200 disaggregated Dynamo SGLang STP configuration / 新增 DSV4 B200 分离式 Dynamo SGLang STP 配置#2560
RohitNagraj wants to merge 2 commits into
mainfrom
dsv4-fp4-b200-dynamo-sglang-nscale

Conversation

@RohitNagraj

Copy link
Copy Markdown
Collaborator

Description

Add a B200 multinode disaggregated Dynamo SGLang STP configuration for DSV4 at 8k/1k.

  • Add ten checked-in srt-slurm recipes matching the configured prefill and decode topologies.
  • Resolve the staged DSV4 checkpoint through the B200 multinode launcher.
  • Use Mooncake for KV transfer with chat-formatted benchmark inputs.

中文说明

新增 DSV4 在 B200 上的多节点分离式 Dynamo SGLang STP 8k/1k 配置。

  • 新增十个已检入的 srt-slurm recipe,与配置中的预填充和解码拓扑一一对应。
  • 通过 B200 多节点启动器解析已暂存的 DSV4 检查点。
  • 使用 Mooncake 进行 KV 传输,并对基准测试输入应用聊天模板。

Related Issue

Supersedes #2251.
替代 #2251

Type of Change

  • Bug fix
  • New feature
  • Configuration change
  • Documentation update
  • Other (please describe)

Checklist

  • I have tested my changes locally
  • I have updated documentation if necessary
  • For every change that can affect benchmark performance and every recipe addition or modification, I have appended a new entry to the physical end of perf-changelog.yaml and have not edited historical entries
  • Before merging via reuse, an authorized maintainer (OWNER/MEMBER/COLLABORATOR) has commented /reuse-sweep-run on this PR

新增 DSV4 B200 Dynamo-SGLang STP 配置,包含十个分离式拓扑并接入 B200 多节点运行环境。
@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

将性能变更日志条目关联到 PR #2560
@@ -0,0 +1,145 @@
name: "disagg-b200-1p1d-dep8-tp8-4-c64"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The name: field in disagg-b200-1p1d-dep8-tp8.yaml is "disagg-b200-1p1d-dep8-tp8-4-c64", with a stray -4- segment that doesn't match this recipe's topology (1 prefill + 1 decode = 2 nodes) or the naming convention used by every other new B200 recipe in this PR (e.g. disagg-b200-1p4d-dep8-tp8-c64, disagg-b200-1p1d-tp8-tp8-c1). It looks like a copy-paste leftover from a GB200-style template where that suffix denotes total node count; harmless at runtime since launch_b200-nscale-slurm.sh overwrites the name via sed before srtctl reads it, but worth cleaning up for consistency.

Extended reasoning...

The name: field on line 1 of disagg-b200-1p1d-dep8-tp8.yaml reads "disagg-b200-1p1d-dep8-tp8-4-c64". Every other new B200 recipe added in this PR follows a clean <topology>-c<concurrency> naming pattern with no extra numeric segment: disagg-b200-1p1d-dep8-dep8.yaml -> disagg-b200-1p1d-dep8-dep8-c512, disagg-b200-1p1d-tp8-tp8.yaml -> disagg-b200-1p1d-tp8-tp8-c1, disagg-b200-1p4d-dep8-tp8.yaml -> disagg-b200-1p4d-dep8-tp8-c64, and so on through the 3p2d/4p2d/5p2d/6p2d files. This file alone inserts a stray -4- between the topology and the concurrency suffix.\n\nOn the existing GB200 recipes in this codebase, that kind of numeric segment is a real, intentional convention: it denotes total node count (prefill_nodes + decode_nodes), e.g. a GB200 disagg-gb200-1p4d-dep8-tp8-10-c64.yaml-style file uses -10- because it sums to 10 nodes. Applying that same mental model here breaks down: this recipe's resources: block sets prefill_nodes: 1 and decode_nodes: 1, i.e. 2 total nodes, not 4. So -4- isn't just inconsistent with sibling files in this PR, it's also the wrong number under the very convention it appears to be borrowed from — strong evidence this is a copy-paste leftover from a GB200-style template rather than a deliberate choice.\n\nProof by walkthrough: (1) open disagg-b200-1p1d-dep8-tp8.yaml, line 1 is name: \"disagg-b200-1p1d-dep8-tp8-4-c64\"\". (2) Check resources: in the same file: prefill_nodes: 1, decode_nodes: 1 -> topology is 1p1d, 2 nodes total. (3) Compare to the sibling disagg-b200-1p4d-dep8-tp8.yaml in the same PR, which has prefill_nodes: 1, decode_nodes: 4 (5 nodes total) and is named disagg-b200-1p4d-dep8-tp8-c64 — no numeric node-count segment despite actually having more nodes. (4) There is therefore no consistent rule under which -4- belongs in the 1p1d file's name.\n\nWhy nothing currently prevents or catches this: recipe names are free-form YAML string values with no schema validation, and nothing in configs/nvidia-master.yaml or the sweep matrix reads the name: field for routing — the matrix entry in nvidia-master.yaml references the recipe purely by its CONFIG_FILE path, not by the embedded name. And at runtime, runners/launch_b200-nscale-slurm.sh executes sed -i \"s/^name:.*/name: \\\"\\\\"/\" \"\\"\" before \"srtctl apply\"\" ever runs, unconditionally replacing whatever is on that line with the runner-generated name. So this typo has zero effect on job submission, GPU allocation, or benchmark results — it is purely a checked-in metadata string that a human reading the repo (or grepping recipe names) would see and find confusing.\n\nFix is trivial: change line 1 to name: "disagg-b200-1p1d-dep8-tp8-c64"`, matching the convention of every sibling file added in this PR. Given the complete absence of functional impact (confirmed by tracing the sed override in the launcher) and the triviality of the fix, this is cosmetic — nit severity, not a merge blocker.

Comment thread perf-changelog.yaml Outdated
Comment on lines +5779 to +5786

- config-keys:
- dsv4-fp4-b200-dynamo-sglang
description:
- "Add a DeepSeek-V4-Pro FP4 Dynamo-SGLang STP configuration for B200 with ten 8k/1k disaggregated topologies."
- "Use Mooncake for KV transfer with chat-formatted benchmark inputs."
- "Image: lmsysorg/sglang:nightly-dev-cu13-20260708-b3632494"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new perf-changelog.yaml entry for dsv4-fp4-b200-dynamo-sglang has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX, a literal unresolved placeholder. Every other entry in the file uses the real PR number, and since this PR is #2560, XXX should be replaced with 2560.

Extended reasoning...

The tail end of the newly-appended perf-changelog.yaml entry (config-keys: [dsv4-fp4-b200-dynamo-sglang]) reads:

pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

XXX is a literal, unresolved placeholder rather than an actual PR number.

Why this happened: perf-changelog.yaml entries are appended before the PR number is known (the PR doesn't exist yet at authoring time), so authors commonly stub the link with a placeholder and are expected to swap in the real number once GitHub assigns it. That final substitution step was missed here.

Evidence this is the convention, not a one-off: every other entry in the file uses a real, resolvable PR number — the immediately preceding entry (the B300 DSV4 AgentX MTP retune) links to /pull/2536, and entries further back link to /pull/2309, /pull/2238, and /pull/2206. This PR's own metadata identifies it as #2560, so the correct value is unambiguous.

Step-by-step proof:

  1. Open perf-changelog.yaml at the new entry (added at the physical end of the file, lines 5779-5786).
  2. Read the last field: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  3. Compare against the pr-metadata for this change: <pr number="2560">.
  4. XXX != 2560 — the link is broken/dangling and does not resolve to this PR (or any PR).
  5. Scan sibling entries in the same file (2536, 2309, 2238, 2206, ...) — all use real numbers, confirming XXX is an anomaly, not an established pattern.

Impact: This is a documentation/traceability defect, not a functional one — it doesn't affect any benchmark execution, config resolution, or CI job. But it does violate the PR's own checklist item ("I have appended a new entry to the physical end of perf-changelog.yaml ... accurate changelog entry") and breaks the changelog's utility as an audit trail back to the originating PR, since anyone auditing changelog history via pr-link will hit a 404 for this entry.

Fix: Replace XXX with 2560 in the pr-link field before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant