-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[Model] Adding Support for Qwen2VL as an Embedding Model. Using MrLight/dse-qwen2-2b-mrl-v1 #9944
[Model] Adding Support for Qwen2VL as an Embedding Model. Using MrLight/dse-qwen2-2b-mrl-v1 #9944
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 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 do one of these:
🚀 |
@DarkLight1337 Do you want any changes to the vision embedding API test? It looks like generally those tests are more about the endpoint than if the model they're calling is returning the correct output. |
…1 as an embedding model. Signed-off-by: FurtherAI <[email protected]>
Signed-off-by: FurtherAI <[email protected]>
It looks quite complicated. Can you factor out the common code so it is more like the other embedding model tests? |
Co-authored-by: Cyrus Leung <[email protected]>
Do you mean the offline test looks complicated? I think the question still stands about if you want anything added to the API test. |
I was referring to the offline API tests. I don't see any changes to the online API tests though (which are under |
This pull request has merge conflicts that must be resolved before it can be |
Those are the API tests I was referring to. I can improve the code for the test I think. I'll test it next week. |
There is no need to update online API tests for new models. |
Signed-off-by: austin.veselka <[email protected]>
Signed-off-by: austin.veselka <[email protected]>
PTAL at the failing model tests. Can we omit using |
Yeah I can probably drop that. I think it is only resizing the image |
Signed-off-by: austin.veselka <[email protected]>
Please also merge from main to fix the broken CI. |
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.
The tests have passed, thanks for your patience!
…ht/dse-qwen2-2b-mrl-v1 (vllm-project#9944) Signed-off-by: FurtherAI <[email protected]> Co-authored-by: FurtherAI <[email protected]>
…ht/dse-qwen2-2b-mrl-v1 (vllm-project#9944) Signed-off-by: FurtherAI <[email protected]> Co-authored-by: FurtherAI <[email protected]> Signed-off-by: OmerD <[email protected]>
…ht/dse-qwen2-2b-mrl-v1 (vllm-project#9944) Signed-off-by: FurtherAI <[email protected]> Co-authored-by: FurtherAI <[email protected]> Signed-off-by: Sumit Dubey <[email protected]>
[Model] Adding Support for Qwen2VL as an Embedding Model. Using MrLight/dse-qwen2-2b-mrl-v1
This is related to #9303 and #9759 and adds support for Qwen2VL as an embedding model. This is only a couple of lines of changes and a jinja template for the API.
I have tested that this is working correctly with MrLight/dse-qwen2-2b-mrl-v1. The test file is a bit ugly though. I don't think the
hf_runner
supports the input processing that needs to be done for this model because it usesprocess_vision_info
and applies a chat template manually. It also takes a minimum size image placeholder for text embedding.Looking for a little help with how the test should be written to cleanly handle these differences.
I have tested the API, but didn't look into how to add it to
test_vision_embedding.py
yet.cc @DarkLight1337