[NV] Update 8k1k B300 Minimax Dynamo Disagg#2184
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 关于重新运行失败任务的文档 |
…m3-b300-8k1k # Conflicts: # perf-changelog.yaml
|
Claude finished @jasonlizhengjian's task in 4m 46s —— View job Review: [DoNotMerge] Update 8k1k B300 Minimax Dynamo Disagg
SummaryNo blocking issues found. One 🟡 WARNING posted inline: the new launcher branch pins srt-slurm at Everything else checked out:
Note: I could not run the pytest matrix suite or reach external APIs (Docker Hub / srt-slurm GitHub) from this sandbox — those checks are covered by CI and the inline comment respectively. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29297512174 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29298636780 |
Add the B300 8P2D TP1-prefill/TEP4-decode 8k1k recipe at concurrency 2048, pinned to the validated newer vLLM nightly and Dynamo 1.3.0.dev20260713.\n\n中文:新增 B300 8P2D TP1 预填充/TEP4 解码的 8k1k 配置,仅测试并发 2048,并固定到已验证的新 vLLM nightly 与 Dynamo 1.3.0.dev20260713。
Restore the proven 4P2D DEP2-prefill/TEP4-decode concurrency-4096 recipe exactly as previously submitted, in an 8k1k-only config that avoids rerunning 1k1k. Remove the experimental TP1/TEP4 concurrency-2048 replacement.\n\n中文:原样恢复已验证的 4P2D DEP2 预填充/TEP4 解码并发 4096 配置,并将其放入仅含 8k1k 的独立配置,避免重新运行 1k1k;移除实验性的 TP1/TEP4 并发 2048 替代配置。
|
Claude finished @jasonlizhengjian's task in 4m 28s —— View job Re-review: Update 8k1k B300 Minimax Dynamo Disagg
Re-review: no new blocking issuesFour commits landed since my previous review (01:02 UTC): the Preflight bypass ( Dynamo bump (merge Legacy max-throughput point (
Repo rules re-verified on the new diff: master-config changes are paired with a Still open from the previous review: the 🟡 inline warning on |
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2198 | ||
| - config-keys: |
There was a problem hiding this comment.
🔴 The new changelog entry for PR #2184 is appended immediately after the prior pull/2198 entry with no blank separator line, unlike every other entry boundary in this file. utils/validate_perf_changelog.py (run in CI via run-sweep.yml) requires exactly one blank line before an appended - config-keys: entry and will raise ChangelogValidationError here, failing the changelog gate. Fix: insert one blank line before the new - config-keys: line at perf-changelog.yaml:4784.
Extended reasoning...
This PR appends a new perf-changelog entry (for the minimaxm3-fp4-b300-dynamo-vllm-8k1k-tp1 / -legacy-max-tput config keys) directly after the previous entry's pr-link: .../pull/2198 line, with no blank line separating the two entries. Every other entry boundary in perf-changelog.yaml has exactly one blank line before the next - config-keys: line — this new entry breaks that pattern.
This is enforced by a real CI gate, not just a style convention. utils/validate_perf_changelog.py::validate_raw_change() is invoked on every sweep PR via .github/workflows/run-sweep.yml:102. For an appended entry, it computes:
suffix = head_raw[len(base_raw):]
expected_start = (
b"- config-keys:"
if base_raw.endswith(b"\n\n")
else b"\n- config-keys:"
)
if not suffix.startswith(expected_start):
raise ChangelogValidationError(
"new changelog entries must be separated from history by one "
"empty line and appended at the end"
)Step-by-step proof:
- The base file (before this PR) ends its final entry (PR fix: recover PR 2108 ingest via sweep reuse / 通过扫描复用恢复 PR 2108 的结果入库 #2198) with a single trailing
\n— the validator itself forbids a base ending in\n\nvia a separate check a few lines down (the final appended changelog entry must end with one newline), sobase_raw.endswith(b"\n\n")isFalse. - Therefore
expected_start = b"\n- config-keys:"— the validator requires the appended suffix to begin with a newline (i.e., a blank line) followed by- config-keys:. - Reading the actual file bytes around perf-changelog.yaml:4783-4784 confirms the diff appends
- config-keys:immediately afterpr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2198\nwith zero blank lines in between. Sosuffixbegins with- config-keys:, not\n- config-keys:. suffix.startswith(expected_start)evaluates toFalse, sovalidate_raw_change()raisesChangelogValidationError("new changelog entries must be separated from history by one empty line and appended at the end"), which exits non-zero and fails the changelog-gate step of the sweep pipeline.
One verifier raised a refutation arguing that utils/process_changelog.py's get_added_lines() (which extracts only +-prefixed diff lines and YAML-parses them) is indifferent to blank lines, and concluded the missing separator has "zero functional impact." That is true of process_changelog.py, but it is the wrong validator to look at — process_changelog.py handles ingesting changelog content into results, while utils/validate_perf_changelog.py is the distinct gate that CI runs against the raw file bytes to enforce formatting invariants (including the exact one-blank-line separator convention), and that gate does not tolerate this diff. The refutation's conclusion doesn't hold once the correct validator is traced.
Fix: insert a single blank line before the new - config-keys: line (i.e., between the pull/2198 pr-link line and the new entry), matching every other entry boundary in the file. This is a one-line fix but it currently produces a deterministic, CI-blocking failure — this PR's entire purpose is to trigger the 8k1k sweep via run-sweep.yml, and that pipeline step will fail before the sweep even runs.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29358158271 |
Summary
nightly-2dfaae752b4db0d43cfc0715c780e33be030d0f1)c1fb6989fc5aca803b4ca0f2d17d8be85fad97328k1k sweep
Validation
srtctl dry-run