Alpaca Dataset Updates and Fixes #303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for torchtune-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
facebook-github-bot
added
the
CLA Signed
kartikayk
requested review from
daniellepintz,
joecummings,
rohan-varma and
ebsmothers
ebsmothers
approved these changes
Feb 4, 2024
torchtune/datasets/alpaca.py
Outdated
Comment on lines
93
to
95
| instruction = self._data[index]["instruction"], | ||
| input = self._data[index]["input"], | ||
| output = self._data[index]["output"] |
There was a problem hiding this comment.
nit: could just define sample = self._data[index] to avoid multiple calls
torchtune/datasets/alpaca.py
Outdated
|
|
||
|
|
||
| class AlpacaDataset(Dataset): | ||
| """ | ||
| PyTorch Representation of the Alpaca Dataset | ||
| Support for the Alpaca dataset and it's variants from HuggingFace Datasets. |
There was a problem hiding this comment.
nit
Suggested change
| Support for the Alpaca dataset and it's variants from HuggingFace Datasets. | |
| Support for the Alpaca dataset and its variants from HuggingFace Datasets. |
| alpaca_dataset = datasets.get_dataset("alpaca", tokenizer=tokenizer) | ||
|
|
||
| # alpaca_dataset._data contains the raw data loaded from HF's dataset. We need the raw data | ||
| # to test the prompt generation since calling __get__item on the alpaca_dataset object will |
There was a problem hiding this comment.
nit
Suggested change
| # to test the prompt generation since calling __get__item on the alpaca_dataset object will | |
| # to test the prompt generation since calling __getitem__ on the alpaca_dataset object will |
| @patch("torchtune.datasets.alpaca.load_dataset") | ||
| def test_prompt_generation(self, load_dataset, tokenizer): | ||
| """ | ||
| Test the the prompt generation based on the alpaca template is correct. |
There was a problem hiding this comment.
nit
Suggested change
| Test the the prompt generation based on the alpaca template is correct. | |
| Test that the prompt generation based on the alpaca template is correct. |
|
Thanks so much @ebsmothers for the quick review! Addressed all comments. |
rohan-varma
reviewed
Feb 5, 2024
| ] | ||
|
|
||
| alpaca_dataset = datasets.get_dataset( | ||
| "alpaca", tokenizer=tokenizer, use_clean=True |
There was a problem hiding this comment.
Can we parametrize instead with the test_label_masking, as only difference is the use_clean flag?
| where `instruction`, `input`, and `output` are fields from the dataset. | ||
|
|
||
| Masking of the prompt during training is controlled by the `train_on_input` flag, which is | ||
| set to `True` by default (ref: https://github.com/tloen/alpaca-lora/blob/main/finetune.py#L49) |
There was a problem hiding this comment.
what are our thoughts on referring to reference implementations in torchtune? Not sure if citing them sort of implies that we as torchtune are sort of certifying that repo is a reference we endorse / want to compare against in an outward fashion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Our current Alpaca dataset implementation doesn't allow us to train on the inputs i.e. not mask the input during training. Looking at reference implementations, this is pretty common and the only way we can replicate training curves.
The class is also written in a way which doesn't allow the user to easily switch in and out the different variations of the alpaca datasets. Using the clean version of the dataset, allows the loss to go down faster.
In this PR, both of these features are added. Plus tests for the alpaca dataset are added.
Thanks @ebsmothers for helping find some of these issues!
Changelog
use_cleanflagtrain_on_inputflagTest plan
test_alpaca_datasetsucceeded.Comment on why we're changing the loss values in
test_finetune_llm.pyThe loss changes because we have a small difference in the input and label generation. This change is:
This creates a small difference in the output which results in changes in the loss: