[Trainer] Allow passing image processor

[NielsRogge]

What does this PR do?

Fixes #29790. Also reported here: https://discuss.huggingface.co/t/vitimageprocessor-object-has-no-attribute-pad/32511.

Currently, passing an image processor to the Trainer is pretty hacky, as users need to do tokenizer=image_processor. 🤔 yes, that's right.

This PR adds the image_processor argument to the Trainer, such that the default data collator is used by default in case an image processor is passed.

To do:

• update example scripts to no longer use tokenizer=image_processor

[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.

[ArthurZucker]

Very good to have

[ArthurZucker]

might not be BC if people pass a ImageProcessor. Let's deprecate passing image processors checking the type

[NielsRogge]

If people currently pass an image processor as tokenizer, then they need to pass a data collator as otherwise they'll hit the issue mentioned at #29790 (as image processors typically don't have a pad method). So it doesn't break backwards

[amyeroberts]

Can we add enabling passing a feature_extractor and processor too?

[NielsRogge]

@ArthurZucker feel free to approve the PR, @amyeroberts I would add those in a separate PR

[ArthurZucker]

Okay, let's do a followup PR then for feature processor and else

[daniellok-db]

cc @NielsRogge @tomaarsen the addition of the new param to trainer_callback.CallbackHandler is currently causing problems for the setfit.Trainer class in the SetFit library. can we either make the param optional, or update setfit.Trainer to pass a value in for the new param?

[chenin-wang]

self.callback_handler = CallbackHandler(callbacks, self.model, self.model.model_body.tokenizer, None, None, None)

Can we add enabling passing a feature_extractor and processor too?

@ArthurZucker feel free to approve the PR, @amyeroberts I would add those in a separate PR

useful, waiting...

[chenin-wang]

cc @NielsRogge @tomaarsen the addition of the new param to trainer_callback.CallbackHandler is currently causing problems for the setfit.Trainer class in the SetFit library. can we either make the param optional, or update setfit.Trainer to pass a value in for the new param?

self.callback_handler = CallbackHandler(callbacks, self.model, self.model.model_body.tokenizer, None, None, None)

[chenin-wang]

@NielsRogge @ArthurZucker For Preprocessing classes. Suggest modifying the tokenizer parameter name instead of adding parameters. Consider changing tokenizer to processors.

[NielsRogge]

Yes @chenin-wang I was also considering this, rather than having various attributes for tokenizer, image processor, feature extractor and multimodal processor it probably makes more sense to just have a single argument called processor. Although that would be a breaking change since many people alreasy use tokenizer=...

Will await opinion of Arthur and Amy

[chenin-wang]

Yes @chenin-wang I was also considering this, rather than having various attributes for tokenizer, image processor, feature extractor and multimodal processor it probably makes more sense to just have a single argument called processor. Although that would be a breaking change since many people alreasy use tokenizer=...

Will await opinion of Arthur and Amy

You are right. Will change user habits.

[NielsRogge]

Reflecting more on this, I think the best way is to have a proper deprecation message.

Basically now whenever people pass the tokenizer argument, we should add a message saying "the tokenizer argument is going to be deprecated in v4.xx of Transformers and you need to update your code to pass processor instead."

=> this way people can safely update their code without having breaking changes from the start. Will open a follow-up PR for this.

[chenin-wang]

@NielsRogge @amyeroberts The problem remains. #30102 (comment)

[NielsRogge]

Hi @chenin-wang could you clarify your issue?

[chenin-wang]

@NielsRogge tokenizer=image_processor is a confusing API (as highlighted by a few issues from users) and considering we have more and more multimodal, audio and vision models increasingly out-of-date，what amyeroberts saied #30102 (comment).

[NielsRogge]

Yes @chenin-wang I agree. So indeed we should continue with #30102. However I'm thinking about the term preprocessor rather than processor, as processor is already used for multimodal processors like CLIP, BLIP-2, etc.

Not sure if people prefer processor=image_processor or preprocessor=image_processor.

Hence I'll ask the question to some HF members and see which term they prefer. Will then proceed.