[NPU][Bugfix] fix Qwen3-VL-30B-A3B-Instruct accuracy loss#15597
[NPU][Bugfix] fix Qwen3-VL-30B-A3B-Instruct accuracy loss#15597iforgetmyname merged 9 commits intosgl-project:mainfrom
Conversation
Summary of ChangesHello @cen121212, 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 Work In Progress pull request aims to resolve an observed accuracy loss when running the Qwen3-VL-30B-A3B-Instruct model on NPU hardware. The changes primarily involve refining how expert weights are normalized within the Mixture of Experts layer and strategically disabling NPU-specific code paths for Rotary Embedding and the model's forward preparation, opting for more generic or native implementations to mitigate the accuracy degradation. Highlights
🧠 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. Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Code Review
The pull request modifies NPU-related code paths across three files. In topk.py, a custom l1_norm for topk_weights was replaced with a standard PyTorch sum-based normalization, but the reviewer noted potential numerical instability, the removal of specific expert handling logic, and requested the removal of commented-out code. Additionally, NPU-specific code paths were commented out in rotary_embedding.py and effectively bypassed in qwen3_moe.py, prompting the reviewer to ask for clarification on whether these changes are temporary and to ensure proper restoration or removal with explanation to prevent maintenance issues.
| topk_weights = topk_weights / topk_weights.sum(dim=-1, keepdim=True) | ||
| # topk_weights = l1_norm( | ||
| # topk_weights | ||
| # if topk_config.num_fused_shared_experts == 0 | ||
| # else topk_weights[:, :-1] | ||
| # ) |
There was a problem hiding this comment.
This change replaces the custom l1_norm with a standard PyTorch implementation, which is good for clarity. However, there are a few points to consider:
- Numerical Stability: The division could be unstable if
topk_weights.sum()is zero. Adding a small epsilon to the denominator would make it more robust. - Logic Change: The original code had special handling for
topk_config.num_fused_shared_experts, excluding the last expert from normalization. This logic is now removed. If this is intentional, a comment explaining the reasoning would be helpful. If not, this could be a regression. - Code Cleanliness: The commented-out old code should be removed before merging.
Here is a suggestion addressing points 1 and 3:
| topk_weights = topk_weights / topk_weights.sum(dim=-1, keepdim=True) | |
| # topk_weights = l1_norm( | |
| # topk_weights | |
| # if topk_config.num_fused_shared_experts == 0 | |
| # else topk_weights[:, :-1] | |
| # ) | |
| topk_weights = topk_weights / (topk_weights.sum(dim=-1, keepdim=True) + 1e-9) |
| # elif _is_npu: | ||
| # return self._forward_npu(positions, query, key) |
There was a problem hiding this comment.
This NPU-specific code path has been commented out. If this is a temporary change for debugging, please ensure it's either restored or removed with a proper explanation before the final merge. Leaving commented-out code can lead to confusion and maintenance issues.
fcec655 to
bd6e4db
Compare
bd6e4db to
82ee493
Compare
|
/tag-and-rerun-ci |
…glang into eagle-sche * 'ifmn/eagle-dp-attn' of https://github.com/sgl-project/sglang: (22 commits) dp scheduler enhance support with chunked prefill (sgl-project#16071) modify suffix decoding CI dependency update (sgl-project#16063) fix rotary_embedding init npu (sgl-project#16011) feat: bugfix and accuracy fix for stablelm2_1_6b (sgl-project#15932) Update model and feature support for Ascend NPU (sgl-project#16005) Bugfix for Llama4 (sgl-project#15929) Bugfix for ds-vl2 (sgl-project#15894) gme qwen vl runners fix (sgl-project#15899) add profiling in scheduler (sgl-project#15876) llama use triton rope op (sgl-project#15855) suffix decoding adapt npu suffix decoding adapt npu Add suffix decoding speculative algorithm from feature 13553 cherry sgl-project#15434: qwen3 vl performance update cherry sgl-project#15597: fix Qwen3-VL-30B-A3B-Instruct accuracy loss [Schedule] bug fix for schedule enhancer (sgl-project#15834) minilb support roundrobin (sgl-project#15824) fix torchair compile issue cherry sgl-project#15187: lora fix ... # Conflicts: # python/sglang/srt/managers/scheduler.py # python/sglang/srt/managers/scheduler_enhancer.py
Motivation
Add the processing for mRoPE for Qwen VL
Modifications
Add the processing for mRoPE in get_cos_sin_with_position
Accuracy Tests
test command:
Before this PR:
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Model | Dataset | Metric | Subset | Num | Score | Cat.0 |
+===========================+===========+==========+=========================+=======+=========+=========+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | coarse perception | 250 | 0.648 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | fine-grained perception | 250 | 0.504 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | instance reasoning | 250 | 0.676 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | logical reasoning | 250 | 0.624 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | math | 250 | 0.748 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | science & technology | 250 | 0.388 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | OVERALL | 1500 | 0.598 | - |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
After this PR:
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Model | Dataset | Metric | Subset | Num | Score | Cat.0 |
+===========================+===========+==========+=========================+=======+=========+=========+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | coarse perception | 250 | 0.668 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | fine-grained perception | 250 | 0.544 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | instance reasoning | 250 | 0.744 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | logical reasoning | 250 | 0.748 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | math | 250 | 0.816 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | science & technology | 250 | 0.508 | default |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
| Qwen3-VL-30B-A3B-Instruct | mm_star | mean_acc | OVERALL | 1500 | 0.6713 | - |
+---------------------------+-----------+----------+-------------------------+-------+---------+---------+
Benchmarking and Profiling
Checklist