fix: use cache_salt for gpt-oss #23186
Conversation
dr75
requested review from
DarkLight1337,
aarnphm,
robertgshaw2-redhat and
simon-mo
as code owners
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run 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 either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add support for cache_salt for gpt-oss models in chat completions. The implementation correctly adds this for the gpt-oss specific code path. A new unit test is also added to verify this functionality.
However, the new test is parameterized to check for cache_salt support for all model types, not just gpt-oss. The current implementation is incomplete as it does not handle cache_salt for other model types, which will likely cause the test to fail. I've left a comment with a suggestion on how to implement this consistently for all chat models.
dr75
force-pushed
the
gpt-oss-cache-salt
branch
from
28ec97d to
45ea2a8
Compare
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
dr75
force-pushed
the
gpt-oss-cache-salt
branch
from
45ea2a8 to
14616b1
Compare
DarkLight1337
left a comment
DarkLight1337
left a comment
There was a problem hiding this comment.
Seems that this is also missed in serving_responses.py, can you fix that as well?
Oh, indeed. Will fix it there as well. |
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
github-actions
bot
added
the
ready
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com> Signed-off-by: Duncan Moss <djm.moss@gmail.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com> Signed-off-by: Xiao Yu <xiao.yu@amd.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
Signed-off-by: Marko Rosenmueller <5467316+dr75@users.noreply.github.com>
2 participants
Purpose
When using gpt-oss, vLLM is ignoring the
cache_saltrequest field in chat completions and responses APIs.Loading the value if set in the request as for other models.
Test Plan
Added a unit test.