-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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. |
Add Tariser model support: #9707
FIX #9707