-
-
Couldn't load subscription status.
- Fork 10.8k
[Bugfix] Set the minimum python version for gpt-oss #26392
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
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 fixes an installation failure of the gpt-oss dependency on Python versions below 3.12 by making it a conditional dependency. While this solves the immediate installation problem, it introduces a more subtle issue: users on older Python versions will experience runtime errors when trying to use the gpt-oss model, as the required package won't be installed. My review points out this degradation in user experience and recommends adding explicit runtime checks to inform the user about the version incompatibility, even though the relevant files are not part of this pull request.
requirements/common.txt
Outdated
| setproctitle # Used to set process names for better debugging and monitoring | ||
| openai-harmony >= 0.0.3 # Required for gpt-oss | ||
| gpt-oss >= 0.0.7 | ||
| gpt-oss >= 0.0.7; python_version > '3.11' |
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.
This change fixes an installation error by making the gpt-oss dependency conditional. However, this approach trades a clear installation-time failure for a silent runtime failure. Users on Python < 3.12 will be able to install the package, but will get a cryptic ImportError if they try to use the gpt-oss model. This is a poor user experience.
A better approach would be to add a runtime check within the gpt-oss model code to detect if the dependency is missing and provide a clear error message to the user, explaining that the model requires Python 3.12 or newer. While that file is not in this PR, proceeding with this change alone is problematic as it can lead to confusion.
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.
gpt-oss is an optional dependency in the code but CI requires it. What about removing this dependency and add it to requirements/test.in?
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.
It was added here 91ac7f7
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.
Done in 30faf26
5f37499 to
30faf26
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.
LGTM!
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
…to loader * 'loader' of https://github.com/dsxsteven/vllm_splitPR: (778 commits) [torchao] Add support for ModuleFqnToConfig using regex (vllm-project#26001) Add: Support for multiple hidden layers in Eagle3 (vllm-project#26164) Enable `RMSNorm` substitution for Transformers backend (vllm-project#26353) [Model] Gemma3: Fix GGUF loading and quantization (vllm-project#26189) Bump Flashinfer to v0.4.0 (vllm-project#26326) Update Dockerfile and install runai-model-streamer[gcs] package (vllm-project#26464) [Core] Relax the LoRA max rank (vllm-project#26461) [CI/Build] Fix model nightly tests (vllm-project#26466) [Hybrid]: Decouple Kernel Block Size from KV Page Size (vllm-project#24486) [Core][KVConnector] Propagate all tokens on resumed preemptions (vllm-project#24926) [MM][Doc] Add documentation for configurable mm profiling (vllm-project#26200) [Hardware][AMD] Enable FlexAttention backend on ROCm (vllm-project#26439) [Bugfix] Incorrect another MM data format in vllm bench throughput (vllm-project#26462) [Bugfix] Catch and log invalid token ids in detokenizer #2 (vllm-project#26445) [Minor] Change warning->warning_once in preprocess (vllm-project#26455) [Bugfix] Set the minimum python version for gpt-oss (vllm-project#26392) [Misc] Redact ray runtime env before logging (vllm-project#26302) Separate MLAAttention class from Attention (vllm-project#25103) [Attention] Register FLASHMLA_SPARSE (vllm-project#26441) [Kernels] Modular kernel refactor (vllm-project#24812) ...
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: Dhruvil Bhatt <bhattdbh@amazon.com>
|
Following the installation guide for spinning up gpt-oss with vllm I still get this error: |
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
Purpose
The following error occurs when using Python 3.11:
FIX #26434
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.