Add DSv4-Pro FP4 B200 disaggregated Dynamo vLLM MTP config - #2390
Add DSv4-Pro FP4 B200 disaggregated Dynamo vLLM MTP config#2390hshrivastava-droid wants to merge 5 commits into
Conversation
- New dsv4-fp4-b200-dynamo-vllm-mtp entry for 8k/1k MTP disagg on cluster:b200-dgxc - Five srt-slurm recipes covering 1P1D through 3P1D topologies - Pin the DSV4 Dynamo vLLM launch to the srt-slurm main branch
|
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 关于重新运行失败任务的文档 |
hshrivastava-droid
added
full-sweep-enabled
full-sweep-fail-fast
and removed
full-sweep-enabled
labels
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30384507275 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding (kv-p2p-transfer mislabeled as mooncake), two other candidates raised during this review were examined and ruled out: the srt-slurm clone for dsv4 dynamo-vllm moving from a pinned branch to --branch main in runners/launch_b200-dgxc.sh — this matches the intentional "pin the DSV4 Dynamo vLLM launch to the srt-slurm main branch" change noted in the perf-changelog entry, though it does affect all existing dsv4+dynamo-vllm configs, not just this one; and the prefill/decode max-model-len mismatch in disagg-b200-1p1d-dep8-tp8-mtp.yaml (9280 vs 16384), which is consistent with the differing prefill/decode roles in this disaggregated recipe.
Extended reasoning...
A real bug was found and is already posted inline (kv-p2p-transfer mislabeled mooncake vs. the actual NixlConnector used by all five recipes), so this PR needs human attention regardless. During this run, two additional candidate issues were raised by finder agents and investigated: (1) the srt-slurm launcher change from a pinned branch/commit checkout to floating on main for dsv4 dynamo-vllm — this turned out to be an intentional, changelog-documented decision ("Pin the DSV4 Dynamo vLLM launch to the srt-slurm main branch"), though worth noting it is a shared launcher change affecting all existing dsv4 dynamo-vllm configs, not scoped to just this new entry; and (2) a prefill (9280) vs decode (16384) max-model-len mismatch in the low-latency tp8 recipe, which is plausible given prefill/decode have different sequence-length responsibilities in a disaggregated setup and matches the pattern of the sibling dep8/dep8 recipes using a single max-model-len. Neither is being flagged as a defect. This note is informational, recording what was already examined so a human reviewer or a future pass doesn't need to re-derive it from scratch.
| image: vllm/vllm-openai:vllm-x86_64-cu13-0.25.1-7a33ba9 | ||
| model: deepseek-ai/DeepSeek-V4-Pro | ||
| model-prefix: dsv4 | ||
| runner: cluster:b200-dgxc |
There was a problem hiding this comment.
🔴 The new dsv4-fp4-b200-dynamo-vllm-mtp entry sets kv-p2p-transfer: mooncake, but all five referenced recipe YAMLs (disagg-b200-*-mtp.yaml) configure kv-transfer-config with kv_connector: NixlConnector for both prefill and decode — every sibling dsv4 dynamo-vllm entry correctly uses nixl. This value is propagated verbatim into the benchmark result JSON as kv_p2p_transfer (via the KV_P2P_TRANSFER env var in generate_sweep_configs.py / process_result.py), so every run from this config will be permanently mislabeled as using Mooncake KV transfer when it actually uses NIXL. Fix is a one-word change: mooncake -> nixl.
Extended reasoning...
The bug: In configs/nvidia-master.yaml, the new dsv4-fp4-b200-dynamo-vllm-mtp entry sets kv-p2p-transfer: mooncake. This is a copy-paste mistake — every one of the five recipe YAMLs it references (disagg-b200-1p1d-dep8-dep8-mtp.yaml, disagg-b200-1p1d-dep8-tp8-mtp.yaml, disagg-b200-1p2d-dep8-dep8-mtp.yaml, disagg-b200-2p1d-dep8-dep8-mtp.yaml, disagg-b200-3p1d-dep8-dep8-mtp.yaml) sets kv-transfer-config: '{"kv_connector": "NixlConnector", ...}' for both the prefill and decode roles, and none of them reference Mooncake anywhere. Every sibling dsv4 dynamo-vllm entry in the same file (e.g. dsv4-fp4-b300-dynamo-vllm, dsv4-fp4-gb300-dynamo-vllm) correctly uses kv-p2p-transfer: nixl to match their identical NixlConnector recipes; mooncake is used elsewhere in the file exclusively by dynamo-sglang entries, confirming this vLLM entry picked up the wrong label.
Why it isn't just cosmetic: kv-p2p-transfer is not inert descriptive text. generate_sweep_configs.py's component_metadata() reads this field and exports it as the KV_P2P_TRANSFER environment variable for the run. utils/process_result.py (and the agentic-path equivalent, process_agentic_result.py) then read that env var and write it verbatim into the benchmark result JSON as data['kv_p2p_transfer'] — this is asserted directly in utils/test_process_result.py. That result JSON is what feeds the perf dashboard/changelog, where cross-connector comparisons (NIXL vs. Mooncake) are made.
Why nothing else catches it: validation.py treats kv_p2p_transfer as a free-form string field (min_length=1) that is only required to be non-null when disagg: true — there is no enum constraint tying it to the actual connector configured in the recipe YAML. So mooncake is accepted as a perfectly valid value even though it doesn't match the recipe, and nothing in the pipeline cross-checks the metadata label against the recipe's real kv_connector.
Step-by-step proof of impact:
- This config entry sets
kv-p2p-transfer: mooncakeat the top level. generate_sweep_configs.pybuilds the sweep matrix and callscomponent_metadata(), which readskv-p2p-transferfrom the config and puts it in the matrix entry that becomesKV_P2P_TRANSFERfor the job.- The GitHub Actions workflow passes
KV_P2P_TRANSFER=mooncakeinto the multinode benchmark job's environment. runners/launch_b200-dgxc.shruns the recipe YAML unmodified — which setskv_connector: NixlConnectorfor prefill and decode — so the run genuinely uses NIXL for KV transfer.- After the run,
utils/process_result.pyreadsKV_P2P_TRANSFERfrom the environment and writesdata['kv_p2p_transfer'] = 'mooncake'into the result JSON. - That JSON is ingested into the perf dashboard/changelog, permanently recording this NIXL-based run as a Mooncake run.
Impact and fix: The actual benchmark execution is unaffected — the connector really used is determined by the recipe YAML's kv-transfer-config, not this field — so the run itself won't fail or produce wrong throughput/latency numbers. But every result published from this config will carry incorrect KV-transfer-backend provenance in the dataset that is the product of this repo, corrupting any cross-connector (NIXL vs. Mooncake) comparison done downstream. The fix is trivial: change kv-p2p-transfer: mooncake to kv-p2p-transfer: nixl at configs/nvidia-master.yaml:5782 to match the recipes and the sibling dsv4 dynamo-vllm entries.
|
sorry @hshrivastava-droid gotta requeue this to pritotize k3 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30384650660 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30585111943 |
中文：更新评估注入器。
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30590035124 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30593514351 |
2 participants
No description provided.