Skip to content
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

🗃️ Use specified data_collator in RLOOTrainer and PPOTrainer #2360

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

bartoszzuk
Copy link
Contributor

What does this PR do?

  • Use the specified data_collator instead of using hard-coded DataCollatorWithPadding
  • When no data_collator is specified use DataCollatorWithPadding (previous behavior)

Fixes #2354

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines.
  • Did you write any new necessary tests?

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.

@kashif
Copy link
Collaborator

kashif commented Nov 18, 2024

thanks @bartoszzuk perhaps it's better to set the self.data_collator to the default one if it is none and then use self.data_collator in the data loaders?

trl/trainer/rloo_trainer.py Outdated Show resolved Hide resolved
@kashif
Copy link
Collaborator

kashif commented Nov 18, 2024

you might need to run make precommit in the root of the TRL to fix styling

@HuggingFaceDocBuilderDev

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.

@bartoszzuk bartoszzuk changed the title Use specified data_collator in RLOOTrainer Use specified data_collator in RLOOTrainer and PPOTrainer Nov 18, 2024
@qgallouedec qgallouedec changed the title Use specified data_collator in RLOOTrainer and PPOTrainer 🗃️ Use specified data_collator in RLOOTrainer and PPOTrainer Nov 18, 2024
@kashif kashif merged commit e7870dd into huggingface:main Nov 18, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Use specified data_collator instead of hard-coding the option?
4 participants