-
Notifications
You must be signed in to change notification settings - Fork 217
Update Minimax M3 B300 FP4 vllm #1994
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
Open
wzhao18
wants to merge
5
commits into
main
Choose a base branch
from
wzhao/minimax-m3-b300-fp4
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+8
−2
Open
Changes from all commits
Commits
Show all changes
5 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
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 new perf-changelog entry uses a separator line with two trailing spaces (
\n) at line 4436 instead of a truly-empty line. This will abortutils/merge_with_reuse.shat theprepare_perf_changelog_merge.py canonicalizestep withChangelogValidationError: new changelog entries must be separated from history by one empty line and appended at the end. Strip the two trailing spaces on line 4436 so the separator is just\n.Extended reasoning...
What's wrong
Between the previous entry (ending at line 4435 with
pull/2001) and the new entry (starting at line 4437 with- config-keys:), the separator on line 4436 is\n(two spaces then newline) instead of a truly-empty line\n. Every other separator between entries in this file (e.g. lines 4403, 4411, 4418, 4425) is a bare empty line — this new one is the odd one out.Why PR-time CI does not catch this
PR CI runs
utils/validate_perf_changelog.py(see.github/workflows/run-sweep.yml:102), whose CLImain()routes tovalidate_matrix_compatible_change→validate_generated_configonly. That path checks for a final newline and thatprocess_changelog.pyaccepts the file, but it does not callvalidate_raw_change— so raw-byte-level whitespace rules are not enforced at PR time. The byte-level check is a deliberate merge-time-only concern.Why merge-time fails
utils/merge_with_reuse.sh(the documented reuse-assisted merge path — see.github/workflows/README.md:185andcodeowner-signoff-verify.yml:281,371) invokesprepare_perf_changelog_merge.py canonicalizeat lines 181-186. That in turn callscanonicalize_appended_links→validate_raw_change(prepare_perf_changelog_merge.py:97). Invalidate_raw_change(utils/validate_perf_changelog.py:211-235), becausebase_rawends withpull/2001\n(single newline, not\n\n),expected_startis set tob"\n- config-keys:". The checksuffix.startswith(expected_start)then fails and raisesChangelogValidationError('new changelog entries must be separated from history by one empty line and appended at the end').merge_with_reuse.shruns underset -euo pipefailwith no error handler on this call, so the non-zero exit aborts the merge.Step-by-step proof
base_raw(fromorigin/main/1f234a6) ends with the bytes...pull/2001\n— verified viaod. Base does NOT end with\n\n.head_rawat that offset continues with the bytes\n- config-keys:\n - minimaxm3-fp4-b300-vllm\n...— the(two spaces) come from the trailing whitespace on line 4436.validate_raw_change, sincebase_raw.endswith(b'\n\n')is False,expected_start = b'\n- config-keys:'.suffix = head_raw[len(base_raw):]— this suffix starts withb' \n- config-keys:'.suffix.startswith(b'\n- config-keys:')→False(suffix starts with two spaces, not a newline).ChangelogValidationErrorwith the message quoted above.prepare_perf_changelog_merge.pyreturns exit code 1; underset -euo pipefailmerge_with_reuse.shaborts before push.Empirically reproduced by running
canonicalize_appended_links(base_raw, head_raw, 1994, 'SemiAnalysisAI/InferenceX')on the current bytes — it raises the exact error.Impact
Blocks the standard supported merge path. This is distinct from the earlier inline comment on this PR (which flagged the missing
1994PR number and missing trailing newline — both since fixed on head); the trailing-space separator remains.Fix
Delete the two trailing spaces on line 4436 so the separator is a truly-empty line (
\nonly).