-
Notifications
You must be signed in to change notification settings - Fork 181
[NV] update Minimax2.5 fp8 h100 vllm #1516
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
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
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
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
Oops, something went wrong.
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.
🟡 The comment block at lines 4498-4502 (just below this entry) explicitly claims the original
minimaxm2.5-fp8-h100-vllmentry is byte-identical to origin/main with metadata identical to origin/main's version. This PR breaks both invariants by changing the image (v0.21.0→v0.19.1-cu130) and rewriting the search-space (tp:4 ep:4 conc-end 64→tp:8 ep:8 conc-end 128), so the comment will mislead future readers. Please update or delete those four comment lines in the same commit.Extended reasoning...
What the stale comment says (lines 4498-4502 post-PR):\n\n
yaml\n# Diverged from minimaxm2.5-fp8-h100-vllm (agentic-coding sibling). Metadata is\n# identical to origin/main's minimaxm2.5-fp8-h100-vllm; the split exists because this\n# PR adds an agentic-coding scenarios block that differs from main\n# (either main had none or had a different conc/offload sweep).\n# The original minimaxm2.5-fp8-h100-vllm entry stays byte-identical to origin/main.\n\n\nThis block was authored when the-agenticsibling was split off, as a promise from that earlier PR that it would not touch the originalminimaxm2.5-fp8-h100-vllmentry. The promise was true at the moment it was written.\n\nWhy this PR invalidates the comment:\n\nThis PR is precisely the one that modifies the original entry, so both present-tense claims in the comment go stale the moment it lands:\n\n1. "Metadata is identical to origin/main's minimaxm2.5-fp8-h100-vllm" — false against post-merge main. After merge, the agentic sibling pinsvllm/vllm-openai:v0.20.2and usestp:4 ep:4 conc-end 64, while the new original pinsvllm/vllm-openai:v0.19.1-cu130and usestp:8 ep:8 conc-end 128. The two entries now diverge in both image AND parallelism strategy — not just the scenarios block.\n2. "The original minimaxm2.5-fp8-h100-vllm entry stays byte-identical to origin/main" — directly contradicted by the very hunk above the comment, which changes the image line and rewrites twosearch-spacelists.\n\nStep-by-step proof:\n\n1. Read line 4480 of the post-PR file:image: vllm/vllm-openai:v0.19.1-cu130.\n2. Read line 4504 of the post-PR file:image: vllm/vllm-openai:v0.20.2(the agentic sibling).\n3. The agentic sibling's image differs from the original's image → claim #1 ("identical to origin/main's minimaxm2.5-fp8-h100-vllm") is no longer the relationship the comment describes; the agentic sibling is no longer a metadata twin of the original.\n4. The PR diff forminimaxm2.5-fp8-h100-vllmshows non-trivial edits (image bump + search-space rewrite at lines 4480, 4492, 4496) → the original entry is no longer byte-identical to origin/main, refuting claim #2.\n\nImpact:\n\nDocumentation-only — no runtime effect, hence nit severity. But a maintainer reading the file post-merge will see two clearly diverged sibling entries with a comment underneath asserting they're metadata twins and that one of them is unchanged from main. That is actively misleading and the kind of thing that wastes future reviewers' time tracing history.\n\nSuggested fix:\n\nReplace the four comment lines with a brief note describing the current relationship (e.g., "-agenticsibling pins v0.20.2 with tp:4/ep:4 for cpu-offload sweeps; the primary entry tracks the latest minimax recipe"), or simply delete the now-obsolete annotation. Either is a one-line change in the same commit.