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

server : fix templates for llama2, llama3 and zephyr in new UI #8196

Closed
wants to merge 3 commits into from

Conversation

mgroeber9110
Copy link
Contributor

This change makes some adjustments to the pre-defined chat templates in the new server UI, which in my interpretation bring them in line to the recommended versions. I have done this for the following templates:

  • Llama2
  • Llama3
  • Zephyr

as these are the models I have some experience with. There might be further similar discrepancies for other templates, but I have not checked those. For the Llama models, I have also removed the start-of-text tokens at the beginning, as they are automatically added by the server, and their duplication leads to a warning message.

It would of course be nicer to connect this to the llama_chat_apply_template() implementation to have only one set of templates to maintain and test in the codebase, for example by making the server UI use the chat endpoint rather than the completion one. Is anyone already working on this?

@mofosyne mofosyne added the Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix label Jun 29, 2024
@mgroeber9110
Copy link
Contributor Author

Just noticed that there is plenty of discussion about the future direction of templating in the server in #4216.

@ngxson
Copy link
Collaborator

ngxson commented Jun 30, 2024

I'm not familiar with the template system in JS, but seems like there are still some issues. For example, llama 3 never has new lines before EOT token (<|eot_id|>\n), or whether historyTemplate of llama2 should also include |INST] or not, etc.

It would of course be nicer to connect this to the llama_chat_apply_template() implementation to have only one set of templates to maintain and test in the codebase, for example by making the server UI use the chat endpoint rather than the completion one. Is anyone already working on this?

Yes it would be nice to do so. Currently there is no task to add this such feature. Probably we can extend /props endpoint to return a formatted chat example (inspired by llama_chat_format_example), then JS code will parse it to get the template.

@mgroeber9110
Copy link
Contributor Author

@ngxson Thanks - I have removed the extra newlines in llama3.

As for the llama2 template, the [INST] is already added through the userMsgPrefix field in the template description (not visible in the diff). Support for this template is still not pretty, because the [INST] ... [/INST] are shown in the Chat UI around the request, rather than being stripped off.

Making the chat example available through the /props endpoint sounds like a good idea. Should I create an Issue for this? As far as I can see, parsing the example to find the "looping" part in a longer conversation is not always trivial, but at least this would give the client code some options to check/select templates automatically. At the very least, it might be possible to bring up a warning if the chosen template does not match the example in the model character by character.

@mgroeber9110
Copy link
Contributor Author

I am closing this PR to be able to update my repo without causing too much "noise" (as I accidentally made it from "master"), and to wait for the outcome of the investigation for #8694.

@mgroeber9110
Copy link
Contributor Author

closing for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants