Skip to content

⬆️ bump vllm upper bound to support 0.10.2 #463

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
Merged
maxdebayser merged 24 commits into from
Sep 24, 2025
Merged

⬆️ bump vllm upper bound to support 0.10.2 #463

maxdebayser merged 24 commits into from
Sep 24, 2025

Conversation

@prashantgupta24
Copy link
Collaborator

@prashantgupta24 prashantgupta24 commented Sep 15, 2025 •
edited
Loading

Description

Bump upper bound of vllm to 0.10.2

Related Issues

@github-actions
Copy link

github-actions bot commented Sep 15, 2025

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes. This can be done with uv directly:

uv sync --frozen --group lint --active --inexact

Or this can be done with pip:

uv pip compile --group lint > requirements-lint.txt
pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

@prashantgupta24 prashantgupta24 added the ready Runs the full CI test suite. Only add to PRs once ready to merge to limit public GHA usage label Sep 15, 2025
@prashantgupta24
⬆️ bump aftu to 0.2.3
d043a22 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24 prashantgupta24 changed the title ⬆️ make vllm >=0.10.1.1,<=0.10.2 ⬆️ bump vllm lower bound and support 0.10.2 Sep 15, 2025
prashantgupta24
prashantgupta24 commented Sep 16, 2025
View reviewed changes
tests/e2e/test_spyre_cb.py

with pytest.raises(BadRequestError,
match="This model's maximum context length is"):
with pytest.raises(BadRequestError, match="maximum context length is"):
Copy link
Collaborator Author

@prashantgupta24 prashantgupta24 Sep 16, 2025

Choose a reason for hiding this comment

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

This is a bug in vllm upstream - opened a PR vllm-project/vllm#24995

@prashantgupta24
🚧 test 0.10.2 instead of main
b00b4e5 
Signed-off-by: Prashant Gupta <[email protected]>
prashantgupta24
prashantgupta24 commented Sep 16, 2025
View reviewed changes
prashantgupta24 and others added 6 commits September 17, 2025 11:21
@prashantgupta24
🐛 fix pooler stuff
db5aea7 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24
⏪ revert change for main
703d59d 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24
⬆️ bump lowest to 0.10.1.1
0de9b1c 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24
⬆️ bump default to 0.10.2
6c03694 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24 @joerunde
♻️ make platform.py check simple
5b2f32c 
Co-authored-by: Joe Runde <[email protected]>
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24
🐛 set vllm_config for ClassifierPooler
1c22ec7 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24 prashantgupta24 changed the title ⬆️ bump vllm lower bound and support 0.10.2 ⬆️ bump vllm upper bound and support 0.10.2 Sep 17, 2025
@prashantgupta24 prashantgupta24 changed the title ⬆️ bump vllm upper bound and support 0.10.2 ⬆️ bump vllm upper bound to support 0.10.2 Sep 17, 2025
joerunde
joerunde reviewed Sep 18, 2025
View reviewed changes
joerunde
joerunde reviewed Sep 18, 2025
View reviewed changes
maxdebayser
maxdebayser reviewed Sep 18, 2025
View reviewed changes
Copy link
Collaborator

@maxdebayser maxdebayser left a comment

Choose a reason for hiding this comment

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

The pooling correction works. So now we only need backwards compatibility tests for the cursor stuff.

@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 18, 2025 •
edited
Loading

The pooling correction works. So now we only need backwards compatibility tests for the cursor stuff.

Oh awesome! Do we want to review and get #468 in first so that we're not stuck in a state where we have to be backward compatible with 0.10.2 (which is going to be much harder vs 0.10.1.1)

Edit: merged!

@prashantgupta24
🎨 fix comment
f93eac5 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24
🎨 remove extra assert
25ba89a 
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24 prashantgupta24 marked this pull request as ready for review September 22, 2025 21:02
@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 22, 2025 •
edited
Loading

Hold on, scoring is still failing on 0.10.2 :(

Nope, all pass! Ready for review!

@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 22, 2025

bot:test

1 similar comment
@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 22, 2025

bot:test

@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 22, 2025

Some CI tests failing, looking...

@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 23, 2025

bot:test

@prashantgupta24
Copy link
Collaborator Author

prashantgupta24 commented Sep 23, 2025

CI tests all pass! Time for a review

rafvasq
rafvasq reviewed Sep 23, 2025
View reviewed changes
Copy link
Collaborator

@rafvasq rafvasq left a comment

Choose a reason for hiding this comment

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

just a typo

@prashantgupta24
🎨 typo
4f1a6a2 
Signed-off-by: Prashant Gupta <[email protected]>
prashantgupta24
prashantgupta24 commented Sep 24, 2025
View reviewed changes
vllm_spyre/v1/worker/spyre_model_runner.py

if task == "embed":
self.pooler = Pooler.for_embed(pooler_config=pooler_config)
with set_current_vllm_config(self.vllm_config):
Copy link
Collaborator Author

@prashantgupta24 prashantgupta24 Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

we need to set_current_vllm_config because now Pooler class needs the vllm config for it to read vllm_config.model_config.head_dtype

@maxdebayser
Copy link
Collaborator

maxdebayser commented Sep 24, 2025

bot:test

@maxdebayser
Copy link
Collaborator

maxdebayser commented Sep 24, 2025

Merging as all non-main tests are passing

@maxdebayser maxdebayser merged commit 5aa87d5 into main Sep 24, 2025
19 of 27 checks passed
@maxdebayser maxdebayser deleted the upstream-versions branch September 24, 2025 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxdebayser maxdebayser maxdebayser left review comments

@joerunde joerunde joerunde left review comments

@rafvasq rafvasq rafvasq left review comments

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

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

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

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

Assignees

No one assigned

Labels

ready Runs the full CI test suite. Only add to PRs once ready to merge to limit public GHA usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@prashantgupta24 @maxdebayser @joerunde @rafvasq