Proposal: Improve llama.cpp snippet #778

ngxson · 2024-06-27T21:34:44Z

In this PR, I propose some changes:

Update binary name to llama-cli (for more details, see this PR: build: rename main → llama-cli, server → llama-server, llava-cli → llama-llava-cli, etc... ggerganov/llama.cpp#7809 and this homebrew formula)
Add method to download llama.cpp via pre-built release
Split snippet into 3 sections: title, setup and command
Use --conversation mode to start llama.cpp in chat mode (chat template is now supported, ref: Add chat template support for llama-cli ggerganov/llama.cpp#8068)

Proposal for the UI:

(Note: Maybe the 3 sections title - setup - command can be more separated visually)

Vaibhavs10

Thanks for making the docs better! 🤗

packages/tasks/src/local-apps.ts

Vaibhavs10 · 2024-06-28T10:10:55Z

packages/tasks/src/local-apps.ts

+			title: "Use pre-built binary",
+			setup: [
+				// prettier-ignore
+				"# Download pre-built binary from:",


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)

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

Good idea I quite like the second image. Let's ask @gary149/ @julien-c for thoughts here

cc @gary149 wdyt

bumping this as I think it'll be good to merge this soon! cc: @gary149 (sorry for creating an extra notification)

okay on the UI, but let's see if it's not busted on the Hub side. (sorry for the late reply)

packages/tasks/src/local-apps.ts

julien-c · 2024-06-28T12:36:56Z

packages/tasks/src/local-apps.ts

@@ -39,36 +45,52 @@ 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) => string | string[];
+			snippet: (model: ModelData) => Snippet | Snippet[];


Suggested change

snippet: (model: ModelData) => Snippet | Snippet[];

snippet: (model: ModelData) => string | string[] | Snippet | Snippet[];

to be backward compatible (and to compile)

Fixed with 98a2637

But I would recommend to move completely to Snippet interface, because my idea is to have dedicated "title" to explain what each snippet does.

julien-c

i've launched the CI

julien-c · 2024-06-28T12:37:57Z

packages/tasks/src/local-apps.ts

-	-n 128`,
+		{
+			title: "Install from brew",
+			setup: "brew install llama.cpp",


btw unrelated but wondering is llama.cpp is on winget? cc @mfuntowicz too

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.

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.

Created the issue: ggerganov/llama.cpp#8188

julien-c

let's note to also update/improve this doc page: https://huggingface.co/docs/hub/en/gguf-llamacpp

(relevant issue)

mishig25 · 2024-07-16T12:04:55Z

packages/tasks/src/local-apps.ts

@@ -1,6 +1,12 @@
 import type { ModelData } from "./model-data";
 import type { PipelineType } from "./pipelines";

+interface Snippet {


Suggested change

interface Snippet {

export interface LocalAppSnippet {

suggestion to export it and name it as LocalAppSnippet so that we can use it from hf.co codebase

Fixed in 7983478

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.

mishig25 · 2024-07-16T12:10:51Z

packages/tasks/src/local-apps.ts

@@ -1,6 +1,12 @@
 import type { ModelData } from "./model-data";
 import type { PipelineType } from "./pipelines";

+interface Snippet {
+	title: string;
+	setup: string;


Suggested change

setup: string;

setup?: string;

make the setup step optional. Maybe some snippets will not need the setup step

Fixed in 7983478

mishig25 · 2024-07-16T12:15:57Z

packages/tasks/src/local-apps.ts

+interface Snippet {
+	title: string;
+	setup: string;
+	command: string;


Suggested change

command: string;

content: string;

rename command -> content to be consistent with how we name this kind of stuff in hf.co codebase

Thanks for the suggestions @mishig25 . Sorry for the late response, I'll have a look later this week when I have more time !

Fixed in 7983478

Vaibhavs10

Re: snippets - it's all good! Thanks for the update @ngxson

Let's wait for @mishig25 to recommend any changes required re: frontend.

pcuenca

Looks great, as mentioned I'd suggest to discuss a Snippet type in a future PR.

julien-c

imo let's merge as is and i can do required changes in moon-landing

mishig25

lgtm !

better llama.cpp snippet

656fc6e

ngxson requested review from osanseviero, SBrandeis, gary149, Wauplin, julien-c and pcuenca as code owners June 27, 2024 21:34

make single target

d8dafa2

Wauplin requested a review from Vaibhavs10 June 28, 2024 09:40

Vaibhavs10 reviewed Jun 28, 2024

View reviewed changes

julien-c reviewed Jun 28, 2024

View reviewed changes

ngxson added 4 commits June 28, 2024 15:13

Merge branch 'main' into xsn/llama_snippet

ce2fe36

fix lint

98a2637

Merge branch 'main' into xsn/llama_snippet

5f4547e

remove guide to install deps

3d5db7d

julien-c mentioned this pull request Jul 2, 2024

Update the GGUF usage with llama.cpp doc page huggingface/hub-docs#1277

Closed

julien-c reviewed Jul 2, 2024

View reviewed changes

gary149 requested a review from mishig25 July 16, 2024 10:41

mishig25 reviewed Jul 16, 2024

View reviewed changes

ngxson mentioned this pull request Jul 23, 2024

Issue: HuggingFace Documentation Refers to Outdated Binaries ggerganov/llama.cpp#8659

Closed

ngxson and others added 2 commits August 6, 2024 13:56

Merge branch 'huggingface:main' into xsn/llama_snippet

31fdf54

resolve review comments

7983478

Vaibhavs10 approved these changes Aug 6, 2024

View reviewed changes

Vaibhavs10 requested a review from mishig25 August 6, 2024 19:46

pcuenca approved these changes Aug 7, 2024

View reviewed changes

julien-c approved these changes Aug 7, 2024

View reviewed changes

mishig25 approved these changes Aug 7, 2024

View reviewed changes

mishig25 merged commit 7e6e143 into huggingface:main Aug 7, 2024
2 of 4 checks passed

mishig25 mentioned this pull request Oct 6, 2024

Rm LocalAppSnippet.setup #950

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal: Improve llama.cpp snippet #778

Proposal: Improve llama.cpp snippet #778

ngxson commented Jun 27, 2024 •

edited

Loading

Vaibhavs10 left a comment

Vaibhavs10 Jun 28, 2024

ngxson Jun 28, 2024

Vaibhavs10 Jun 28, 2024

julien-c Jun 28, 2024

Vaibhavs10 Jul 4, 2024

gary149 Jul 13, 2024 •

edited

Loading

julien-c Jun 28, 2024

ngxson Jun 28, 2024

julien-c left a comment

julien-c Jun 28, 2024

Vaibhavs10 Jun 28, 2024

ngxson Jun 28, 2024

ngxson Jun 28, 2024

julien-c left a comment

mishig25 Jul 16, 2024

ngxson Aug 6, 2024

pcuenca Aug 7, 2024

mishig25 Jul 16, 2024

ngxson Aug 6, 2024

mishig25 Jul 16, 2024

ngxson Jul 23, 2024

ngxson Aug 6, 2024

Vaibhavs10 left a comment

pcuenca left a comment

julien-c left a comment

mishig25 left a comment

	snippet: (model: ModelData) => Snippet \| Snippet[];
	snippet: (model: ModelData) => string \| string[] \| Snippet \| Snippet[];

Proposal: Improve llama.cpp snippet #778

Proposal: Improve llama.cpp snippet #778

Conversation

ngxson commented Jun 27, 2024 • edited Loading

Vaibhavs10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gary149 Jul 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien-c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien-c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vaibhavs10 left a comment

Choose a reason for hiding this comment

pcuenca left a comment

Choose a reason for hiding this comment

julien-c left a comment

Choose a reason for hiding this comment

mishig25 left a comment

Choose a reason for hiding this comment

ngxson commented Jun 27, 2024 •

edited

Loading

gary149 Jul 13, 2024 •

edited

Loading