-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Add tarsier model support #18985
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
Add tarsier model support #18985
Conversation
Signed-off-by: 汪志鹏 <wangzhipeng628@gmail.com>
👋 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: 汪志鹏 <wangzhipeng628@gmail.com>
Signed-off-by: 汪志鹏 <wangzhipeng628@gmail.com>
PTAL at the failing tests |
Signed-off-by: 汪志鹏 <wangzhipeng628@gmail.com>
@DarkLight1337 It seems that some other model has an error. |
cc @Isotr0py do you have time to help validate this model? |
Here is my simple test code, you can refer it |
According to the model outputs, model implementation should be fine. |
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.
Can you also update the document's supported_models
and vllm/entrypoints/chat_utils.py
?
vllm/vllm/entrypoints/chat_utils.py
Lines 503 to 506 in b9f61e1
def _placeholder_str(self, modality: ModalityStr, | |
current_count: int) -> Optional[str]: | |
# TODO: Let user specify how to insert image tokens into prompt | |
# (similar to chat template) |
… entrypoints placeholder Signed-off-by: 汪志鹏 <wangzhipeng628@gmail.com>
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.
Processor tests passed on my side locally as well. So this PR should be good to go!
@Isotr0py Sorry to bother you to review it again. This morning I noticed all 75 checks have passed but a button "update branch" occurred. I directly clicked the button and the whole process restarted again. Please forgive me for being a rookie. Do I not need to click the update branch button after all checks are passed? |
Hmmm, I remember we have disabled this button before... No need to update, I think this limitation would be disabled again, and we can merge this directly then. |
@DarkLight1337 Can you merge it, thank you. |
Does this support https://huggingface.co/omni-research/Tarsier2-Recap-7b as well ? |
No, because they have the different architecture, I read their paper, the tarsier2 is fine tune from qwen2-vl, have you tried use qwen2-vl to load tarsier2? If can't, I can try to support the tarsier2 model |
you can pass |
@patil-suraj can you use Qwen2VLForConditionalGeneration run the Tarsier2-Recap-7b? I got an error:
|
@princepride Thank you! from transformers import Qwen2VLForConditionalGenerationmodel
model = Qwen2VLForConditionalGeneration.from_pretrained("./Tarsier2-Recap-7b/") The error you have seems to be coming from the processor which probably has different/missing keys. |
@princepride Seems tarsier2 is using a different processor instead of qwen2-vl processor: https://github.com/bytedance/tarsier/blob/4581ecf213596f50c9b38fff753facf07f198b94/dataset/tarsier_processor.py#L24-L29 Perhaps we need to register a different multimodal processor for it just like Mantis. |
That's correct, the inference code actually uses this |
👌, I will handle it |
@Isotr0py The Tarsier2 model's config file specifies the model_type as "llava". This causes vllm_config.model_config.hf_config to be automatically created as a LlavaConfig object. How should I resolve this? |
You should also override it using |
It seems not work, the model still auto create a LlavaConfig object, can I manual mapping the config value and create a Qwen2VLConfig? |
What do you mean by manual mapping? |
Sorry, the problem solved |
Add Tariser model support: #9707
FIX #9707