Skip to content

Conversation

@yannicks1
Copy link
Collaborator

[embedding] support newest vllm main branch

parameters default_normalize and default_softmax are deprecated

@github-actions
Copy link

github-actions bot commented Aug 6, 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 🚀

@yannicks1
Copy link
Collaborator Author

@maxdebayser do we need to include that in tests/utils/test_upstream_compatibility.py? We already have test_pooler_from_config(), so anyone replacing that will see that there is another if branch there...

@maxdebayser
Copy link
Collaborator

Yeah, I think it's better to add a test specifically for this, since the default_normalize refactoring is a discrete change in vllm.

@yannicks1 yannicks1 self-assigned this Aug 7, 2025
Signed-off-by: Yannick Schnider <[email protected]>
@yannicks1 yannicks1 requested a review from rafvasq as a code owner August 8, 2025 07:08
@yannicks1
Copy link
Collaborator Author

@maxdebayser, I added the test in tests/utils/test_upstream_compatibility.py. If you could have a quick look and approve that would be great.

@yannicks1 yannicks1 enabled auto-merge (squash) August 8, 2025 07:09
@github-actions github-actions bot added the ready label Aug 8, 2025
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.

LGTM

@yannicks1 yannicks1 merged commit e8ca058 into main Aug 8, 2025
24 checks passed
@yannicks1 yannicks1 deleted the ysc-fix-embedding-main branch August 8, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants