fix mistral3 vllm backend ignore consolidated.safetensors#7074
fix mistral3 vllm backend ignore consolidated.safetensors#7074hjh0119 merged 2 commits intomodelscope:mainfrom
Conversation
Summary of ChangesHello @hjh0119, 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 critical loading issue for Mistral 3 models when utilizing the vLLM inference backend. Previously, the backend would overlook essential Highlights
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 an ignore_patterns parameter to the mistral_2512 and mistral_2512_thinking model registrations to address an issue with the vLLM backend. My review points out a potential discrepancy between the pull request's title, which suggests ignoring consolidated.safetensors, and the code change, which sets an empty ignore list. I've provided suggestions to correct this potential issue.
| model_arch=ModelArch.llava_hf, | ||
| requires=['transformers>=5.0.0.dev0', 'mistral-common>=1.8.6'], | ||
| tags=['vision'], | ||
| ignore_patterns=[], |
There was a problem hiding this comment.
The pull request title suggests that consolidated.safetensors should be ignored. However, setting ignore_patterns=[] will result in no files being ignored, which seems to contradict the stated goal. If the intention is to ignore consolidated.safetensors, this should be ignore_patterns=['*consolidated.safetensors'] to match files with that name.
| ignore_patterns=[], | |
| ignore_patterns=['*consolidated.safetensors'], |
| model_arch=ModelArch.llava_hf, | ||
| requires=['transformers>=5.0.0.dev0', 'mistral-common>=1.8.6'], | ||
| tags=['vision'], | ||
| ignore_patterns=[], |
There was a problem hiding this comment.
Similar to the comment above, this change seems to contradict the intent described in the pull request title. If consolidated.safetensors should be ignored, this should be ignore_patterns=['*consolidated.safetensors'].
| ignore_patterns=[], | |
| ignore_patterns=['*consolidated.safetensors'], |
test with vllm 0.12.0