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

Added --chat-template-file to llama-run #11961

Merged

Conversation

engelmi
Copy link
Contributor

@engelmi engelmi commented Feb 19, 2025

Relates to: #11178

Added --chat-template-file CLI option to llama-run. If specified, the file will be read and the content passed for overwriting the chat template of the model to common_chat_templates_from_model.

This also enables running the granite-code model from ollama:

# using a jinja chat template file 
# (when prefix, e.g. hf://, is not specified, llama-run pulls from ollama)
$ llama-run  --chat-template-file ./chat.tmpl granite-code
> write code

Here is a code snippet in Python:

"""
def f(x):
    return x**2
"""

# without a jinja chat template file
$ llama-run granite-code
> write code
failed to apply the chat template

Make sure to read the contributing guidelines before submitting a PR

@engelmi
Copy link
Contributor Author

engelmi commented Feb 19, 2025

Preceding PR: #11922

And still this error even though there is no merge commit:

Merge is not an allowed merge method in this repository.
This branch must not contain merge commits. 

@engelmi engelmi force-pushed the added-chat-template-file-to-llama-run branch 3 times, most recently from 193eb87 to d1b57cf Compare February 19, 2025 20:12
Relates to: ggml-org#11178

Added --chat-template-file CLI option to llama-run. If specified, the file
will be read and the content passed for overwriting the chat template of
the model to common_chat_templates_from_model.

Signed-off-by: Michael Engel <mengel@redhat.com>
@engelmi engelmi force-pushed the added-chat-template-file-to-llama-run branch from d1b57cf to 530ba31 Compare February 19, 2025 20:14
@ggerganov ggerganov merged commit 0d55958 into ggml-org:master Feb 20, 2025
46 checks passed
if(!chat_template_file.empty()){
chat_template = read_chat_template_file(chat_template_file);
}
auto chat_templates = common_chat_templates_init(llama_data.model.get(), chat_template.empty() ? nullptr : chat_template);
Copy link
Collaborator

@ericcurtin ericcurtin Feb 20, 2025

Choose a reason for hiding this comment

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

We should do:

chat_template.empty() ? "" : chat_template

here. Passing nullptr to a reference is not allowed. I wish the compiler caught these things.

common_chat_templates_ptr common_chat_templates_init(
                                    const struct llama_model * model,
                                           const std::string & chat_template_override,
                                           const std::string & bos_token_override = "",
                                           const std::string & eos_token_override = "")

Copy link
Collaborator

@ericcurtin ericcurtin Feb 20, 2025

Choose a reason for hiding this comment

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

Maybe the std::string class is smart enough to interpret all these as the same thing:

"", '', 0, NULL, nullptr

and that's why it compiles/works 🤷 . So it might be just implicitly converting it to "".

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