[RewardTrainer] Tokenize inputs within trainer #2102
Conversation
|
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. |
README.md
Outdated
| preprocess_function, | ||
| batched=True, | ||
| ) | ||
| dataset = dataset.map(maybe_apply_chat_template, fn_kwargs={"tokenizer": tokenizer}) |
There was a problem hiding this comment.
what about adding it in the trainer as well?
There was a problem hiding this comment.
Yes, I think this could be nice to make it consistent with the SFTTrainer! I'll push a change and fix the tests.
We should later apply this to the other trainers
|
Can you add/modify the tests? You should be able to use |
Co-authored-by: Quentin Gallouédec <[email protected]>
|
@qgallouedec I've refactored the tests to mostly use the |
|
|
||
|
|
||
| class RewardTrainerTester(unittest.TestCase): | ||
| def setUp(self): | ||
| self.model_id = "hf-internal-testing/tiny-random-LlamaForCausalLM" |
There was a problem hiding this comment.
I think we should gradually move towards testing the most popular LLM architectures instead of relying on gpt2 which has a bunch of annoying things like a missing PAD token
|
LGTM! |
What does this PR do?
This PR aligns the
RewardTrainerwith the other TRL trainer to apply tokenization within the trainer itself. This has the nice effect of simplifying the example script significantly.The training logs before/after this PR look within noise from random seed IMO
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines.
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.