Skip to content
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

llama-cpp: merge upstream changes #299271

Merged
merged 3 commits into from
Mar 29, 2024
Merged

llama-cpp: merge upstream changes #299271

merged 3 commits into from
Mar 29, 2024

Conversation

josephst
Copy link
Member

Description of changes

llama-cpp upstream has had some recent changes in their package.nix, namely embedding Metal shaders on Darwin to avoid xcrun and Xcode dependency and renaming cuBlas backend to CUDA. This PR brings those changes into the llama-cpp that's packaged in nixpkgs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed (on Darwin aarch64)
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

port of ggerganov/llama.cpp#6118, although compiling shaders with XCode disabled as it requires disabling sandbox (and only works on MacOS anyways)
@philiptaron
Copy link
Contributor

I'll be able to build this on aarch64-darwin tonight if ofBorg doesn't snipe me first.

@drupol
Copy link
Contributor

drupol commented Mar 28, 2024

@abysssol Do you think you could have a look at this?

@abysssol
Copy link
Contributor

abysssol commented Mar 28, 2024

@drupol I took a look at the changes, but I'm afraid I can't offer much insight, as I'm not familiar with llama-cpp (the nix package, the flake, nor the project itself). If you want me to try to figure something specific out, you'll need to be more detailed about what you actually want me to do.

The only thing I noticed is that a version update is probably necessary.
The pr renaming LLAMA_CUBLAS to LLAMA_CUDA is newer than release b2481, which is the version currently used in the nix package. So, LLAMA_CUDA probably won't have any effect due to being used with an outdated version of llama-cpp that hasn't yet renamed the environment variable.
LLAMA_METAL_EMBED_LIBRARY probably doesn't have this problem, since the pr adding support for it is older than release b2481.

Copy link
Member Author

@josephst josephst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to latest upstream package - b2568

@drupol drupol merged commit 40fe01a into NixOS:master Mar 29, 2024
22 of 23 checks passed
@josephst josephst deleted the fix-llamacpp branch April 6, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants