Pipeline: no side-effects on model.config and model.generation_config 🔫

[gante]

What does this PR do?

Fixes #33398 with two changes (and corresponding regression-preventing tests):

• [major change] Pipelines now hold their own generation_config. Modifications to model.config or model.generation_config were redirected to the pipeline's generation_config, and thus avoiding side-effects on the model.

from transformers import pipeline, T5ForConditionalGeneration, AutoTokenizer
import tempfile

model = T5ForConditionalGeneration.from_pretrained("t5-small")
pipe = pipeline(
    model=model,
    tokenizer=AutoTokenizer.from_pretrained("t5-small", model_max_length=100),
    task="translation_en_to_de"
)

with tempfile.TemporaryDirectory() as tmpdirname:
    model.save_pretrained(tmpdirname)
    model_2 = T5ForConditionalGeneration.from_pretrained(tmpdirname)

# Prior to this PR, `model_2` != `model` because the pipeline initialization changed `model.config`!

• [minor change] in Forbid PretrainedConfig from saving generate parameters; Update deprecations in generate-related code 🧹  #32659, we started moving custom generate parameterization in model.config to model.generation_config when we called model.save_pretrained(). When we moved a parameter, model.config set that parameter to None. This None was not being properly ignored due to the legacy code path (parameterization in model.config takes precedence when the user never explicitly defined a GenerationConfig). After this PR, None fields are ignored by the generation config.

[LysandreJik]

Thanks! The changes make sense to me; I'd like to have @Rocketknight1's opinion as well

[Rocketknight1]

This approach makes sense to me! I don't think breaking that dark pattern is too much of a problem.

The only question, though, is whether it would be better to just get rid of those side effects in the pipelines, rather than doing this copy to patch over the problem. Do you know how many pipelines do this?

[gante]

The only question, though, is whether it would be better to just get rid of those side effects in the pipelines, rather than doing this copy to patch over the problem. Do you know how many pipelines do this?

@Rocketknight1 agreed 🙌

You raised a good question -- in a rush to get the problem solved, I forgot to see the forest from the trees. From a quick check, only pipelines that call generate actually have this pattern, where config is modified to control the generate call. Good thing we have created a tool precisely to avoid this pattern, the generation_config! :D

As such, instead of doing this copy to patch over the problem, I think we can instead execute the following plan:

1. Text generation pipelines hold their own generation config
2. We apply the task-specific parameters (sometimes present in model.config, e.g. here) exclusively on the pipeline's generation_config. In other words, we keep the model's config and generation_config untouched
3. store prefix (an item that might present in the task-specific parameters but is not present in GenerationConfig) separately as a pipeline attribute
4. redirect all references to generation parameters to the pipeline's generation_config
5. we pass the pipeline's generation_config to generate in all pipelines

WDYT?

[Rocketknight1]

Yes, that sounds good, and a lot cleaner!

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

[gante]

@Rocketknight1 v2 is ready for a review 🤗

[Rocketknight1]

Yes, LGTM now! I think this approach is way cleaner.