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

Feat/custom chat template #1233

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

Conversation

komninoschatzipapas
Copy link

Description

Adds a custom_chat_template parameter on the config that allows to set a custom chat template in Jinja format.

Motivation and Context

It's convenient so those of use who use this won't need to edit the tokenizer config directly after the fine-tune has finished.

How has this been tested?

I did not write any test cases for this. I tested it manually on my own environment and it does seem to work.

Copy link
Collaborator

@winglian winglian left a comment

Choose a reason for hiding this comment

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

generally this is good. One thing I'm considering would be to overload the chat_template: option as Optional[Union[bool, str]]

@@ -615,6 +615,8 @@ rl:
chat_template: chatml
# Changes the default system message
default_system_message: You are a helpful assistant. Please give a long and detailed answer. # Currently only supports chatml.
# A custom chat template in Jinja format. Will overwrite `chat_template` and `default_system_message`
Copy link
Collaborator

Choose a reason for hiding this comment

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

"May overwrite the default system message" since you could design the chat template to accept one.

@@ -218,7 +218,9 @@ def load_tokenizer(cfg):
LOG.debug(f"PAD: {tokenizer.pad_token_id} / {tokenizer.pad_token}")
LOG.debug(f"UNK: {tokenizer.unk_token_id} / {tokenizer.unk_token}")

if cfg.chat_template:
if cfg.custom_chat_template:
tokenizer.chat_template = cfg.custom_chat_template
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar to the chatml replacement, we could consider some sort of standardized string in the chat template that could be replaced based on the configuration.

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.

None yet

2 participants