Pipeline: use tokenizer pad token at generation time if the model pad token is unset.

[gante]

What does this PR do?

Fixes #29378

The tagged issue describes the problem, the title describes the fix :D

Example of a script that no longer emits a warning, after this PR:

from transformers import AutoTokenizer, AutoModelForCausalLM, pipeline

model = AutoModelForCausalLM.from_pretrained("openai-community/gpt2")
tokenizer = AutoTokenizer.from_pretrained("openai-community/gpt2")
tokenizer.pad_token_id = tokenizer.eos_token_id

llm = pipeline(task='text-generation', model=model, tokenizer=tokenizer, framework='pt')
response = llm('The capital of France ')

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

[amyeroberts]

Thanks for working on this!

Just some qs about the implementation

[gante]

minimum_tokens is an unused internal variable, probably a legacy version of min_length.

Initially, I removed it from the signature of the private _forward, as I was touching it. Then, I realized we could remove all traces since it is unused :)

[gante]

Note regarding this file's diff, also applicable to the diff in src/transformers/pipelines/image_to_text.py:

The conventional strategy to pass kwargs to generate is through **forward_params. Previously in this file, the generation kwargs were held as forward_params["generate_kwargs"], which prevented the use of the conventional strategy. There isn't really a reason to hold these kwargs separately, generate is the only sink for kwargs in models that can generate. Models that can't generate will should throw an exception regardless of the container for kwargs. As such, this diff aims at minimizing the difference for generate parameterization across pipelines :)

[amyeroberts]

I'm not a fan of this - it's far cleaner to clearly outline what are generate kwargs and what are not. In the current pipelines the models might be the only sink, but that's not guaranteed.

[amyeroberts]

Actually - I realise what I've said about the forward kwargs is wrong here - we can just assume they're passed to the model. In this case, my preference is to still have "generate_kwargs" explicitly in the forward_kwargs, but I don't feel strongly and don't mind if you leave as-is

[gante]

I think we can agree that regardless of the pattern we choose here, it should be applied to all pipelines with generative capabilities for consistency. Based on this premise, enforcing a separation of generate_kwargs this exact way will break backward compatibility, i.e. the following would not be possible

from transformers import pipeline

llm = pipeline(task='text-generation', model="openai-community/gpt2")
response = llm('The capital of France ', max_length=50)

Nevertheless, I am aligned with you -- we should separate them! We can do it through generation_config.update(**kwargs), and perform the required validation with the aid of generation_config.validate(). One of the requirements to do so is to have a single big blob of keyword arguments to untangle, and thus these changes go in this direction.

Let me know if you agree, in which case I'll merge the PR and prepare this follow-up. [My instinct was to merge this PR now, but I've held it back -- I've merged too many not-100%-approved PRs recently 😉 ]

[amyeroberts]

Yeah, let's merge atm so this is unblocked and then we can iterate on something different :)

[gante]

@amyeroberts ready for a re-review :)

[amyeroberts]

Thanks for iterating on this!

Happy with the changes in general but I think we should maintain the clear separation between "generate_kwargs" and forward_kwargs. It makes it easier to understand what controls what in the pipeline