Skip to content

fix: DeepSeek-V3.2 DeepGEMM RuntimeError#30251

Closed
KeeProMise wants to merge 3 commits intovllm-project:mainfrom
KeeProMise:30206
Closed

fix: DeepSeek-V3.2 DeepGEMM RuntimeError#30251
KeeProMise wants to merge 3 commits intovllm-project:mainfrom
KeeProMise:30206

Conversation

@KeeProMise
Copy link
Copy Markdown

@KeeProMise KeeProMise commented Dec 8, 2025

Purpose

see: #30206

Test Plan

Test Result


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.

@mergify mergify bot added the deepseek Related to DeepSeek models label Dec 8, 2025
Copy link
Copy Markdown
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 addresses a RuntimeError in DeepSeek-V3.2 DeepGEMM by introducing a crucial alignment check for the input K dimension. The changes correctly extend the should_use_deepgemm_for_fp8_linear function to include input_k_dim, ensuring that DeepGEMM is only utilized when the input tensor's K dimension is properly aligned. This is particularly important for tensor parallelism scenarios, where input_size_per_partition might not always meet DeepGEMM's alignment requirements. The addition of clear comments and an updated docstring enhances code clarity and maintainability. Overall, this is a well-implemented fix that improves the robustness and correctness of the DeepGEMM integration.

@mergify
Copy link
Copy Markdown

mergify bot commented Dec 8, 2025

Hi @KeeProMise, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Signed-off-by: JianZhang <keepromise@apache.org>
Signed-off-by: JianZhang <keepromise@apache.org>
Signed-off-by: JianZhang <keepromise@apache.org>
@robertgshaw2-redhat
Copy link
Copy Markdown
Collaborator

robertgshaw2-redhat commented Dec 8, 2025

Couple notes:

  • please add a clear PR description and before + after
  • shouldn't we be able to detect this at startup time rather than runtime if this is based on the hidden dim?
  • why is this just issue occuring now?

@coval3nte
Copy link
Copy Markdown
Contributor

why is this just issue occuring now?

official recipe advises TP=8.
I tried building vLLM against multiple commit in last few days and always got the same error.

Do you have a working version and the commit built against?

Copy link
Copy Markdown
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

Thanks for the work! Prefer this fix #30267

@KeeProMise
Copy link
Copy Markdown
Author

why is this just issue occuring now?

official recipe advises TP=8. I tried building vLLM against multiple commit in last few days and always got the same error.

Do you have a working version and the commit built against?

How about trying the latest version?

@KeeProMise KeeProMise closed this Dec 9, 2025
@IKACE
Copy link
Copy Markdown
Contributor

IKACE commented Dec 10, 2025

I applied the fix on vllm 0.12.0 but still got the same error, do you know if there are other ways to resolve this?

@yewentao256
Copy link
Copy Markdown
Member

I applied the fix on vllm 0.12.0 but still got the same error, do you know if there are other ways to resolve this?

Please use the main branch, there is another fix for this merged to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants