-
-
Notifications
You must be signed in to change notification settings - Fork 757
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
base: main
Are you sure you want to change the base?
Feat/custom chat template #1233
Conversation
There was a problem hiding this 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` |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
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.