-
Notifications
You must be signed in to change notification settings - Fork 430
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
Adding generic preference dataset builder #1623
Adding generic preference dataset builder #1623
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/1623
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit adb8934 with merge base c5db813 (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
do you want to also enable packing for preference datasets? should be easy to test and there's nothing blocking it
@@ -111,3 +112,46 @@ def test_get_item(self, mock_load_dataset, dialogue, expected): | |||
prompt, label = ds[0]["rejected_input_ids"], ds[0]["rejected_labels"] | |||
assert prompt == expected_rejected_tokens | |||
assert label == expected_rejected_labels | |||
|
|||
def test_load_local_json(self): |
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.
nice, we hoenstly need more of these local dataset tests
"content": "What do I do when I have a hole in my trousers?", | ||
"role": "user" | ||
}, | ||
{ "content": "Take them off.", "role": "assistant" } |
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.
🫢 🫢 🫢
torchtune/datasets/_preference.py
Outdated
... split="train", | ||
>>> ) | ||
>>> tokenizer.decode(dataset[0]["chosen_input_ids"], skip_special_tokens=True) | ||
What do I do when I have a hole in my trousers?Fix the hole. |
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.
We should fix this trim whitespace issue at some point...
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.
actually renders for me as 'user\n\nWhat do I do when I have a hole in my trousers?assistant\n\nTake them off.'
with llama3 tokenizer and skip_special_tokens=True.
I'm actually pretty out of the loop on packed datasets, sorry. TLDR how to? |
just emulate the logic here and that's all you need, then expose the packed parameter: torchtune/torchtune/datasets/_instruct.py Line 257 in c5db813
then just run a DPO config with preference_dataset and use the override You could do this in a follow-up, but would be a nice boost to our RLHF recipes |
Looks suspiciously straightforward - will follow up with this. |
Context
What is the purpose of this PR? Is it to
I was writing the docs for the preference datasets but noticed we don't have a generic builder to point to, so, here we are.
Changelog
What are the changes made in this PR?
preference_dataset
builder which usesChosenToRejectedMessages
. I don't think it's worth adding the stack exchange message format as an option at this time.Test plan
Please make sure to do each of the following if applicable to your PR. If you're unsure about any one of these just ask and we will happily help. We also have a contributing page for some guidance on contributing.
pre-commit install
)pytest tests
pytest tests -m integration_test
UX
If your function changed a public API, please add a dummy example of what the user experience will look like when calling it.
Here is a docstring example
and a tutorial example