[AgentX] vLLM DeepSeek-V4 GB200 disagg test#2282
Conversation
…to dynamo version mismatch vs aiperf
Backport srt-slurm health expectations for per-node vLLM data-parallel launches and apply the patch from the GB200 agentic launcher.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29696954138 |
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
perf-changelog.yaml:4950-4963— The new perf-changelog.yaml entry lists 4 disagg config-keys (1p1d-dep8-dep8, 1p1d-dep8-dep12, 2p1d-dep8-dep12, 3p1d-dep8-dep16) that are entirely commented out in configs/nvidia-master.yaml, so process_changelog.py's get_config_keys_from_master() will raise ValueError and fail the changelog-validation step in CI. Separately, the entry's pr-link points to PR #2260 instead of this PR (#2282), which will block the reuse-merge tooling's link-validation check, and the description's concurrency lists don't match the actual conc-list values in the corresponding recipe blocks. Either uncomment the four disagg blocks in nvidia-master.yaml (if they are meant to ship) or drop them from the changelog entry, and fix the pr-link and concurrency prose.Extended reasoning...
What's wrong: The changelog entry appended at
perf-changelog.yaml:4950-4963lists fiveconfig-keys:dsv4-fp4-gb200-dynamo-vllm-agentic-agg,-1p1d-dep8-dep8,-1p1d-dep8-dep12,-2p1d-dep8-dep12, and-3p1d-dep8-dep16. Inconfigs/nvidia-master.yaml, only the-agentic-aggblock is an active (uncommented) top-level key; the other four disagg blocks are entirely commented out (every line, including the key itself, is prefixed with#), soyaml.safe_load/load_config_filesnever produces them as keys in the parsed master config.Where it breaks:
utils/process_changelog.py'sget_config_keys_from_master()(lines 101-102) doeselif key not in master_config: raise ValueError(f\"Config key '{key}' not found in master configs.\")for any literal (non-wildcard) key.main()parses only the added lines of the changelog viaget_added_lines()and calls this function for every entry'sconfig_keys(line ~155-158 in the snippet above). Since this changelog entry is itself the addition, all five of its keys get validated, and four of them are missing frommaster_config— the first missing key raises immediately. This function is invoked by CI:.github/workflows/run-sweep.yml:242runsprocess_changelog.py --changelog-file perf-changelog.yamlon PR builds, and per AGENTS.md changelog validation still runs even when the benchmark stages are skipped. So this PR, as written, fails that CI step outright.Why existing code doesn't catch it earlier: there's no schema-level cross-check between
perf-changelog.yamlandconfigs/nvidia-master.yamlat authoring time — the mismatch is only caught whenprocess_changelog.pyactually resolves the keys against the loaded master config, which only happens in this specific validation script/CI step.Step-by-step proof:
- CI checks out this PR and runs
process_changelog.py --base-ref <main> --head-ref <PR> --changelog-file perf-changelog.yaml. get_added_lines()extracts the diff-added YAML block, which is exactly the entry at lines 4950-4963, containing all fiveconfig-keys.yaml.safe_loadparses that block into oneChangelogEntrywithconfig_keys = [agentic-agg, 1p1d-dep8-dep8, 1p1d-dep8-dep12, 2p1d-dep8-dep12, 3p1d-dep8-dep16].main()loadsmaster_configfromconfigs/nvidia-master.yaml(and other master configs) viaload_config_files, in which the four disagg keys don't exist (they're commented out).get_config_keys_from_master(entry.config_keys, master_config)iterates the keys; on reachingdsv4-fp4-gb200-dynamo-vllm-agentic-1p1d-dep8-dep8(not a wildcard, not inmaster_config), it raisesValueError(\"Config key 'dsv4-fp4-gb200-dynamo-vllm-agentic-1p1d-dep8-dep8' not found in master configs.\").- The workflow step fails, blocking the PR's sweep-gating CI.
Secondary issues on the same entry: the
pr-linkis.../pull/2260, but this change is PR #2282. The reuse-merge tooling (prepare_perf_changelog_merge.py'svalidate_added_pr_link) requires an appended entry'spr-linkto equal the merging PR number or be anXXX-style placeholder;#2260is neither, socanonicalizewill raise and block that merge path. Additionally, the description's concurrency lists (e.g. "1P/1D DEP8/DEP8 at [64,128,256,320]") don't match the actualconc-listvalues in the corresponding (commented-out) recipe blocks ([64, 128, 192, 256], etc., with similar mismatches for the other three disagg configs) — likely leftover text from an earlier draft.Fix: Either uncomment the four disagg blocks in
nvidia-master.yamlif they're meant to be part of this rollout, or drop those four keys from the changelog entry so onlydsv4-fp4-gb200-dynamo-vllm-agentic-aggremains. Updatepr-linkto.../pull/2282(or anXXXplaceholder), and correct the concurrency lists in the description to match the recipes actually being added. - CI checks out this PR and runs
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29697053098 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29701690243 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29702245088 |
No description provided.