-
Notifications
You must be signed in to change notification settings - Fork 21
🎨 Remove new_token_ids from warmup #292
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.
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
base: main
Are you sure you want to change the base?
Conversation
👋 Hi! Thank you for contributing to vLLM support on Spyre.
Or this can be done with
Now you are good to go 🚀 |
dd4e9e8
to
5113a4f
Compare
bot:test |
9ac70cb
to
f8fcd6d
Compare
Signed-off-by: Prashant Gupta <[email protected]>
f8fcd6d
to
83059cb
Compare
Signed-off-by: Prashant Gupta <[email protected]>
Signed-off-by: Prashant Gupta <[email protected]>
Signed-off-by: Prashant Gupta <[email protected]>
@prashantgupta24 do we wanna get this merged? If so we should definitely test to triple-check that this works with the upcoming compiler changes for continuous batching |
No hurry as such, don't want to add anything before the release lol |
Signed-off-by: Prashant Gupta <[email protected]>
Signed-off-by: Prashant Gupta <[email protected]>
Signed-off-by: Prashant Gupta <[email protected]>
bot:test |
1 similar comment
bot:test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Failed one of the tests on spyre
will investigate, seems flaky... |
bot:test |
bot:test |
Failed 2 different tests this time -
Seems flaky since running only CB tests made them all pass 🤔 |
Too much flakiness going on, going to convert it to a draft for now. |
bot:test |
Description
🎨 Remove
new_token_ids
from warmup sincenew_token_ids
are not used anymore.