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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] ArgillaTrainer - allow passing initialized model & tokenizer #3631

Closed
tomaarsen opened this issue Aug 25, 2023 · 0 comments · Fixed by #3751
Closed

[FEATURE] ArgillaTrainer - allow passing initialized model & tokenizer #3631

tomaarsen opened this issue Aug 25, 2023 · 0 comments · Fixed by #3751
Assignees
@tomaarsen
Labels
area: trainer Indicates that an issue or pull request is related to the Argilla Trainer type: enhancement Indicates new feature requests

Comments

@tomaarsen
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Related: #3467 (comment)

Describe the solution you'd like
I want users to be able to provide an initialized model and tokenizer:

model = AutoModel...
tokenizer = AutoTokenizer...

trainer = ArgillaTrainer(
    dataset=...,
    model=model,
    tokenizer=tokenizer,
    ...
)

Describe alternatives you've considered

  • Providing a model string and trying to hack together your desired model and tokenizer via trainer.update_config
  • Extending the ArgillaTrainer somehow to implement this feature.
  • Train separate from the ArgillaTrainer, but with the framework directly.

Additional context
Should be implemented after #3467

  • Tom Aarsen
@tomaarsen tomaarsen added type: enhancement Indicates new feature requests area: trainer Indicates that an issue or pull request is related to the Argilla Trainer labels Aug 25, 2023
@tomaarsen tomaarsen self-assigned this Sep 12, 2023
@tomaarsen tomaarsen mentioned this issue Sep 12, 2023
Merged
14 tasks
tomaarsen added a commit that referenced this issue Sep 15, 2023
@tomaarsen @davidberenstein1957
feat: Allow passing model and tokenizer to ArgillaTrainer directly (#…
3aac61f 
…3751)

Hello!

# Description

Closes #3631.

This is important to give users freedom to very specifically set up
their tokenizer. This is required e.g. for SFT with TRL.

**Type of change**

- [x] New feature (non-breaking change which adds functionality)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**

Updated the relevant tests (TRL, Transformers) to also train with the
passed model & tokenizer.

**Checklist**

- [ ] I added relevant documentation
- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---

**TODO**:

- [x] CHANGELOG
- [x] Documentation
- [x] Double-check docstrings

---

- Tom Aarsen

---------

Co-authored-by: David Berenstein <david.m.berenstein@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tomaarsen tomaarsen

Labels
area: trainer Indicates that an issue or pull request is related to the Argilla Trainer type: enhancement Indicates new feature requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Allow passing model and tokenizer to ArgillaTrainer directly argilla-io/argilla
1 participant
@tomaarsen