-
- Notifications
You must be signed in to change notification settings - Fork 11.2k
[Model] Support tensor parallel for timm ViT in Deepseek_vl2 #21494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 introduces tensor parallelism support for the timm Vision Transformer's MLP blocks within the deepseek_vl2 model. The implementation correctly monkey-patches timm.layers.Mlp to substitute nn.Linear with vLLM's ColumnParallelLinear and RowParallelLinear. A wrapper class is also added to adapt the output format of these parallel layers for compatibility with the timm library.
The overall approach is solid and directly addresses the goal of improving performance through tensor parallelism. However, I have identified a critical issue in the exception handling logic that could lead to a runtime error, as well as a high-severity issue in the implementation of the wrapper class that could introduce subtle bugs. Addressing these points will significantly improve the robustness and correctness of the code.
| 👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
Signed-off-by: wzqd <1057337859@qq.com>
Signed-off-by: wzqd <1057337859@qq.com>
Signed-off-by: wzqd <1057337859@qq.com>
Isotr0py left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now!
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com> Signed-off-by: x22x22 <wadeking@qq.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
…oject#21494) Signed-off-by: wzqd <1057337859@qq.com>
Support tensor parallel for timm ViT
Purpose
The vision transformer from timm library does not support tensor parallel and has bad performance. Currently minicpmv2_0 and deepseek_vl2 are the only two models using timm ViT, but minicpmv2_6 has changed to idefics ViT. The purpose of this pr is to enhance performance of the ViT in deepseekvl2 without changing the model structure.