-
Notifications
You must be signed in to change notification settings - Fork 242
[NV] minimax-m3-gb300-dynamo: nightly image + 2xDEP8 decode to 1xDEP16 / [NV] minimax-m3-gb300-dynamo:nightly 镜像 + 2×DEP8 解码合并为 1×DEP16 #2340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+35
−70
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c79991c
[NV] minimax-m3-gb300-dynamo: vLLM nightly + dynamo bump; collapse 2x…
xinli-sw 5dbc379
[NV] minimax-m3-gb300-dynamo: add perf-changelog pr-link
xinli-sw fa24e0b
Merge remote-tracking branch 'origin/main' into minimaxm3-fp8-gb300-d…
xinli-sw 77fa834
Merge remote-tracking branch 'origin/main' into pr-2340-reuse-11657
Ankur-singh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
38 changes: 0 additions & 38 deletions
38
benchmarks/multi_node/srt-slurm-recipes/configs/minimax-m3-gb300-vllm-fixes.sh
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 In benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3-gb300-fp8/8k1k/3p1d-dep2-dep16-8k1k.yaml, the descriptive comment on line 3 still says '2D DEP8 decode (TP1 DP8 EP, 8 GPU/worker = 2 nodes each)', describing the pre-PR topology. This PR collapsed decode to a single DEP16 worker (decode_workers 2->1, gpus_per_decode 8->16, data-parallel-size 8->16) and correctly updated the
name:field and the matching comment in configs/nvidia-master.yaml, but missed this in-file comment. It's a documentation-only inconsistency with no runtime effect — worth a quick fix before merge for clarity.Extended reasoning...
What's wrong: Line 3 of
3p1d-dep2-dep16-8k1k.yaml(renamed from3p2d-dep2-dep8-8k1k.yaml) reads:This describes the pre-PR decode topology: two DEP8 decode workers, each spanning 2 nodes (2 x 8 GPU = 16 GPU total across 2 workers). That topology no longer exists in this file.
What actually changed: The PR's decode-collapse diff for this file is:
So post-PR the decode side is a single DEP16 worker (TP1 DP16 EP, 16 GPU spanning 4 nodes) — not 2D DEP8. The comment on line 3 is now self-contradictory with the
resources:block a few lines below it (decode_workers: 1,gpus_per_decode: 16) and with thevllm_config.decodeblock (data-parallel-size: 16).Why this wasn't caught elsewhere: the author clearly intended to keep documentation in sync — they updated the
name:field from...3p2d-dep2-dep8...to...3p1d-dep2-dep16..., and they updated the analogous comment inconfigs/nvidia-master.yamlfrom# 3p2d DEP2+DEP8, 6nto# 3p1d DEP2+DEP16, 6n. Both of those sibling annotations were fixed, which is strong evidence this in-file comment on line 3 was simply missed rather than intentionally left as-is.Proof walkthrough:
decode_workers: 2,gpus_per_decode: 8→ 2 workers x 8 GPU = 16 GPU, each worker = 8/4 = 2 nodes. Comment ('2D DEP8 ... 2 nodes each') matches.decode_workers 2->1,gpus_per_decode 8->16,data-parallel-size 8->16.Impact: This is a comment-only defect — the executable
resources:andvllm_config:blocks are correct, so the recipe runs with the intended 1xDEP16 topology regardless. The only harm is to a future reader (or another automation adapting this recipe) who trusts the comment over the config and misunderstands the decode topology.Fix: Update line 3 to match the pattern already used in
configs/nvidia-master.yaml, e.g.:Severity is nit since it has zero effect on execution or benchmark results — it is purely a stale explanatory comment.