-
Notifications
You must be signed in to change notification settings - Fork 2.1k
💭 [Data] Fix DeepSeek-R1 case #3522
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
Conversation
…tes to full conversations
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Nice, just a quick question, this issue occurs with DeepSeek-v3 but not with DeepSeek-v3-0528, right? |
@qgallouedec checking! |
@qgallouedec yes correct no issue with DeepSeek-v3-0528 on main |
# Ensure that the prompt is the initial part of the prompt-completion string | ||
if "prompt" in example: | ||
error_message = ( | ||
"The chat template applied to the prompt + completion does not start with the chat template applied to " | ||
"the prompt alone. This can indicate that the chat template is not supported by TRL." | ||
"\n**Prompt**:\n{}\n\n**Prompt + Completion**:\n{}" | ||
) | ||
if "chosen" in example and not prompt_chosen.startswith(prompt): | ||
raise ValueError(error_message.format(prompt, prompt_chosen)) | ||
if "rejected" in example and not prompt_rejected.startswith(prompt): | ||
raise ValueError(error_message.format(prompt, prompt_rejected)) | ||
if "completion" in example and not prompt_completion.startswith(prompt): | ||
raise ValueError(error_message.format(prompt, prompt_completion)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't occur anymore
What does this PR do?
fixes the issue that:
add_generation_prompt=True
, DeepSeek's tokenizer would add<think>
tokens<think>
tokens were addedprompt_completion[len(prompt):]
, but since the strings didn't match at the beginning, it would fail thestartswith
checkWe now add
add_generation_prompt=False
to all calls that apply chat templates to full conversations (prompt + completion/chosen/rejected) to ensure consistency.Fixes #3490
Before submitting
Pull Request section?
to it if that's the case.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.