- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 10.9k
fix: cuda 12.6 installation #19095
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?
fix: cuda 12.6 installation #19095
Conversation
| 👋 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  🚀 | 
5666007    to
    88d99cf      
    Compare
  
    | @huydhn , could you help review this one? | 
        
          
                setup.py
              
                Outdated
          
        
      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.
I feel we can move most of the changes to some file like setup_utils.py, and only keep the call / override logic here to minimize the changes in the setup.py.
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.
I feel we can move most of the changes to some file like setup_utils.py, and only keep the call / override logic here to minimize the changes in the setup.py.
agreed; it'll be a bit easier to remove if this ever gets fixed
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! Thanks!
Signed-off-by: Mickael Seznec <[email protected]>
Signed-off-by: Mickael Seznec <[email protected]>
Signed-off-by: Mickael Seznec <[email protected]>
Signed-off-by: Mickael Seznec <[email protected]>
a40bf44    to
    7fbb98e      
    Compare
  
    Signed-off-by: Lucas Wilkinson <[email protected]>
| This pull request has merge conflicts that must be resolved before it can be | 
Essential Elements of an Effective PR Description Checklist
Purpose
Fix #15435
Test Plan
CUDA 12.6 isn't used as default but might still be useful for some people. Not sure we want to add a CI check to this? I've tested locally, but if you want more testing, let me know what I should do 🙂
Test Result