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

[LLM Batch][5/N] vLLM Engine Processor #50494

Merged
merged 28 commits into from
Feb 18, 2025
Merged

[LLM Batch][5/N] vLLM Engine Processor #50494

merged 28 commits into from
Feb 18, 2025

Conversation

comaniac
Copy link
Collaborator

Why are these changes needed?

  1. vLLM engine processor that supports
    1. vLLM v0 and v1.
    2. Text/Embedding/Vision models.
  2. Use enum for vLLM task type.
  3. Support images in chat template stage.
  4. Change kwargs in build_llm_processor to the Processor kwargs for better lookup.
  5. Set default values to the base stage.

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@comaniac comaniac requested review from a team as code owners February 12, 2025 19:34
@comaniac comaniac added actor-based-usecase Issues when running large scale actor-based usecases. go add ONLY when ready to merge, run all tests and removed actor-based-usecase Issues when running large scale actor-based usecases. labels Feb 12, 2025
GeneDer
GeneDer approved these changes Feb 12, 2025
View reviewed changes
Copy link
Contributor

@GeneDer GeneDer left a comment

Choose a reason for hiding this comment

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

LGTM!

@comaniac
work
6335485 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
work
669ed17 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
test
cb097ab 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
wip
f14e06e 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
done
2c81a36 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
gpu
03a2064 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
comments
7d84c97 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
dc239a4 
Signed-off-by: Cody Yu <[email protected]>
kouroshHakha
kouroshHakha reviewed Feb 14, 2025
View reviewed changes
python/ray/llm/_internal/batch/stages/chat_template_stage.py Outdated
Comment on lines 48 to 53
messages = [
row["messages"].tolist()
if hasattr(row["messages"], "tolist")
else row["messages"]
for row in batch
]
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this more readable? It's too much inline syntax.

Why can't we always do messages=[row["messages"] for row in batch]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because when you have images in the messages, row["messages"] will already be a list and doesn't have tolist()

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah then messages=[row["messages"] for row in batch] should work for both messages with image and without

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But it doesn't work. That's why we have this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

:) ok ok. I'll run stuff later when this is merged to see if I have a better way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The root case is when messages include images, PyArrow cannot serialize it so Ray Data uses pickle. In this case the messages is deserialized directly to the original data type (list of dicts).

@comaniac
fix
e91fbe9 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
546e630 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
36b8c6f 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
test
573fb39 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
Merge branch 'master' into llm-batch-proc
9bfbf49 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
add conftest.py
48aec6a 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
Merge branch 'master' into llm-batch-proc
e2e8b9e 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
try
e2d07b9 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
try
ba04eda 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
try
28164df 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
test
1ac586e 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
test
898a53c 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
test
1518b15 
Signed-off-by: Cody Yu <[email protected]>
@comaniac comaniac requested a review from a team as a code owner February 17, 2025 20:40
@comaniac
test
93218da 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
6e6be97 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
fbce2bc 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
fix
582c0e0 
Signed-off-by: Cody Yu <[email protected]>
This was referenced Feb 18, 2025
Open
Merged
aslonnie
aslonnie reviewed Feb 18, 2025
View reviewed changes
ci/docker/llm.build.Dockerfile Outdated
@@ -18,4 +18,8 @@ SKIP_PYTHON_PACKAGES=1 ./ci/env/install-dependencies.sh

pip install --no-deps -r python/requirements_compiled_rayllm_test_py311.txt

# FIXME(comaniac)
pip uninstall -y torch
pip install torch==2.5.1 --index-url https://download.pytorch.org/whl/cu121
Copy link
Collaborator

Choose a reason for hiding this comment

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

merge master?

@comaniac
fix
a9b0b9f 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
Merge branch 'master' into llm-batch-proc
c216cf8 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
revert
50fe6a7 
Signed-off-by: Cody Yu <[email protected]>
@comaniac
Copy link
Collaborator Author

Should be able to merge once the CI is green. @richardliaw @kouroshHakha

@richardliaw richardliaw enabled auto-merge (squash) February 18, 2025 02:10
@richardliaw richardliaw merged commit 5ab0e70 into ray-project:master Feb 18, 2025
6 checks passed
@comaniac comaniac deleted the llm-batch-proc branch February 18, 2025 16:40
richardliaw added a commit that referenced this pull request Feb 19, 2025
@richardliaw @comaniac
[data/llm/docs] Initial draft of user guide for Data LLM APIs (#50674)
6aa079a 
## Why are these changes needed?

Adds user guide and link-ins for Ray Data documentation.

This is part of the #50639 thread of work.

This is based on #50494 

cc @comaniac @gvspraveen @kouroshHakha  

## Related issue number

<!-- For example: "Closes #1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: Richard Liaw <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
400Ping pushed a commit to 400Ping/ray that referenced this pull request Feb 20, 2025
@comaniac @400Ping
[LLM Batch][5/N] vLLM Engine Processor (ray-project#50494)
6f4c397 
Signed-off-by: 400Ping <[email protected]>
400Ping pushed a commit to 400Ping/ray that referenced this pull request Feb 20, 2025
@richardliaw @comaniac @400Ping
[data/llm/docs] Initial draft of user guide for Data LLM APIs (ray-pr…
5ef0f18 
…oject#50674)

## Why are these changes needed?

Adds user guide and link-ins for Ray Data documentation.

This is part of the ray-project#50639 thread of work.

This is based on ray-project#50494

cc @comaniac @gvspraveen @kouroshHakha

## Related issue number

<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: Richard Liaw <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Signed-off-by: 400Ping <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kouroshHakha kouroshHakha kouroshHakha left review comments

@aslonnie aslonnie aslonnie left review comments

@richardliaw richardliaw richardliaw approved these changes

@GeneDer GeneDer GeneDer approved these changes

@gvspraveen gvspraveen Awaiting requested review from gvspraveen

Assignees
No one assigned
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@comaniac @richardliaw @GeneDer @kouroshHakha @aslonnie