Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Model] Add support for embedding model GritLM #10816

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

[Model] Add support for embedding model GritLM #10816

wants to merge 21 commits into from
+451 −0

Conversation

pooyadavoodi
Copy link
Contributor

This model is a fork of GritLM/GritLM-7B. The main change in the fork wrt the original repo is the name of the architecture to make vLLM adoption easier.

@pooyadavoodi
Add support for embedding model parasail-ai/GritLM-7B-vllm
1d60ff6 
This model is a fork of GritLM/GritLM-7B. The main change in the fork
wrt the original repo is the name of the architecture to make vLLM
adoption easier.

Signed-off-by: Pooya Davoodi <[email protected]>
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 2, 2024

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@mergify mergify bot added the documentation Improvements or additions to documentation label Dec 2, 2024
DarkLight1337
DarkLight1337 reviewed Dec 2, 2024
View reviewed changes
vllm/inputs/data.py Outdated Show resolved Hide resolved
zifeitong
zifeitong reviewed Dec 6, 2024
View reviewed changes
vllm/model_executor/models/gritlm.py Outdated Show resolved Hide resolved
vllm/model_executor/models/gritlm.py Outdated Show resolved Hide resolved
DarkLight1337
DarkLight1337 reviewed Dec 8, 2024
View reviewed changes
Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM!

@pooyadavoodi
Add PP and LORA support to docs
070483e 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Improve xformers check
e84c2a4 
In case of PP, some layers don't have self_attn attribute

Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Fix format
7a06527 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Merge branch 'main' into add_gritlm_vllm
2b5c5af 
Signed-off-by: Pooya Davoodi <[email protected]>
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) December 10, 2024 04:07
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 10, 2024
auto-merge was automatically disabled December 10, 2024 05:12

Head branch was pushed to by a user without write access

@pooyadavoodi
Copy link
Contributor Author

Looks like we have two failures in ToT that are unrelated to this PR:

@pooyadavoodi
Support generate task
dddf385 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Skip tests if xformers is not available
f7fdd77 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Add GritLM to _TEXT_GENERATION_MODELS
8d2c4d3 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Reduce context length in tests
9579e37 
And make test prompt shorter

Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Fix format
9a5cb50 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Copy link
Contributor Author

I ended up adding generation support because some model tests expect it due to GritLM using LLamaForCausalLM.

@pooyadavoodi
Relax xformers req for generation
9c9deea 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Move import below skip
a273596 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Fix format
703c09c 
Signed-off-by: Pooya Davoodi <[email protected]>
@pooyadavoodi
Add gritlm to generation doc
6666445 
Signed-off-by: Pooya Davoodi <[email protected]>
@DarkLight1337
Copy link
Member

You might have to merge from main to fix the CI failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zifeitong zifeitong zifeitong left review comments

@DarkLight1337 DarkLight1337 DarkLight1337 approved these changes

@ywang96 ywang96 Awaiting requested review from ywang96 ywang96 is a code owner

@zhuohan123 zhuohan123 Awaiting requested review from zhuohan123

@youkaichao youkaichao Awaiting requested review from youkaichao

@alexm-neuralmagic alexm-neuralmagic Awaiting requested review from alexm-neuralmagic

@comaniac comaniac Awaiting requested review from comaniac

@njhill njhill Awaiting requested review from njhill

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pooyadavoodi @DarkLight1337 @zifeitong