Skip to content

Conversation

@ngxson
Copy link
Collaborator

@ngxson ngxson commented Jul 23, 2024

Resolve #8655

Fix llama_chat_format_single incorrectly format system message.

Also added some logs and test cases for this.

The output with this PR:

[1721763727] formatted: [INST] You are an assistant

[1721763727] tokenize the prompt
[1721763727] prompt: "[INST] You are an assistant
"
[1721763727] tokens: [ '<s>':1, '[INST]':3, ' You':3213, ' are':1584, ' an':1420, ' assistant':27089, '':1010 ]

...

[1721763730] buffer: 'hello
'
[1721763730] formatted: 
hello
 [/INST]
[1721763730] input tokens: [ '':1010, 'hello':29706, '':1010, ' ':1032, '[/INST]':4 ]

@ngxson ngxson requested a review from ggerganov July 23, 2024 19:45
@github-actions github-actions bot added testing Everything test related examples labels Jul 23, 2024
Copy link
Contributor

@HanClinto HanClinto left a comment

Choose a reason for hiding this comment

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

Looks good to me!

The added test is very good. Without the fix applied, I can confirm that the problem is exhibited (and the test fails), and then after applying the fix, the test passes, and behavior seems to match what's expected.

Can't ask for more than that! Nice fix! :)

@ngxson ngxson changed the title fix llama_chat_format_single for mistral llama : fix llama_chat_format_single for mistral Jul 23, 2024


// test llama_chat_format_single for user message
std::cout << "\n\n=== llama_chat_format_single (user message) ===\n\n";
Copy link
Member

Choose a reason for hiding this comment

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

nit : prefer printf over std::cout, mainly for consistency with the rest of the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup that makes sense, I changed to printf : 20f56a0

Will merge after CI pass

@ngxson ngxson added the merge ready indicates that this may be ready to merge soon and is just holding out in case of objections label Jul 24, 2024
@ngxson ngxson merged commit 96952e7 into ggml-org:master Jul 24, 2024
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 27, 2024
* fix `llama_chat_format_single` for mistral

* fix typo

* use printf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples merge ready indicates that this may be ready to merge soon and is just holding out in case of objections testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Mistral-Nemo-Instruct Chat template seems to be applied completely wrong

3 participants