Fix _save_checkpoint for online methods#2288
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. |
qgallouedec
changed the title
_save_checkpoint for online methods
.github/workflows/tests-main.yml
Outdated
| # install PEFT & transformers from source | ||
| pip install -U git+https://github.com/huggingface/peft.git | ||
| pip install -U git+https://github.com/huggingface/transformers.git | ||
| pip install -U git+https://github.com/huggingface/transformers.git@f339042b0b8bdc0b57a70d37f67cafbea960a2ab |
There was a problem hiding this comment.
There is another recent breaking change. I set this commit to make the CI run anyway. I will fix the new breaking change in a followup PR
There was a problem hiding this comment.
Not for this PR. But do you think we should add a CI workflow that tests against our lowest supported version of transformers? This may have picked up the issue with the examples being broken on main when processing_class was added?
There was a problem hiding this comment.
Good point. For ref, #2298 aims to make tests clearer/cleaner. I'll extend the work with another PR that implements your suggestion.
There was a problem hiding this comment.
pip doesn't have an option to prefer the min version yet pypa/pip#8085
Let's do it manually
There was a problem hiding this comment.
There is another recent breaking change. I set this commit to make the CI run anyway. I will fix the new breaking change in a followup PR
This other breaking change is solved here: #2302
There was a problem hiding this comment.
add a CI workflow that tests against our lowest supported version of transformers
Done in #2303
|
If at least one of you @muellerzr @SunMarc can take a look please 🙏 |
muellerzr
left a comment
muellerzr
left a comment
There was a problem hiding this comment.
Thanks! Sorry that's a bit breaking (but also I suppose somewhat understandable since it's _save_checkpoint). Fix makes sense to me to deprecate on your own :)
* Update trainer_utils import and save strategy in online_dpo_trainer.py * fix back-compat for online-dpo * better comment * Update transformers dependency to commit f33904
5 participants
What does this PR do?
The change in huggingface/transformers#31817 is breaking for TRL since it removes
metricsfrom_save_checkpoint.We need to use the new style, using the new method
Trainer._determine_best_metric. We need to copy this method fromTrainerto ensure compatibly.Compatibility
Trainer._determine_best_metricintoOnlineDPOTrainerNote that once we've set the minimum transformers version to version > 4.46, we will be able to remove
OnlineDPOTrainer._determine_best_metric.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.