[DSv4] Refresh GB200 dynamo-vLLM 1k1k sweep - #2190
Closed
Oseltamivir wants to merge 2 commits into
Closed
Oseltamivir wants to merge 2 commits into
Oseltamivir wants to merge 2 commits into
Claude / Claude Code Review
completed
Jul 14, 2026 in 14m 9s
Code review found 2 important issues
Found 6 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/1k1k/disagg-gb200-1p1d-dep8-tep8.yaml:130-149 |
tep8 decode enables expert-parallel, contradicting the ep:1 TEP label in master config |
| 🔴 Important | benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/1k1k/disagg-gb200-1p1d-dep8-dep16.yaml:87-106 |
Missing tokenizer-mode: deepseek_v4 in dep8-dep16 recipes |
| 🟡 Nit | benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/1k1k/disagg-gb200-1p1d-dep8-tep8.yaml:10-12 |
Broken file-path references in recipe header comments |
| 🟡 Nit | benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/1k1k/disagg-gb200-1p1d-dep8-dep16.yaml:136 |
use_chat_template: false diverges from all other DSv4 recipes |
Annotations
claude / Claude Code Review
tep8 decode enables expert-parallel, contradicting the ep:1 TEP label in master config
In the restored `disagg-gb200-1p1d-dep8-tep8.yaml` recipe, the decode block sets `tensor-parallel-size: 8` and `enable-expert-parallel: true` (no `data-parallel-size`), so the decode engine actually runs with expert parallelism enabled. But `configs/nvidia-master.yaml` labels this decode as `tp: 8, ep: 1, dp-attn: false` (pure TEP), so the published conc 1-64 row for this topology will be mislabeled relative to what the engine actually runs. The fix is to comment out `enable-expert-parallel` in
claude / Claude Code Review
Missing tokenizer-mode: deepseek_v4 in dep8-dep16 recipes
Both new dep8-dep16 recipes (1p1d and 3p1d) omit `tokenizer-mode: deepseek_v4` in their prefill and decode `vllm_config` blocks, unlike every other DeepSeek-V4-Pro recipe in the tree (including the tep8 sibling added in this same PR).
claude / Claude Code Review
Broken file-path references in recipe header comments
Two header-comment path references in the restored 1k1k recipes point to nonexistent files. In disagg-gb200-1p1d-dep8-tep8.yaml (lines 10-12), the comment points to '../8k1k/disagg-gb200-1p1d-dep8-tep8.yaml' but the actual GB200 8k1k low-latency sibling is named disagg-gb200-low-latency.yaml. In disagg-gb200-1p1d-dep8-dep16.yaml (lines 2-4), the comment references 'kimi-k2.5/1k1k/disagg-gb200-1p1d-dep4-dep16.yaml' but the actual directory is kimi-k2.5-fp4/1k1k/ (missing the -fp4 suffix). Both ar
claude / Claude Code Review
use_chat_template: false diverges from all other DSv4 recipes
The three new 1k1k vLLM DSv4-Pro recipes set `benchmark.use_chat_template: false` with no `custom_tokenizer`, while every one of the 20 existing DeepSeek-V4-Pro vLLM 8k1k recipes uses `use_chat_template: true` + `custom_tokenizer: sa_bench_tokenizers.vllm_deepseek_v4.VLLMDeepseekV4Tokenizer`. This looks like a leftover from the kimi-k2.5 template cited in the header comments rather than a deliberate choice, and isn't called out in the PR description — worth a quick confirmation, though the actua
Loading