Skip to content

Rename trainer arg tokenizer to processing_class#2162

Merged
qgallouedec merged 34 commits intomainfrom
tokenizer_to_processing_class
Oct 7, 2024
Merged

Rename trainer arg tokenizer to processing_class#2162
qgallouedec merged 34 commits intomainfrom
tokenizer_to_processing_class

Conversation

@qgallouedec
Copy link
Copy Markdown
Member

@qgallouedec qgallouedec commented Oct 3, 2024

What does this PR do?

Follows huggingface/transformers#32385
Fixes #2161

Ensure backward compatibility for DPO and SFT only

>>> from datasets import load_dataset
>>> from transformers import AutoModelForCausalLM, AutoTokenizer
>>> from trl import DPOConfig, DPOTrainer
>>> model = AutoModelForCausalLM.from_pretrained("Qwen/Qwen2.5-0.5B-Instruct")
>>> tokenizer = AutoTokenizer.from_pretrained("Qwen/Qwen2.5-0.5B-Instruct")
>>> dataset = load_dataset("trl-lib/Capybara-Preferences", split="train")
>>> training_args = DPOConfig(output_dir="Qwen2.5-0.5B-DPO")
>>> trainer = DPOTrainer(model=model, args=training_args, train_dataset=dataset, tokenizer=tokenizer)
/fsx/qgallouedec/miniconda3/envs/trl/lib/python3.11/site-packages/huggingface_hub/utils/_deprecation.py:101: FutureWarning: `tokenizer` is deprecated and will be removed in version 0.14.0 for `DPOTrainer.__init__`. Use `processing_class` instead.
  return f(*args, **kwargs)

TODO

  • BCO
  • CPO
  • DPO
  • GKD
  • KTO
  • Nash-MD
  • Online DPO
  • ORPO
  • PPOv2
  • Reward
  • RLOO
  • SFT
  • XPO

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.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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 qgallouedec linked an issue Oct 3, 2024 that may be closed by this pull request
4 tasks
@qgallouedec qgallouedec marked this pull request as ready for review October 4, 2024 12:24
def generate_completions(self, sampling: bool = False):
args = self.args
tokenizer = self.tokenizer
processing_class = self.processing_class
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I may be missing something but is this required? Cannot we just use self.processing_class?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right. Same for args. It will probably need some refactoring in the future

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've seen that in other places too so maybe there's a rationale that I don't see for that? Not sure, but sure we'll keep it in mind

qgallouedec and others added 6 commits October 4, 2024 16:36
Co-authored-by: Alvaro Bartolome <36760800+alvarobartt@users.noreply.github.com>
Co-authored-by: Alvaro Bartolome <36760800+alvarobartt@users.noreply.github.com>
Co-authored-by: Alvaro Bartolome <36760800+alvarobartt@users.noreply.github.com>
Co-authored-by: Alvaro Bartolome <36760800+alvarobartt@users.noreply.github.com>
@qgallouedec qgallouedec merged commit 47d08a9 into main Oct 7, 2024
@qgallouedec qgallouedec deleted the tokenizer_to_processing_class branch October 7, 2024 07:39
@yanan1116
Copy link
Copy Markdown

Traceback (most recent call last):
File "/workspace/trl/examples/scripts/sft.py", line 93, in
trainer = SFTTrainer(
File "/usr/local/lib/python3.10/dist-packages/huggingface_hub/utils/_deprecation.py", line 101, in inner_f
return f(*args, **kwargs)
File "/usr/local/lib/python3.10/dist-packages/transformers/utils/deprecation.py", line 165, in wrapped_func
return func(*args, **kwargs)
File "/workspace/trl/trl/trainer/sft_trainer.py", line 409, in init
super().init(
TypeError: Trainer.init() got an unexpected keyword argument 'processing_class'

@yanan1116
Copy link
Copy Markdown

trl version: 0.12.0.dev0

@BUILDERlym
Copy link
Copy Markdown

BUILDERlym commented Oct 8, 2024

Traceback (most recent call last):
File "/workspace/trl/examples/scripts/sft.py", line 93, in
trainer = SFTTrainer(
File "/usr/local/lib/python3.10/dist-packages/huggingface_hub/utils/_deprecation.py", line 101, in inner_f
return f(*args, **kwargs)
File "/usr/local/lib/python3.10/dist-packages/transformers/utils/deprecation.py", line 165, in wrapped_func
return func(*args, **kwargs)
File "/workspace/trl/trl/trainer/sft_trainer.py", line 409, in init
super().init(
TypeError: Trainer.init() got an unexpected keyword argument 'processing_class'

same issue. checked the local source code, indeed no argument 'processing_class'. why's that?
seems like current version still uses 'tokenizer'

@kashif
Copy link
Copy Markdown
Collaborator

kashif commented Oct 8, 2024

you need to use the main version of transformers

@BUILDERlym
Copy link
Copy Markdown

you need to use the main version of transformers

got it, thanks

ChanderG added a commit to foundation-model-stack/hf-resource-scanner that referenced this pull request Nov 4, 2024
we use positional args to obtain model and optimizer, however, this
has the un-needed tokenizer argument between them

due to recent changes, the tokenizer arg is now renamed to
processing_class, see:
+ huggingface/trl#2162
+ huggingface/transformers#32385
leading to unexpected breakdown of scanner

the line relevant to us is here:
https://github.com/huggingface/transformers/blob/main/src/transformers/trainer_callback.py#L523

since we anyway don't depend on this arg, switch out to using model
and opt from the kwargs
yxliu-TAMU pushed a commit to mincheolseong/ECEN743-GRPO-Project-Proposal that referenced this pull request Apr 20, 2025
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.

AttributeError: property 'tokenizer' of 'DPOTrainer' object has no setter

7 participants