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

python: follow-up to Jinja PR #3225

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

python: follow-up to Jinja PR #3225

wants to merge 4 commits into from

Conversation

cebtenzzre
Copy link
Member

@cebtenzzre cebtenzzre commented Dec 5, 2024

  • Bump the major version because we have made a breaking change to the chat templates
  • Require explicit naming of chat_template and system_message parameters (e.g. chat_session(chat_template=...)), to make clear that they are not the same as the old prompt_template/system_prompt
  • Implement loading of chat templates from model files when the one from models3.json is not present/available
  • Warn if the chat template or system message looks like the old style, using the same regexes as the chat UI
  • Fix use of llmodel_model_foreach_special_token before the model is loaded, which was causing an exception when initializing a model
  • Mention the possible solution of allow_download=True if the user passed allow_download=False and the model has no built-in chat template
  • Provide a model.current_chat_template property to identify which chat template is in use, which may be different from model.config['chat_template'] if overridden or loaded from the GGUF file

I've tested a few different combinations of models which do/don't have a default chat template, do/don't have a chat template in models3.json, and do/don't have an overridden chat template.

I've also tested with some old-style templates with and without warn_legacy=False.

@cebtenzzre cebtenzzre requested a review from manyoso December 5, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant