Skip to content

Allow user to define the string to concatenate the role name and prompt in DefaultHistoryTransform #322

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

Closed
AsakusaRinne opened this issue Nov 20, 2023 · 5 comments
Labels
stale Stale issue will be autoclosed soon

Comments

@AsakusaRinne
Copy link
Collaborator

Currently in DefaultHistoryTransform we use ":" by defualt to connect the role name and the prompt. However in some language, ":" is not a commonly used string, for example, Chinese. In some other conditions, users may just want to use a self-defined connector. Therefore we'd better allow user to define the connector themselves.

@patrick-hovsepian
Copy link
Contributor

I'm new to the space of LLMs and was trying to get Llama 3 to run. At first, I copied the chat example in the readme but got some random odd behavior like run off prompts and gibberish replies or replies to older queries interleaved with others -- all sorts of whack. I then tried running with phi-3 and got weirdness there too. I finally came across the example from #708 and things started working for Llama. This made me realize I needed a transform for phi3 as well and ended up creating one for it and things have been working much better.

All that's to say, is it worth it to provide a variant of DefaultHistoryTransform for different models or at least a best guess if possible and then fallback to the existing Default? I was looking a bit at the llama.cpp source and saw they had some checks to detect the absence of those special tokens and add them but maybe it's worthwhile to do it from here as a transformer?

I might be conflating a few issues or just using the tools incorrectly but figured I'd bring it up after a weekend of debugging as a total newcomer to the space 😅

@AsakusaRinne
Copy link
Collaborator Author

Yes, what you are talking about is often called chat template. The DefaultHistoryTransform is an old thing developed a year ago and we'll replace it with chat templates, which will have different template for different models. Thank you for your suggestions.

@patrick-hovsepian
Copy link
Contributor

I'd be happy to help with development if possible. Let me know if there's any existing design or if you're interested in seeing a PR to attempt a first pass.

@AsakusaRinne
Copy link
Collaborator Author

HI, sorry for the late reply. A bit busy these days. It will be great if you'd like to contribute for it!

I haven't had a design of it yet but here're something I will consider.

  1. In Llama Text Templater #715 a wrapper for the template in llama.cpp was introduced.
  2. llama.cpp-python has a chat template which may help you as a reference.
  3. Different templates are required by different models. So when doing this, it should be able to serve different formats while keep the API for users stable.

Please let me know if you need any help.

Copy link

This issue has been automatically marked as stale due to inactivity. If no further activity occurs, it will be closed in 7 days.

@github-actions github-actions bot added the stale Stale issue will be autoclosed soon label May 10, 2025
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issue will be autoclosed soon
Projects
None yet
Development

No branches or pull requests

2 participants