[NPU]support model MiniCPM3-4B for npu#16866
[NPU]support model MiniCPM3-4B for npu#16866iforgetmyname merged 13 commits intosgl-project:mainfrom
Conversation
Summary of ChangesHello @McZyWu, 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 integrates the MiniCPM3-4B model with NPU hardware, significantly expanding the supported model ecosystem. The changes primarily involve adapting the attention mechanisms and rotary embedding calculations to leverage NPU capabilities, resulting in a verified 70% accuracy on the GSM8k benchmark. This enables efficient execution of the MiniCPM3-4B model on NPU devices. 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
This pull request adds support for the MiniCPM3-4B model on NPU hardware. The changes involve adding a new model configuration, a fallback attention implementation for NPU when the fused kernel is not available, and some minor refactoring in related components. While the core logic for adding model support is sound, I've identified a critical bug in the new attention implementation and a few areas for improvement in terms of code style and clarity. The most significant issue is in ascend_backend.py, where the value tensor passed to scaled_dot_product_attention is incorrect, which will lead to wrong attention outputs. I've provided a detailed comment and suggestion to fix this. Other comments focus on improving code readability and maintainability.
| k_kpe = torch.cat([k_, v_], dim=-1).transpose(0, 1) | ||
| o_ = ( | ||
| torch.nn.functional.scaled_dot_product_attention( | ||
| q_.unsqueeze(0), | ||
| k_kpe.unsqueeze(0), | ||
| v.unsqueeze(0), | ||
| enable_gqa=use_gqa, | ||
| scale=layer.scaling, | ||
| is_causal=True, | ||
| ) |
There was a problem hiding this comment.
There appears to be a bug in how the value tensor is passed to scaled_dot_product_attention. The v tensor, which represents the value for the entire batch, is being used directly without being sliced for the current sequence within the loop. This will result in incorrect attention calculations as the value tensor will have mismatched sequence lengths and will not correspond to the correct sequence's keys.
For MiniCPM-like models, the value in attention is the kv_c part. In your implementation, k_ represents the kv_c slice for the current sequence. Therefore, k_ should be used as the value tensor. It needs to be correctly shaped for the scaled_dot_product_attention function.
I've suggested a fix that uses k_ as the value tensor, ensuring its dimensions are correctly aligned for the attention calculation.
| k_kpe = torch.cat([k_, v_], dim=-1).transpose(0, 1) | |
| o_ = ( | |
| torch.nn.functional.scaled_dot_product_attention( | |
| q_.unsqueeze(0), | |
| k_kpe.unsqueeze(0), | |
| v.unsqueeze(0), | |
| enable_gqa=use_gqa, | |
| scale=layer.scaling, | |
| is_causal=True, | |
| ) | |
| k_kpe = torch.cat([k_, v_], dim=-1).transpose(0, 1) | |
| v_ = k_.transpose(0, 1) | |
| o_ = ( | |
| torch.nn.functional.scaled_dot_product_attention( | |
| q_.unsqueeze(0), | |
| k_kpe.unsqueeze(0), | |
| v_.unsqueeze(0), | |
| enable_gqa=use_gqa, | |
| scale=layer.scaling, | |
| is_causal=True, | |
| ) |
| assert ( | ||
| layer.qk_head_dim != layer.v_head_dim | ||
| ), "FIA only supports qk_head_dim != v_head_dim" | ||
| if not hasattr(self, "not_use_fused_infer_attention_score"): |
There was a problem hiding this comment.
what if not_use_fused_infer_attention_score=False ?
There was a problem hiding this comment.
it is set to be true.
f4a376d to
1019a2e
Compare
98c552f to
02dc5c8
Compare
|
/tag-and-rerun-ci |
Motivation
Previously, npu does not support model minicpm3-4B.
The MiniCPM3-4B model was not supported by the NPU operators in the current codebase, making it impossible to run the model on NPU hardware. Additionally, enabling Data Parallelism (DP) for attention layers triggered a division-by-zero exception (when the first dimension of the DP attention tensor was 0), further blocking functional deployment of the model.
Modifications
As follows.
The changes in ascend backend are to solve not-support and accuracy problems.
The changes to the files rotary_embedding.py and minicpm3.py ensure full compatibility with DP, to resolve the division-by-zero exception when DP attention is enabled, eliminating runtime errors and enabling distributed execution.
Accuracy Tests
70% Accuracy on GSM8k 5-shot evaluation
test method:
change the model path in test_ascend_minicpm3_4b.py
python test/registered/ascend/llm_models/test_ascend_minicpm3_4b.py
Supported features: TP DP
Unsupported features: Graph mode, prefix tuning...