-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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
cmake: add ggml find package #11369
cmake: add ggml find package #11369
Conversation
@bandoti Thank you for helping out. Do you plan to finish the rest of the check items within this PR? |
I need to at very least complete the second item as I haven't tested a link against it yet. Since the other examples depend on common I can't use them and need something very minimal, preferably in C to ensure the APIs are clean. If you happen to have such a simple example kicking around the office I could copy and paste it. 😉 Otherwise I am happy to work through it. I am thinking of deferring the CI change however as there's a couple ways I can wire it into the general build. It would be nice to have it tested on all the existing builds (instead of adding new jobs) since they have various backends already and we don't want to stress the CI unnecessarily with more redundant builds. |
The simple example does not depend on |
Perfect! Thank you. I will use that as a basis then. |
I have finished this change and tested with CPU/Vulkan builds to ensure multiple backends works okay. Although I was careful to ensure the other transient dependencies are roped in for the other backends, I expect some issues will come up and can be fixed in later PRs. In addition, both with/without shared libraries has been tested. My plan is to follow up with some CI related changes to add regression tests here. As of now, none of this is being tested by the CI, so at the very least I will add one in the coming week to test CPU + Vulkan to ensure nothing breaks the transient dependencies. This would ensure that should issues arise it is likely due only to a specific dependency not being met as opposed to a broken cmake package (hopefully)! @slaren, @ggerganov |
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.
Tested on macOS and it works as expected:
otool -L ./examples/simple-cmake-pkg/build/llama-simple-cmake-pkg
./examples/simple-cmake-pkg/build/llama-simple-cmake-pkg:
@rpath/libllama.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libggml-cpu.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libggml-blas.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libggml-metal.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libggml.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libggml-base.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
@bandoti Sounds great, help is appreciated. Sent you a collaborator invite. |
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
@ggerganov Thank you for the vote of confidence! I am excited to be part of such an interesting project and look forward to adding more value over time. Should I merge this one in once the CI finishes? 😅 |
This change adds a cmake find package to the ggml library. It is responsible for providing link targets (of the form
ggml::<backend>
) for each backend. In addition, there is aggml::all
target which will link all backends.Example usage:
This change also now requires users of the llama find-package to explicitly request the backends:
Work progress:
main-cmake-pkg
)