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

🤝 Mixture of judges #2159

Merged
merged 46 commits into from
Nov 18, 2024
Merged

Conversation

gaetanlop
Copy link
Contributor

What does this PR do?

This PR adds the Mixture of judges part of the CGPO paper (https://arxiv.org/pdf/2409.20370). The judges are described in section 4.1.4 and the mixture of judges simply labels a generation as “violated” (0) if it fails any one of the constraint judgments and “satisfied” (1) otherwise.

Related to #2156

Before submitting

  • 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?

@kashif @lewtun

@gaetanlop
Copy link
Contributor Author

gaetanlop commented Oct 3, 2024

Still a draft PR Ready

@gaetanlop gaetanlop marked this pull request as ready for review October 4, 2024 01:01
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
@qgallouedec qgallouedec added the 👨‍⚖️ judge Related to judges label Oct 4, 2024
@qgallouedec
Copy link
Member

Thanks a lot @gaetanlop. Added some suggestion and open questions

@qgallouedec
Copy link
Member

Also, please make sure to run the pre-commits (make precommit)

gaetanlop and others added 5 commits October 4, 2024 16:54
Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com>
Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com>
@gaetanlop
Copy link
Contributor Author

Hey @qgallouedec, thanks for the review. I have added a gold_answers parameter to the judge function for both the base and MoJs class. Also, I have added a safety_judge and factuality_judge as described in the CGPO paper. They also have rule based judges, but in my opinion they are a little bit too tailored to specific tasks (coding/maths) to be added to the trl library. If you think the factuality and safety judges are also too specific to be in the lib I can remove them from the PR.

For the naming of the judges, let's do BaseBinaryJudge for the base class and AllTrueJudge for the moj following your suggestions?

trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
trl/trainer/judges.py Outdated Show resolved Hide resolved
@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.

@qgallouedec
Copy link
Member

LGTM, thanks @gaetanlop

@qgallouedec qgallouedec changed the title [CGPO] Mixture of judges 🤝 Mixture of judges Nov 18, 2024
@qgallouedec
Copy link
Member

We're having the issue

FAILED tests/test_nash_md_trainer.py::TestNashMDTrainer::test_nash_md_trainer_judge_training_1_conversational_prompt_only - ValueError: Cannot find pytorch_model.bin or model.safetensors in C:\Users\runneradmin\.cache\huggingface\hub\llm-blender\PairRM

that is mainly due to the fact that PairRM is requested for download simultaneously for different tests. This happens quite randomly, and is not related to any actual bug in the code base. I'll ignore it and merge.

@qgallouedec qgallouedec merged commit b5eabbe into huggingface:main Nov 18, 2024
4 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👨‍⚖️ judge Related to judges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants