[save/load model] authorized keys, no save keys, etc.
Author: stas00
Pull request: no

We already have:

```
   # modeling_bart:
    authorized_missing_keys = [r"final_logits_bias", r"encoder\.version", r"decoder\.version"]
```
Once https://github.com/huggingface/transformers/pull/7224 is merged we will have:

```
class SomeModel(PretrainedFSMTModel):
    base_model_prefix = "model"
    authorized_missing_keys = [
        "model.encoder.embed_positions.weight",
        "model.decoder.embed_positions.weight",
    ]
    keys_to_never_save= [
        "model.encoder.embed_positions.weight",
        "model.decoder.embed_positions.weight",
    ]
```

I'd like to discuss several things:

1. let's pick consistent intuitive names for the group of these class variables - e.g. both ending with `_keys` or starting with `keys_` and use more descriptive mnemonics?

I suggest:

* `keys_to_ignore_on_load`
* `keys_to_ignore_on_save`

---------

2. why is the current implementation for `authorized_missing_keys` uses a regex search?

```
   # modeling_bart:
    authorized_missing_keys = [r"final_logits_bias", r"encoder\.version", r"decoder\.version"]
    [...]
   # modeling_utils:
            if cls.authorized_missing_keys is not None:
                for pat in cls.authorized_missing_keys:
                    missing_keys = [k for k in missing_keys if re.search(pat, k) is None]
```
when a simple direct comparison `k in list` would work just fine? i.e.: (untested)
```
if cls.authorized_missing_keys
    missing_keys = [k for k in missing_keys if k is not in cls.authorized_missing_keys]
```

it'd make the listing of the keys easier to write/read.
```
    authorized_missing_keys = ["final_logits_bias", "encoder.version", "decoder.version"]
```

-------------------

3. I think we may have an issue with `model.` prefix present in some saved model `state_dict`s and lacking in others - or is it a save/load issue where it has the prefix on the way out, but not back in? Note, that `authorized_missing_keys` doesn't have the `model.` prefix. Perhaps that's why the regex was used - to catch w/ and w/o the prefix?

So perhaps the normalization could happen in the core libs (load?) and the models listing special "needs" keys should all either have the prefix or not. 

----

Thank you for your input.


Comments:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


@LysandreJik, @sgugger - sorry didn't think to tag you when I initially posted this. Thoughts? Or just leave it as is?

For consistency's sake it would indeed be better. For backwards compatibility's sake, it would not be better.

However, I'm thinking that these attributes should be private, to some extent. They're not meant to be read/written to by users. 

I think we can rename these, and take the opportunity to prepend an underscore to say it's a private attribute. What do you think? However, keeping breaking changes in mind, it would be great to include this in the v4.0.0, which we should release tomorrow morning. Do you think you could propose a fix by then? If not, then let's aim for v5.0.0 instead.

Definitely no rush here, but if you want it now: https://github.com/huggingface/transformers/pull/8737

Good call on making those private!!!