Repository navigation
[core] Officially Support Reward Modeling - #303
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
lvwerra
left a comment
There was a problem hiding this comment.
Generally looks really good and clean to me. Left a few comments to try to make it a bit more user friendly.
|
Maybe @lewtun would also be interested to have a look to see if there is feedback from the H4 team. |
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
lewtun
left a comment
There was a problem hiding this comment.
Awesome feature @younesbelkada 🔥
I left some tiny nits and a feature request to compute accuracy by default :)
|
|
||
| ## Using the `RewardTrainer` | ||
|
|
||
| After standardizing your dataset, you can use the `RewardTrainer` as a classic HugingFace Trainer. |
There was a problem hiding this comment.
Maybe explain what format the raw dataset should have here? E.g. you could use samples of the StackExchange or Anthropic dataset (https://huggingface.co/datasets/Anthropic/hh-rlhf) as a guide
There was a problem hiding this comment.
I added few lines in 4bcd96e but not sure if what I said is 100% correct, would love to have a second look here!
There was a problem hiding this comment.
Will also add an example now - EDIT added it
| from peft import get_peft_model | ||
|
|
||
|
|
||
| class RewardTrainer(Trainer): |
There was a problem hiding this comment.
Since accuracy is the most common metric for evaluating reward models, would it make sense to provide it as a default in compute_metrics? E.g. something like this should work:
def compute_metrics(eval_pred):
predictions, _ = eval_pred
# Here, predictions is rewards_chosen and rewards_rejected.
# We want to see how much of the time rewards_chosen > rewards_rejected.
predictions = np.argmax(predictions, axis=0)
labels = np.zeros(predictions.shape)
return accuracy.compute(predictions=predictions, references=labels)There was a problem hiding this comment.
This would add evaluate as an additional dependency to the library, we can also have it as an optional dependency similar as peft! For me it's totally fine to have it as a core dependency, but l want to hear @lvwerra's opinion to make sure we are aligned on this
There was a problem hiding this comment.
Ah true, I think having it as an optional dep would be the way to go (unless evaluate is already so light that it's deps are covered by the trl core deps)
There was a problem hiding this comment.
Accuracy is not a very hard metric, maybe we can just build it from scratch here :)
Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
lvwerra
left a comment
There was a problem hiding this comment.
Just a few small comments, otherwise this is good to go!
| compute_metrics (`Callable[[transformers.EvalPrediction], Dict]`): | ||
| The metrics to use for evaluation. |
| eval_dataset, | ||
| tokenizer, | ||
| model_init, | ||
| compute_metrics, |
There was a problem hiding this comment.
i am not sure this works: if we overwrite the class method with our own metric the compute metrics in the parent class is never used, no?
what about defining a compute_accuracy function outside the class and pass if compute_metrics from the init is None
There was a problem hiding this comment.
Sounds like a great plan!
* v1 - add working version - add all possible tests - add docs * add some contents * clean up * fixes * patch test for now * fix test * clean up * fix * this time fix * Update docs/source/trainer.mdx Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com> * fixe * update * final changes * oops * Update docs/source/reward_trainer.mdx Co-authored-by: lewtun <lewis.c.tunstall@gmail.com> * Update docs/source/reward_trainer.mdx Co-authored-by: lewtun <lewis.c.tunstall@gmail.com> * Update docs/source/reward_trainer.mdx Co-authored-by: lewtun <lewis.c.tunstall@gmail.com> * switch to chosen / rejected * fixes * add example * add accuracy metric * pass PEFT config * refactor compute metrics --------- Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com> Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
What does this PR do?
With Reward modeling being an important piece of PPO algorithm, it would be cool to support an "official" RewardTrainer in
trl.The
RewardTrainersimply inherits fromtransformers.Trainer, but with some constraints. Users should be responsible to create a paired dataset that containsinput_ids_j,input_ids_k,attention_mask_j,attention_mask_k, if they want to use the defaultRewardDataCollatorWithPaddingdata collator.Also I propose to add the possibility to create the PEFT model under the hood, if a user passes a
PeftConfigto the Trainer.This PR adds a first version of it, adds also nice tests and cool documentation about that
TODO: update the README &
reward_trainer.mdxfilecc @lvwerra