Skip to content
This repository was archived by the owner on Jun 24, 2024. It is now read-only.

fix(metal): Copy ggml-metal.metal to target directory #324

Closed
wants to merge 1 commit into from

Conversation

nightscape
Copy link
Contributor

Rust noob here 😉
I've hacked a way to copy the required ggml-metal.metal file into the target/<profile> directory, otherwise users would always have to do it by themselves.
Not sure if there's a nicer way, especially the Go up 3 directories part rubs me the wrong way.
Still better than having a semi-broken build imo.

@LLukas22
Copy link
Contributor

@pixelspark What are your thoughts about this? I have absolutely no idea about the metal stuff, so i won't comment on this 😅

@pixelspark
Copy link
Contributor

The current situation is temporary - at some point the shader code will be embedded in ggml-metal.c (using the equivalent of Rust's include_str!) - at least that is the intention indicated in the code.

For now I think this PR adds some value. Please add a comment/message somewhere as a reminder that this can be removed when ggml-metal.metal is not found anymore (and/or add an issue to keep track of this).

@nightscape
Copy link
Contributor Author

Added a comment in the code linking to the discussion here.
@pixelspark do you have a link for the corresponding discussion so we can reference it?

Good to merge from my side 😃

@pixelspark
Copy link
Contributor

See here: https://github.com/ggerganov/llama.cpp/blob/7487137227eb32ed9b12156338b865cb29b2dfd1/ggml-metal.m#L80

@philpax
Copy link
Collaborator

philpax commented Jun 22, 2023

Thanks for the PR! Yeah, this is better than having a build that doesn't work as expected.

However, I suspect this may not work as well if the build setup is non-standard, and the resulting file's likely to be dropped as part of deployment. It doesn't matter too much because this should be fixed upstream by being included in the actual file, but still...

Let me see if I can think of an alternate fix that might be a little more robust.

@philpax philpax added the topic:backend-support Support for alternate non-GGML backends, or for particular GGML backend features label Jun 22, 2023
@nightscape
Copy link
Contributor Author

Closing this in favor of #328

@nightscape nightscape closed this Jun 23, 2023
@nightscape nightscape deleted the copy-ggml-metal branch June 23, 2023 08:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic:backend-support Support for alternate non-GGML backends, or for particular GGML backend features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants