-
Notifications
You must be signed in to change notification settings - Fork 200
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
Chat templates #293
base: chat-templates
Are you sure you want to change the base?
Chat templates #293
Conversation
…hitespace handling
…my text for template split
Tested with this example @lbeurerkellner |
State handling looks good now thanks. I just wanted to test with First of all, some templates seem to expect a I ignored exceptions using the following implementation of
I am testing with the following query:
With this, we get the following.
|
I see, we could track the previous role as well and pass it in with another dumy text but parsing the start+end from the resulting string would be very tricky. Also for the same meta-llama/Llama-2-7b-chat-hf template returns an empty string when passing a system role without any messages after so that would also break it. Not sure I see any reasonable way to deal with these kind of templates. Would only supporting "simple" templates make sense? Or maybe make this feature more general where the user can pass a function that takes in a role, message and spits out start, end so people can deal with more complex requirements on a case by case basis? |
The problem with the Llama template seems to be that it does not allow all tag interleavings as expressible with our current {:role} syntax. If users specify e.g. two system messages, it will raise an error. I think it would be fine to limit support to simpler templates for now. For Llama, we should probably still add a custom template so that it works out of the box, as it is a common prompt template. But this can be a separate PR. For this, it should be fine to merge if we can make sure that template errors are reported properly and that users know how to fix errors by providing a simpler template. With the current system, what is the concrete requirement a template has to satisfy to be considered "simple"? Maybe that could be specified in the error message? |
Don't know if this is (still) relevant, but in the last line of code the hf prompt is compared to itself instead of the lmql one. Just wanted to let you know in case you didn't catch that yet :) |
Moved state to the PromptState and made further changes to align with the expected templating of huggingface.