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

examples : do not use common library in simple example #9803

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

slaren
Copy link
Collaborator

@slaren slaren commented Oct 9, 2024

Using the common library defeats the purpose of having an example that is supposed to serve as a simple starting point.

Comment on lines 125 to 128
char buf[128];
int n = llama_token_to_piece(model, new_token_id, buf, sizeof(buf), 0, true);
std::string s(buf, n);
fprintf(stderr, "%s", s.c_str());
Copy link
Owner

Choose a reason for hiding this comment

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

I recently found that some models can have tokens with length of up to 256. So it would be a good idea to check for negative n here.

Btw, is std::string necessary here? can we just set null terminator manually and print buf directly?

The logic for the LOG_ messages was to output the generated text to stdout and everything else to stderr. This has the benefit of being able to redirect info messages to somewhere else (e.g. /dev/null).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not really understand why llama_token_to_piece does not return a NUL-terminated string in the first place. I assume that it is because some tokens may contain \0 characters, in which case it is necessary to use std::string to represent them, even though they are discarded again in the call to printf.

Printing the generated text to stderr was a mistake, I will fix that.

@slaren slaren force-pushed the sl/simplify-simple-example branch from c4276b0 to 8f83141 Compare October 9, 2024 12:14
@slaren slaren force-pushed the sl/simplify-simple-example branch from 8f83141 to 1c4d573 Compare October 9, 2024 12:16
@slaren
Copy link
Collaborator Author

slaren commented Oct 9, 2024

I have changed n_predict to mean number of tokens to generate excluding the prompt, which I believe is a bit easier to understand and implement, but it may be inconsistent with other examples.

@slaren slaren merged commit c7499c5 into master Oct 10, 2024
54 checks passed
@slaren slaren deleted the sl/simplify-simple-example branch October 10, 2024 17:50
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
* examples : do not use common library in simple example

* add command line parser, simplify code
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* examples : do not use common library in simple example

* add command line parser, simplify code
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* examples : do not use common library in simple example

* add command line parser, simplify code
@amqdn
Copy link
Contributor

amqdn commented Nov 21, 2024

@slaren Appreciate this change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants