Skip to content

[BugFix] Fix minimax m2 model partial_rotary_factor#30486

Closed
rogeryoungh wants to merge 2 commits intovllm-project:mainfrom
rogeryoungh:m2-fix-rotary-again
Closed

[BugFix] Fix minimax m2 model partial_rotary_factor#30486
rogeryoungh wants to merge 2 commits intovllm-project:mainfrom
rogeryoungh:m2-fix-rotary-again

Conversation

@rogeryoungh
Copy link
Contributor

@rogeryoungh rogeryoungh commented Dec 11, 2025

Purpose

In #30384, I fixed an inference issue with MiniMax-M2, but this caused some community-contributed quantized models to fail(#30445), some of which were forked before I added partial_rotary_factor. This update provides compatibility with those community-contributed quantized models.

Test Plan

lm_eval --model local-completions \
  --model_args base_url=http://localhost:10086/v1/completions,tokenizer=/model,model=/model \
  --tasks gsm8k_cot  \
  --batch_size 128 \
  --num_fewshot 5

Test Result

local-completions (base_url=http://localhost:10086/v1/completions,tokenizer=/model,model=/model), gen_kwargs: (None), limit: None, num_fewshot: 5, batch_size: 128
|  Tasks  |Version|     Filter     |n-shot|  Metric   |   |Value |   |Stderr|
|---------|------:|----------------|-----:|-----------|---|-----:|---|-----:|
|gsm8k_cot|      3|flexible-extract|     5|exact_match||0.9227|±  |0.0074|
|         |       |strict-match    |     5|exact_match||0.9105|±  |0.0079|

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@rogeryoungh rogeryoungh changed the title update: fixed the issue with rotary_dim when `partial_rotary_factor… [BugFix] Fix minimax m2 model partial_rotary_factor Dec 11, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to fix a compatibility issue with older quantized models by conditionally setting the rotary_dim. While the overall logic seems correct, I've identified a critical issue in the implementation that could lead to an UnboundLocalError at runtime. My review includes a suggested fix for this issue.

…` is not set

Signed-off-by: xuebi <xuebi@minimaxi.com>
Signed-off-by: xuebi <xuebi@minimaxi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant