Skip to content

[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable#20759

Merged
HaiShaw merged 2 commits intosgl-project:mainfrom
ZLkanyo009:dp-hang-bugfix
Mar 19, 2026
Merged

[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable#20759
HaiShaw merged 2 commits intosgl-project:mainfrom
ZLkanyo009:dp-hang-bugfix

Conversation

@ZLkanyo009
Copy link
Copy Markdown
Contributor

@ZLkanyo009 ZLkanyo009 commented Mar 17, 2026

Motivation

The Qwen3-VL type model hangs during forward when --mm-enable-dp-encoder is enabled. This is because the VocabParallelEmbedding part remains as TP8 after enabling --mm-enable-dp-encoder, when it should actually be TP1. Therefore, this PR is needed to fix the issue.

Accuracy Tests

before:
hang

after:
Evaluating...

answers saved to: ./answer_sglang.json
{'Accounting': {'acc': 0.467, 'num': 30},
 'Agriculture': {'acc': 0.6, 'num': 30},
 'Architecture_and_Engineering': {'acc': 0.267, 'num': 30},
 'Art': {'acc': 0.767, 'num': 30},
 'Art_Theory': {'acc': 0.933, 'num': 30},
 'Basic_Medical_Science': {'acc': 0.833, 'num': 30},
 'Biology': {'acc': 0.6, 'num': 30},
 'Chemistry': {'acc': 0.6, 'num': 30},
 'Clinical_Medicine': {'acc': 0.8, 'num': 30},
 'Computer_Science': {'acc': 0.567, 'num': 30},
 'Design': {'acc': 0.867, 'num': 30},
 'Diagnostics_and_Laboratory_Medicine': {'acc': 0.4, 'num': 30},
 'Economics': {'acc': 0.8, 'num': 30},
 'Electronics': {'acc': 0.3, 'num': 30},
 'Energy_and_Power': {'acc': 0.567, 'num': 30},
 'Finance': {'acc': 0.567, 'num': 30},
 'Geography': {'acc': 0.6, 'num': 30},
 'History': {'acc': 0.833, 'num': 30},
 'Literature': {'acc': 0.833, 'num': 30},
 'Manage': {'acc': 0.567, 'num': 30},
 'Marketing': {'acc': 0.6, 'num': 30},
 'Materials': {'acc': 0.4, 'num': 30},
 'Math': {'acc': 0.367, 'num': 30},
 'Mechanical_Engineering': {'acc': 0.3, 'num': 30},
 'Music': {'acc': 0.4, 'num': 30},
 'Overall': {'acc': 0.619, 'num': 900},
 'Overall-Art and Design': {'acc': 0.742, 'num': 120},
 'Overall-Business': {'acc': 0.6, 'num': 150},
 'Overall-Health and Medicine': {'acc': 0.733, 'num': 150},
 'Overall-Humanities and Social Science': {'acc': 0.767, 'num': 120},
 'Overall-Science': {'acc': 0.573, 'num': 150},
 'Overall-Tech and Engineering': {'acc': 0.429, 'num': 210},
 'Pharmacy': {'acc': 0.9, 'num': 30},
 'Physics': {'acc': 0.7, 'num': 30},
 'Psychology': {'acc': 0.7, 'num': 30},
 'Public_Health': {'acc': 0.733, 'num': 30},
 'Sociology': {'acc': 0.7, 'num': 30}}
eval out saved to ./val_sglang.json
Overall accuracy: 0.619

@gemini-code-assist
Copy link
Copy Markdown
Contributor

gemini-code-assist Bot commented Mar 17, 2026

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a significant operational issue in the Qwen3-VL model, preventing it from hanging when specific distributed encoding features are activated. By correctly configuring tensor parallelism for a key embedding layer, the change ensures the model functions as expected in distributed environments, improving stability and reliability.

Highlights

  • Qwen3-VL Model Hang Fix: Addressed a critical bug where the Qwen3-VL model would hang during its forward pass when the --mm-enable-dp-encoder flag was enabled. This issue stemmed from the VocabParallelEmbedding component incorrectly maintaining a TP8 configuration instead of switching to TP1 under data parallel encoding.
  • Tensor Parallelism Configuration: Implemented a fix by explicitly setting the enable_tp parameter for VocabParallelEmbedding to False when use_data_parallel is active, ensuring correct tensor parallelism behavior and preventing the hang.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • python/sglang/srt/models/qwen3_vl.py
    • Modified the __init__ method of the Qwen3VLLM class to pass the enable_tp argument to the VocabParallelEmbedding constructor, conditionally disabling tensor parallelism based on the use_data_parallel flag.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Mar 17, 2026
View reviewed changes
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 hang issue in the Qwen3-VL model when --mm-enable-dp-encoder is enabled. The fix correctly disables tensor parallelism for the VocabParallelEmbedding of position embeddings in this scenario.

I've identified a potential issue where an assertion error could occur if both --mm-enable-dp-encoder and --enable-dp-attention are enabled simultaneously. I've provided a suggestion to prevent this by ensuring use_attn_tp_group is also disabled when data parallelism is used for the vision encoder.

Comment thread python/sglang/srt/models/qwen3_vl.py Outdated
@zhuyuhua-v zhuyuhua-v mentioned this pull request Mar 18, 2026
Open
@HaiShaw
Copy link
Copy Markdown
Collaborator

HaiShaw commented Mar 18, 2026

/tag-and-rerun-ci

@HaiShaw
Copy link
Copy Markdown
Collaborator

HaiShaw commented Mar 18, 2026

@Lzhang-hub @mickqian Please have a review.

@HaiShaw
Copy link
Copy Markdown
Collaborator

HaiShaw commented Mar 18, 2026

@ZLkanyo009
With a merged fix from #20788, do we still need this PR?
Please check, and solve code conflict if to update.

@ZLkanyo009
Copy link
Copy Markdown
Contributor Author

ZLkanyo009 commented Mar 19, 2026 •
edited
Loading

I think we can make the final change: is_dp_attention_enabled() and not use_data_parallel, otherwise the two options dp_attention and mm_dp_encoder might conflict.

ZLkanyo009 and others added 2 commits March 19, 2026 02:27
@ZLkanyo009
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable
6b2e79e
@ZLkanyo009 @gemini-code-assist
Update python/sglang/srt/models/qwen3_vl.py
ff8de6a 
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@HaiShaw HaiShaw merged commit f85455a into sgl-project:main Mar 19, 2026
66 of 103 checks passed
Wangzheee pushed a commit to Wangzheee/sglang that referenced this pull request Mar 21, 2026
@ZLkanyo009 @Wangzheee
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable (sgl-…
c8c98c4 
…project#20759)
0-693 pushed a commit to 0-693/sglang that referenced this pull request Mar 25, 2026
@ZLkanyo009 @0-693
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable (sgl-…
8af871e 
…project#20759)
dutsc pushed a commit to dutsc/sglang that referenced this pull request Mar 30, 2026
@ZLkanyo009
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable (sgl-…
1ad89be 
…project#20759)
JustinTong0323 pushed a commit to JustinTong0323/sglang that referenced this pull request Apr 7, 2026
@ZLkanyo009
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable (sgl-…
73c78f3 
…project#20759)
yhyang201 pushed a commit to yhyang201/sglang that referenced this pull request Apr 22, 2026
@ZLkanyo009
[Bugfix] fix qwen3vl hang when --mm-enable-dp-encoder is enable (sgl-…
dcb3bd8 
…project#20759)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mickqian mickqian mickqian approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

run-ci

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ZLkanyo009 @HaiShaw @mickqian