-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
[OpenAI] Extend VLLMValidationError to additional validation parameters #31870
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
[OpenAI] Extend VLLMValidationError to additional validation parameters #31870
Conversation
9d6405f to
cb83f9e
Compare
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 extends the VLLMValidationError to provide more detailed error messages for several validation parameters, improving the API's error reporting. The changes are well-structured, introducing a local _ValueError to prevent circular imports and updating validation logic across sampling_params.py and input_processor.py. The error handling in serving_engine.py is also updated to correctly process these new exception types. Overall, this is a good improvement. I've found one issue in serving_engine.py related to a redundant logical condition that should be simplified for clarity and maintainability.
cb83f9e to
7dbfac4
Compare
a0d15af to
6f3890b
Compare
baf6058 to
67a77b9
Compare
DarkLight1337
left a comment
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.
Thanks, LGTM now
Extend existing VLLMValidationError implementation to populate the param field for additional validation scenarios. Changes: - Create vllm/exceptions.py module to hold VLLMValidationError and avoid circular import between protocol.py and sampling_params.py - Move VLLMValidationError from protocol.py to the new exceptions module - Update sampling_params.py to import and use VLLMValidationError for 7 validation cases: temperature, top_p, max_tokens, logprobs, prompt_logprobs, truncate_prompt_tokens, and bad_words - Update v1/engine/input_processor.py to use VLLMValidationError for logprobs, prompt_logprobs, and logit_bias validations - Simplify error handling in serving_engine.py by removing duck-typing checks that are no longer needed - Update inline imports in serving_engine.py and api_server.py to use the new exceptions module New validations now populate the `param` field: - temperature, top_p, logprobs, prompt_logprobs, logit_bias, truncate_prompt_tokens, bad_words Signed-off-by: Rehan Khan <[email protected]>
Head branch was pushed to by a user without write access
67a77b9 to
d5b7bd1
Compare
…rs (vllm-project#31870) Signed-off-by: Rehan Khan <[email protected]>
Purpose
Extend existing VLLMValidationError implementation to populate the param field for additional validation scenarios.
Changes:
_ValueErrorin sampling_params.py to avoid circular imports with protocol.pyNew validations now populate the
paramfield:Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.