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

[bugfix] Fix DataCollatorForChatML unexpected generation prompt #2450

Merged
merged 10 commits into from
Dec 11, 2024

Conversation

NIL-zhuang
Copy link
Contributor

What does this PR do?

Fixes #2449

Fix DataCollatorForChatML unexpected generation prompt
DataCollatorForChatML will add generation prompt like <|start_header_id|>assistant<|end_header_id|> after model response, which is not suitable for instruction tuning, as is referred in #2449

Before submitting

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.

@kashif
Copy link
Collaborator

kashif commented Dec 11, 2024

thanks @NIL-zhuang great catch!

@HuggingFaceDocBuilderDev

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.

@qgallouedec
Copy link
Member

Do we test this collator somewhere?

@kashif
Copy link
Collaborator

kashif commented Dec 11, 2024

we do test it.. but more for the labels rather than the content of the ids... let me see if i can add a failing test

tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
@kashif
Copy link
Collaborator

kashif commented Dec 11, 2024

@qgallouedec added failng tests

tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
@kashif kashif added the 🐛 bug Something isn't working label Dec 11, 2024
@kashif kashif merged commit c9c4f18 into huggingface:main Dec 11, 2024
12 of 13 checks passed
yaricom pushed a commit to yaricom/trl that referenced this pull request Dec 11, 2024
…ingface#2450)

* [bugfix] Fix DataCollatorForChatML unexpected generation prompt

* Update utils.py

* Update test_utils.py

* Update tests/test_utils.py

* Update tests/test_utils.py

* Update tests/test_utils.py

* Update tests/test_utils.py

* Update test_utils.py

* Update tests/test_utils.py

* Update tests/test_utils.py

---------

Co-authored-by: Kashif Rasul <kashif.rasul@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataCollatorForChatML unexpected generation prompt
4 participants