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

[Frontend] Add Command-R and Llama-3 chat template #10496

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ccs96307
Copy link

@ccs96307 ccs96307 commented Nov 20, 2024

This PR adds several chat templates in Jinja format.

The added templates are:

  • examples/rag_chat_template_command_r.jinja
  • examples/template_command_r.jinja
  • examples/template_llama3.jinja
  • examples/tool_chat_template_command_r.jinja

I noticed there are a feature request issue #9904 about adding chat template and I think I can help (I am researching how to use jinja to format my chat history processing recently). I have always wanted to contribute to the vLLM project, and I hope this PR can be a meaningful addition.

If the maintainers think these templates are unnecessary or beyond the project's scope, I am open to feedback and will happily adjust or close the PR as needed.

FIX #9904

Added: My Command-R chat template is based on CohereForAI/c4ai-command-r-08-2024

Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@DarkLight1337
Copy link
Member

cc @K-Mistele

Copy link
Contributor

@K-Mistele K-Mistele left a comment

Choose a reason for hiding this comment

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

One final note, might be good to have the chat templates for command-r named consistently, e.g. command_r_tool_chat_template, command_r_rag_chat_template, comand_r_default_chat_template or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this is necessary. Llama 3's chat template is already included in the model's tokenizer_config.json and this omits functionality like tool calls. What is the use case for this?

@ccs96307
Copy link
Author

Sure, I'll standardize the template names. Honestly, in terms of usefulness, Llama-3 isn't something I particularly care about—it's just that it was mentioned in the issue, so I tried adding it.

Should I remove it?

Copy link

mergify bot commented Nov 25, 2024

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @ccs96307.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Nov 25, 2024
@ccs96307 ccs96307 changed the title [Frontend] Add Command-R and Llama-3 chat template [WIP][Frontend] Add Command-R and Llama-3 chat template Nov 25, 2024
@ccs96307 ccs96307 marked this pull request as draft November 25, 2024 14:32
@ccs96307 ccs96307 closed this Nov 25, 2024
@ccs96307 ccs96307 force-pushed the add-command-r-jinja branch from a403868 to 2b0879b Compare November 25, 2024 14:37
@ccs96307 ccs96307 reopened this Nov 25, 2024
@mergify mergify bot removed the needs-rebase label Nov 25, 2024
@ccs96307 ccs96307 marked this pull request as ready for review November 25, 2024 14:49
@ccs96307 ccs96307 changed the title [WIP][Frontend] Add Command-R and Llama-3 chat template [Frontend] Add Command-R and Llama-3 chat template Nov 25, 2024
@ccs96307
Copy link
Author

Hi @K-Mistele , just following up on this PR. Please let me know if any further changes are needed or if there's anything else I can do to help. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Llama 3 and Command-R Chat Templates
3 participants