Repository navigation
Rename only MiniMax-M3-VL's MoE block, so a save keeps the dense and vision mlp keys - #49786
Merged
Merged
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
from
October 20, 2026 16:04
daad369 to
5dd6ef5
Compare
vasqu
reviewed
Oct 20, 2026
| WeightRenaming(source_patterns=r"\.block_sparse_moe\.", target_patterns=r"\.mlp\."), | ||
| # Only the MoE block. Otherwise a save would also rename the dense and vision `.mlp.` keys. | ||
| WeightRenaming( | ||
| source_patterns=r"\.block_sparse_moe\.(experts\.|gate\.|shared_experts\.|e_score_correction_bias$)", |
Member
Author
There was a problem hiding this comment.
no, no other key under the moe block starts with e_score_correction_bias, so it matches without it. dropped it
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
from
October 20, 2026 17:34
5dd6ef5 to
b506cb2
Compare
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
from
October 21, 2026 07:24
b506cb2 to
9c63025
Compare
IlyasMoutawwakil
marked this pull request as ready for review
October 21, 2026 07:37
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
from
October 21, 2026 07:53
ffef87f to
43f32c4
Compare
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
2 times, most recently
from
October 23, 2026 13:35
5a70522 to
19d9f5c
Compare
Draft
1 of 6 tasks
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: minimax_m3_vl |
Cyrilvallez
approved these changes
Oct 27, 2026
Cyrilvallez
left a comment
Cyrilvallez
left a comment
Member
There was a problem hiding this comment.
I updated the pattern since we need to repass over e_score_correction_bias anyway, that way the first pattern does not contain any \. litteral dots inside the capturing groups and it's a bit more readable than doing 2 patterns for the same e_score_correction_bias key.
I don't mind still merging the linked PR for litteral dots in capturing groups though, as it makes the system more robust in general even if not used here
…ision mlp keys The `.block_sparse_moe.` -> `.mlp.` rename reversed on save turned every `.mlp.` key back into `.block_sparse_moe.`, the dense and vision ones too. Name the MoE block's children in the pattern; the reversed group keeps its escaped dots. Run the reverse-mapping test that was skipped, on the composite model only, as for Llava. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No other key under the MoE block starts with `e_score_correction_bias`, so the alternative matches without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IlyasMoutawwakil
force-pushed
the
fg-split/fix-minimax-m3-vl-save-mapping
branch
from
October 27, 2026 10:38
2429e29 to
98a71e8
Compare
IlyasMoutawwakil
enabled stacked auto-merge
October 27, 2026 10:41
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 27, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What. The
.block_sparse_moe.to.mlp.rename, reversed on save, turned every.mlp.key into.block_sparse_moe., the dense and vision ones too. The pattern now names the MoE block's children. The reversemapping test, skipped until now, runs on the composite model, as for Llava.
Why we need it. MiniMax-M3 MXFP8 support (#49780) blocked on it. Its tiny-checkpoint test
(
IlyasMoutawwakil/tiny-random-MiniMaxM3-MXFP8) loads the checkpoint, saves it back and compares the keys. Thesave wrote the dense and vision MLP weights under MoE names, so the saved checkpoint no longer matched the original.
save_pretrainedon MiniMax-M3-VL does the same on main. The un-skipped reverse-mapping test catches it.Depends on. #49772 (the reversed group needs its escaped dots).
How tested.
tests/models/minimax_m3_vl -k "reverse_loading or save_load".The comments and docstrings that say why each change exists are there for readability. Feel free to ask for any of them to go if it's self-evident.
🤖 Generated with Claude Code