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

FEAT: add tagging support to axolotl for DPOTrainer #1209

Merged
merged 2 commits into from
Jan 27, 2024

Conversation

filippo82
Copy link
Contributor

Description

Add axolotl tag to HuggingFace model cards created when fine tuning with RL (using the DPOTrainer).

Motivation and Context

This PR is required to extend #1004 to include the DPOTrainer. The relevant issue is #979

How has this been tested?

I have tested this by fine tuning a model and pushing it to the HuggingFace hub.

Types of changes

  • create a new AxolotlDPOTrainer class which subclasses the trl DPOTrainer class
  • move _sanitize_kwargs_for_tagging outside the AxolotTrainer class

Social Handles (Optional)

X/Twitter: @olgias

@filippo82
Copy link
Contributor Author

@younesbelkada @winglian This is a follow up for #1004.

I am not entirely convinced by my solution though:

  • probably _sanitize_kwargs_for_tagging can be move to utility module
  • there is currently code duplication (the two def push_to_hub(self, *args, **kwargs) -> str:) inside AxolotlTrainer and AxolotlDPOTrainer)

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! Note you can also call model.add_model_tags() now with the newest transformers release but this solution looks great

https://huggingface.co/docs/transformers/v4.37.1/en/main_classes/model#transformers.PreTrainedModel.add_model_tags

@filippo82 filippo82 force-pushed the add-axolotl-tag branch 2 times, most recently from 9a317c6 to 23f07d6 Compare January 26, 2024 19:44
@filippo82
Copy link
Contributor Author

Thank you @younesbelkada 🙏🏻

I rebased the branch and force-pushed a last change to fix a linting issue which was causing a test to fail.

@winglian
Copy link
Collaborator

@filippo82 I've fixed the issue and rebased again for you

@winglian winglian merged commit 18f8119 into axolotl-ai-cloud:main Jan 27, 2024
7 checks passed
@filippo82 filippo82 deleted the add-axolotl-tag branch January 27, 2024 09:31
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.

None yet

3 participants