-
-
Notifications
You must be signed in to change notification settings - Fork 12
Enable Vulkan support on Linux and Windows to support non-Nvidia GPUs #64
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
|
@conda-forge-admin, please rerender |
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe/recipe.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/17761278666. Examine the logs at this URL for more detail. |
|
It seems that we need to understand how to correctly handle cross-compilation for |
|
xref: ggml-org/llama.cpp#10448 |
|
The test is failing with: I am not sure why the package build worked locally. |
|
Beside Nvidia and Intel I wanted also to test on AMD, but unfortunatly the only AMD GPU I have access to is a MI300X, that is not currently supported by AMD Vulkan driver. |
|
Hi! This is the friendly automated conda-forge-linting service. I failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/17765048761. Examine the logs at this URL for more detail. |
70c1d7d to
9592fa2
Compare
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe/recipe.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/17779036826. Examine the logs at this URL for more detail. |
|
osx-arm64 still fails as the LDFLAGS of the cross-compile build conflict with the one of the native build, see: tipically this is solved with a separate build, see for example https://github.com/conda-forge/gz-msgs-feedstock/blob/5275f5d8402b69aeebf5f766620db5b148cc9c88/recipe/build_cxx.sh#L15 . |
|
Alternatively, we can indeed define a toolchain to pass along to the |
895bea0 to
493a6a0
Compare
I ended up disabling vulkan on macOS as I doubt it would be useful and it requires a complex workarounds to ensure that it works fine. |
10e97bb to
3f655e5
Compare
499cd10 to
3bfbf82
Compare
|
@conda-forge/llama.cpp the PR is ready for discussion and review, thanks! |
jjerphan
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.
Thank you for the contribution!
While supporting Vulkan is valuable, I think I would slightly be in favor of having dedicated build variants for it. Ideally, those build variants would be picked after hardware detection (e.g. like with the __cuda dependency), but I guess a CEP is necessary for this.
Also could you become a maintainer of this feedstock?
| # Vulkan is only useful on Linux, as on macOS | ||
| # metal is used | ||
| {%- if linux %} | ||
| ${{ ggml_args("VULKAN=ON") }} | ||
| {%- endif %} |
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 to me.
Sure! |
|
Let me have you approve #66 (comment). |
Ack! Indeed, some kind of vulkan-based hardware detection would be cool, even if a bit longer term (interesting, even on the WheelNext variant providers side I could not see anything related to Vulkan) and even if in general virtual packages and the corresponding variants have the downside of making more complex dealing with lock files. Just to understand, what is the downside for which you would prefer to have a different vulkan variant? Just the increase in size of the package, or something else? |
|
The size of packages could continue to grow overtime if we continue to optimize builds. I am fine with this approach since elements for packaging this variant are missing, but I would like to have the other maintainers' perspective before approving. |
I totally agree on this, that is a common concern in binary-based package distributions like conda-forge or Linux distro. From source distro typically mitigate the problem by having optional flags to enable/disable feature, but also in that case a lot of optional flags can really complexify caching artifacts. My impression is that given there is a tradeoff between "increase in size" and "convenient", the call needs to be done on specific cases, while it is difficult to take a decision in general (or imagine what will happen in the future, that is difficult to forecast). In this case the increase of size seems to me to justify the increase usefulness, but I totally understand if the opinion of other maintainers may be different. |
jjerphan
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.
Note sure if the other maintainers are going to reply.
|
How about introducing a |
If you prefer, I can do that. The question is: without any virtual package to select if |
|
My personal impression/assumption is that most users actually have a working vulkan GPU, and so we would make them a good service in making the |
|
I would not weight variants for now. I do not have time to garden this feedstock. Feel free to implement the changes. |
Why? If we do not give a priority to a variant or another, it is quite difficult in my experience to understant which variants gets installed by default.
Ack, I will wait anyhow to see if anyone is interested in this as anyhow I am not in a hurry. |
|
@traversaro: I do not think that other maintainers will reply. Feel free to rebase and merge this PR. |
Yes, I sorted of figured that out. : ) However, I think I have way to address most comments/concerns in this PR, I need to update this PR with those. |
|
Closing until I update the PR for clarity. |
This PR enables Vulkan support in all builds. With just an additional ~5 MB more, we can a package that can use GPU also on non-Nvidia GPUs (I tested just on an integrated Intel GPU and the perfomance are not at the level of CUDA, but definitely better then CPU).
For example, on my testing machine Windows laptop with NVIDIA GeForce RTX 4050 running Linux binaries via WSL these are the results:
In case users want for use cpu for any reason, they can do that by passing
---device nonetollama-cliorllama-server.Thinks to discuss:
cpuvariant also supports GPU (via Vulkan). So it is ok to keep calling itcpu, or should we use another name?__cudadefined, the cuda package will continue to be installed. For completeness, I also included the Vulkan backend also on CUDA builds, to permit users to easily test them with--deviceoption, as the cost is just ~5 MB of size more. However we can also disable.Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)