[AutoProcessor] Correct AutoProcessor and automatically add processor…
Author: patrickvonplaten
Pull request: yes

… class.

This PR makes sure that all preprocessors, when saved automatically save the corresponding class to `preprocessing_config.json` so that they can always be loaded from `AutoProcessor` afterward. E.g. the following should always be possible:

```python
auto_processor.save_pretrained("./")
processor = AutoProcessor.from_pretrained("./")
```

Comments:
Error is unrelated

> Before being able to merge this PR, you will need to rebase and fix the docstrings. If we go with it that is.
> 
> I have a general issue with feature extractors and processors sharing the same config file. It implies every processor will always have a feature extractor but different modalities in the future might make us do processors with different components, none of them being a feature extractor.
> 
> Then why is the processor class saved in the feature extractor config and not the tokenizer config? This is all very weird and I think we should just have a separate processor config which would avoid passing the processor_class to a `sace_pretrained` call. This feels a bit too much like a hack.

I understand your view and I also don't think it's very clean, but I think it's better than the alternative because:

- 1.) One [can already load the preprocessor from the feature extractor](https://github.com/huggingface/transformers/blob/13504dcbea231d2cae701d1ffdeb0810d62aff81/src/transformers/models/auto/processing_auto.py#L156) which already made me adapt a lot of feature extractor configs: https://huggingface.co/microsoft/wavlm-large/blob/main/preprocessor_config.json . If we can load it from the feature extractor, then why not save it? Reverting this behavior is already quite a breaking change by now.

- 2.) I don't think a `processor.json` config will every have any real configuration arguments. IMO it'll always stay a wrapper for tokenizers, feature extractors, language models, ... => So IMO it's really just the `"processor_class"` argument that we need to save somewhere

- 3.) We also save `tokenizer_class` in the model's config (I guess that is a bit different because `config.json` is more of an "overall" configuration. I would also be fine with having a `processor_class` in `tokenizer_config.json` as well BTW

-4.) The naming of `FeatureExtractor` is maybe general enough to use it as a base class for all future processors of whatever modality? Thinking of Video, TextToSpeech, .... 

- 5.) The problem I'm having with creating a config class for a processor is the following: 
       - Do we add the whole config of the feature extractor and the tokenizer_json.config to the processor config as well? Then we will have duplicated configs in model repos (we can't really remove existing config from repos with a huge breaking change). It'll be a mess to have duplicated configs IMO. If we don't add the tokenizer and feature extractor config then we can't load a processor just from it's config and the config will IMO only ever have `processor_class` as an argument.
 
What do you think @sgugger ? Also curious to hear other thoughts on this. 

I think the role of processors isn't well defined enough to take a decision, and I think we don't necessarily align on their purpose.

If a `Processor` is an umbrella over tokenizers and feature extractors, in that the following are possible:

```
Processor
    | Tokenizer
    | Feature extractor

Processor
    | Feature extractor

Processor
    | Tokenizer
```

Then I think that there should either be a `processor_config.json` which saves the class of the `Processor`, or that the processor class should be saved in all its dependants (tokenizer and/or feature extractor). The latter being not adding a new file (yay!) but it isn't necessarily the cleanest.

If that's not the case, and `Processor` is really an umbrella just over the feature extractor, so that only the following is possible:
```
Processor
    | Feature extractor
    | Tokenizer

Processor
    | Feature extractor
```

Then I understand why you'd want to only save it in the feature extractor configuration. But if that's the case, I wonder why there's a need to have a `Processor` at all, and why the feature extractor couldn't just contain a `tokenizer` alongside everything else that it does. If processor == feature extractor but serves as an additional abstraction so that the API is cleaner, then I also understand where you're coming from @patrickvonplaten.


> 1.) One can already load the preprocessor from the feature extractor 

I don't think that's always possible. It works for Wav2Vec2-style models where the tokenizer is only used to decode the outputs, but for some model, the feature extractor **and** the tokenizer are both needed. So in that case, loading from just the feature extractor will fail.

> 2.) I don't think a processor.json config will every have any real configuration arguments. IMO it'll always stay a wrapper for tokenizers, feature extractors, language models, ... => So IMO it's really just the "processor_class" argument that we need to save somewhere

I agree with that comment, and it also does not make sense to put everything in `processor.json` since we want to be able to load the feature extractor/tokenizer separately. In this case the `processor_class` should be saved in the tokenizer config, feature extractor config and model config if accessible, just to make sure the info is widely available.

But this only works for as long as we have processors that are just grouping together some modality-specific processing components and do not need any config of their own. Maybe in the future we will have some processors that need a config? Not sure. We can certainly re-open this debate only when the case arises and try to avoid a processor config for now.

> 4.) The naming of FeatureExtractor is maybe general enough to use it as a base class for all future processors of whatever modality? Thinking of Video, TextToSpeech, ....

I think we said we would keep feature extractor modality-specific and that processors are specifically multi-modal, so we will always need processors.

So to summarize: okay for now with this PR but we also save the `processor_class` in the tokenizer config (and look for it there if we don't find a feature extractor). Note that the `save_pretrained` method of all the processors could also be abstracted in a base class if we keep the list of "processing_blocks" names (for now almost always tokenizer and feature_extractor) in a class attribute.

Last bit: to remove the hack of passing a processor_class to the `save_pretrained` methods, can we just add it as an attribute of the feature extractor/tokenizer config? It doesn't make sense for either `FeatureExtractorMixin.save_pretrained` or `PreTrainedTokenizerBase.save_pretrained` to have this argument.

Added it to the tokenizer config now as well :-)

> Looks good! Let me know if you need an extra pair of hands for updating the old configs :)

Think it's fine to leave them as is :-)

Merging this as it's blocking me and I think @LysandreJik is fine with the solution as well. 

@LysandreJik, please let me know if you would like to change something here before the next release.