Attributes explicitly defined in model configurations are now overridden by the default type.
Author: LysandreJik
Pull request: no

## Environment info
<!-- You can run the command `transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->

- `transformers` version: 4.11.0.dev0
- Platform: Linux-5.14.11-arch1-1-x86_64-with-glibc2.33
- Python version: 3.9.7
- PyTorch version (GPU?): 1.9.1+cu102 (True)
- Tensorflow version (GPU?): 2.6.0 (False)
- Flax version (CPU?/GPU?/TPU?): 0.3.5 (cpu)
- Jax version: 0.2.21
- JaxLib version: 0.1.71
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>

## The issue

The issue is made visible from the introduction of parameter setters in https://github.com/huggingface/transformers/pull/13026.

This PR moved the initialization of the parent object to be the last statement of the configuration creation - while this could be benign, it isn't due to the fact that some arguments are defined both in the model configuration and in the upstream configuration.

Such an example is the FSMT configuration. It defines the generate arguments here:

https://github.com/huggingface/transformers/blob/408b2d2bd08f667cf4154730cc323c4e49657eed/src/transformers/models/fsmt/configuration_fsmt.py#L183-L185

At the end of the method, it initializes the parent configuration *without* passing the parameter:

https://github.com/huggingface/transformers/blob/408b2d2bd08f667cf4154730cc323c4e49657eed/src/transformers/models/fsmt/configuration_fsmt.py#L199-L208

Finally, in the parent configuration, the `num_beams` is set once again:

https://github.com/huggingface/transformers/blob/408b2d2bd08f667cf4154730cc323c4e49657eed/src/transformers/configuration_utils.py#L264

This is an issue now as this overrides the previously set `num_beams` to be 1. The issue wasn't caught before because the superclass initialization happened at the beginning, being overridden by the parameters afterwards. This is not the case anymore.

This makes the following test fail: `tests/test_modeling_fsmt.py -k test_translation_direct_2_en_de`.

IMO the issue comes from the redefinition of arguments in the FSMT configuration which should not be done as the superclass will already correctly define these arguments given the kwargs. The simplest patch for this (apart from making sure that the parameters are only set once) would be to make sure all previously applied parameters are taken into account by the superclass by adding the following statement to the initialization of the `PretrainedConfig` superclass:

```diff
[...]

    def __init__(self, **kwargs):
+        kwargs = {**kwargs, **self.__dict__}

        # Attributes with defaults
        self.return_dict = kwargs.pop("return_dict", True)
[...]
```

WDYT? cc @sgugger @stas00 @nreimers @patrickvonplaten 

The cleanest solution would be to make sure that all parameters are only set once, however, which is slightly harder to test.

### Reproducible code sample:

```py
from transformers import AutoTokenizer, FSMTForConditionalGeneration

pair = "en-de"


text = {
    "en": "Machine learning is great, isn't it?",
    "ru": "Машинное обучение - это здорово, не так ли?",
    "de": "Maschinelles Lernen ist großartig, oder?",
}

src, tgt = pair.split("-")
print(f"Testing {src} -> {tgt}")
mname = f"facebook/wmt19-{pair}"

src_text = text[src]
tgt_text = text[tgt]

tokenizer = AutoTokenizer.from_pretrained(mname)
model = FSMTForConditionalGeneration.from_pretrained(mname)
print(model.config)
input_ids = tokenizer.encode(src_text, return_tensors="pt")

outputs = model.generate(input_ids)
decoded = tokenizer.decode(outputs[0], skip_special_tokens=True)
assert decoded == tgt_text, f"\n\ngot: {decoded}\nexp: {tgt_text}\n"
```

Comments:
Interesting bug! I'm a bit afraid of the solution you propose @LysandreJik so I would prefer adding a strong test that check every common attribute can be properly set on every config. That way it would highlight the problems we can have in specific configs and we can fix them easily.

For instance, adding this to the config common tests:
```py
    def check_config_arguments_init(self):
        config = self.config_class(num_beams=3)
        self.parent.assertEqual(config.num_beams, 3)
```
reproduces the bug and creates a failure in the tests of FSMT. We can loop that test over a big dictionary containing all the common config attributes as keys and some values distinct from the defaults as values.

A few meta comments specific to FSMT:
- It's very possible that FSMT wasn't written in the best way - I was modeling after Bart - and there must have been some thing I couldn't figure out and had to invent a workaround. So please don't hesitate to adjust it to the better way.
- @patil-suraj refactored FSMT https://github.com/huggingface/transformers/pull/11218 but his PR didn't get merged because it introduced a speed and memory regression (which we suspect is possibly an issue in all recently refactored Bart-base models) and nobody had a chance to investigate / resolve
- Further, FSMT in master has received mods since Suraj's PR https://github.com/huggingface/transformers/commits/c8be8a9adb218ecc593c687020e952554a5a55b5/src/transformers/models/fsmt


I highly doubt the bug is only in FSMT @stas00 The fact that #13026 moved all the super calls at the end of the configuration init has probably created multiple instances of it. It's just FSMT had good tests that showed us the bug :-)

wrt introduction of config setters, I missed that change - we also have/use `config.update` that does the same as a setter - so now we have 2 ways of doing setters? `update` is nice when pushing a large multi-arg change.

One solution would also be to move back the super call at the start of the configuration init. This would reduce the risk of errors / that parameters are set twice as for FSMT.

I moved the call to `super().__init__` to the end so that common parameters (like hidden_size for `GPT2Config`) can correctly be set. But this can could be split in two call:

```python
class MyConfig(PretrainedConfig):
	def __init__(num_beams=5, **kwargs):
		 super().__init__(only=fixed, parameters=are_passed)  #Remove **kwargs from the initial __init__ call
		 
		 # ... Custom parameter setting for MyConfig 
		 
                 # Now handle all remaining parameters
		 super().set_common_attribute(**kwargs)
```

What do you think?

This seems clean to me, and since we now have a strong test checking we can properly set the common attributes, I won't worry over things breaking again with this change :-)

I like this proposition too @nreimers!