-
Notifications
You must be signed in to change notification settings - Fork 269
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
Proposal: Improve llama.cpp snippet #778
Changes from 6 commits
656fc6e
d8dafa2
ce2fe36
98a2637
5f4547e
3d5db7d
31fdf54
7983478
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,12 @@ | ||||||
import type { ModelData } from "./model-data"; | ||||||
import type { PipelineType } from "./pipelines"; | ||||||
|
||||||
interface Snippet { | ||||||
title: string; | ||||||
setup: string; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
make the setup step optional. Maybe some snippets will not need the setup step There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 7983478 |
||||||
command: string; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestions @mishig25 . Sorry for the late response, I'll have a look later this week when I have more time ! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 7983478 |
||||||
} | ||||||
|
||||||
/** | ||||||
* Elements configurable by a local app. | ||||||
*/ | ||||||
|
@@ -39,36 +45,48 @@ export type LocalApp = { | |||||
* And if not (mostly llama.cpp), snippet to copy/paste in your terminal | ||||||
* Support the placeholder {{GGUF_FILE}} that will be replaced by the gguf file path or the list of available files. | ||||||
*/ | ||||||
snippet: (model: ModelData, filepath?: string) => string | string[]; | ||||||
snippet: (model: ModelData, filepath?: string) => string | string[] | Snippet | Snippet[]; | ||||||
} | ||||||
); | ||||||
|
||||||
function isGgufModel(model: ModelData) { | ||||||
return model.tags.includes("gguf"); | ||||||
} | ||||||
|
||||||
const snippetLlamacpp = (model: ModelData, filepath?: string): string[] => { | ||||||
const snippetLlamacpp = (model: ModelData, filepath?: string): Snippet[] => { | ||||||
const command = (binary: string) => | ||||||
[ | ||||||
"# Load and run the model:", | ||||||
`${binary} \\`, | ||||||
` --hf-repo "${model.id}" \\`, | ||||||
` --hf-file ${filepath ?? "{{GGUF_FILE}}"} \\`, | ||||||
' -p "You are a helpful assistant" \\', | ||||||
" --conversation", | ||||||
Vaibhavs10 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
].join("\n"); | ||||||
return [ | ||||||
`# Option 1: use llama.cpp with brew | ||||||
brew install llama.cpp | ||||||
|
||||||
# Load and run the model | ||||||
llama \\ | ||||||
--hf-repo "${model.id}" \\ | ||||||
--hf-file ${filepath ?? "{{GGUF_FILE}}"} \\ | ||||||
-p "I believe the meaning of life is" \\ | ||||||
-n 128`, | ||||||
`# Option 2: build llama.cpp from source with curl support | ||||||
git clone https://github.com/ggerganov/llama.cpp.git | ||||||
cd llama.cpp | ||||||
LLAMA_CURL=1 make | ||||||
|
||||||
# Load and run the model | ||||||
./main \\ | ||||||
--hf-repo "${model.id}" \\ | ||||||
-m ${filepath ?? "{{GGUF_FILE}}"} \\ | ||||||
-p "I believe the meaning of life is" \\ | ||||||
-n 128`, | ||||||
{ | ||||||
title: "Install from brew", | ||||||
setup: "brew install llama.cpp", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw unrelated but wondering is llama.cpp is on winget? cc @mfuntowicz too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked here: https://winget.run and didn't find any. To think of it, it'd be a pretty cool idea to add that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it would be nice to have, knowing that llama.cpp already have pre-built binary via CI. Unfortunately I'm not very familiar with windows stuff, so I'll create an issue on llama.cpp to see if someone can help. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created the issue: ggerganov/llama.cpp#8188 |
||||||
command: command("llama-cli"), | ||||||
}, | ||||||
{ | ||||||
title: "Use pre-built binary", | ||||||
setup: [ | ||||||
// prettier-ignore | ||||||
"# Download pre-built binary from:", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Conceptually LGTM. Just wondering if this doesn't bloat the overall UI for snippets for a user i.e. we present too many options to the end-user. (just food for thought) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, I switched my mind to "UX/UI designer" when I drafted this proposal ;-) The current UI has a problem that these multiple snippets but no title for them (visually hard to distinct between the 2 snippets): My first iteration would be to have a title for each snippet, then only "expand" one section at a time (while other options are "collapsed") But then I think we can also split between the "setup" and "run" step, since ideally the user will setup just once but run multiple times. Feel free to give other suggestions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea I quite like the second image. Let's ask @gary149/ @julien-c for thoughts here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @gary149 wdyt There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bumping this as I think it'll be good to merge this soon! cc: @gary149 (sorry for creating an extra notification) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay on the UI, but let's see if it's not busted on the Hub side. (sorry for the late reply) |
||||||
"# https://github.com/ggerganov/llama.cpp/releases", | ||||||
].join("\n"), | ||||||
command: command("./llama-cli"), | ||||||
}, | ||||||
{ | ||||||
title: "Build from source code", | ||||||
setup: [ | ||||||
"git clone https://github.com/ggerganov/llama.cpp.git", | ||||||
"cd llama.cpp", | ||||||
"LLAMA_CURL=1 make llama-cli", | ||||||
].join("\n"), | ||||||
command: command("./llama-cli"), | ||||||
}, | ||||||
]; | ||||||
}; | ||||||
|
||||||
|
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.
suggestion to export it and name it as
LocalAppSnippet
so that we can use it from hf.co codebaseThere 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.
Fixed in 7983478
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.
Actually, I like the visual improvements mentioned above and we could use them for other snippets as well (i.e., use in transformers), not just local apps. We can discuss about a
Snippet
type in a separate PR and move forward with this one for now.