-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Core] Drop overly aggressive whisper assertion #25408
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
Conversation
This assertion is causing a failure in a legitimate configuration. The assertion isn't as valuable anymore since whisper is the only encoder-decoder model left in vLLM. It made more sense we had all of the other models that were still supoprted in V0. Closes vllm-project#25318 Signed-off-by: Russell Bryant <[email protected]>
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.
Code Review
This pull request removes an assertion that was restrictively checking for 'whisper' in the model name for encoder-decoder models. This change is intended to support custom-named or fine-tuned Whisper models. While this addresses the immediate issue, it also removes a safeguard that prevents non-Whisper models from using Whisper-specific logic, which could lead to silent failures or incorrect behavior. My review suggests replacing the removed assertion with a warning to better balance flexibility and safety, alerting users to potential incompatibilities without blocking execution.
Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: Russell Bryant <[email protected]> Signed-off-by: charlifu <[email protected]>
Signed-off-by: Russell Bryant <[email protected]> Signed-off-by: yewentao256 <[email protected]>
Signed-off-by: Russell Bryant <[email protected]> Signed-off-by: gaojc <[email protected]>
Signed-off-by: Russell Bryant <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
Signed-off-by: Russell Bryant <[email protected]>
This assertion is causing a failure in a legitimate configuration. The
assertion isn't as valuable anymore since whisper is the only
encoder-decoder model left in vLLM. It made more sense we had all of
the other models that were still supoprted in V0.
Closes #25318
Signed-off-by: Russell Bryant [email protected]