Bert Checkpoint Breaks 3.02 -> 3.1.0 due to new buffer in BertEmbeddings
Author: Laksh1997
Pull request: no
Labels: name: wontfix

Hi,

Thanks for the great library. I noticed this line being added (https://github.com/huggingface/transformers/blob/v3.1.0/src/transformers/modeling_bert.py#L190) in the latest update.

It breaks checkpoints that were saved when this line wasn't there. 

```
	Missing key(s) in state_dict: "generator_model.electra.embeddings.position_ids", "discriminator_model.electra.embeddings.position_ids". 
```



Comments:
I understand it makes the code slightly cleaner; in terms of speed it is most likely negligible (compared to the embedding lookup, for example).

But not sure what to do now as all the pretrained models (that used a lot of compute to pretrain) don't work anymore in the new update.

Hey @Laksh1997 - note that this line does not break anything. You can neglect warnings about `position_ids` since those are created at instantiation. Will open a PR to fix the warning

@patrickvonplaten seems to break it for me:

```

16:43:52
Traceback (most recent call last):

16:43:52
File "/opt/conda/envs/py36/bin/transformervae", line 33, in <module>

16:43:52
sys.exit(load_entry_point('exs-transformervae', 'console_scripts', 'transformervae')())

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/click/core.py", line 829, in __call__

16:43:52
return self.main(*args, **kwargs)

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/click/core.py", line 782, in main

16:43:52
rv = self.invoke(ctx)

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/click/core.py", line 1259, in invoke

16:43:52
return _process_result(sub_ctx.command.invoke(sub_ctx))

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/click/core.py", line 1066, in invoke

16:43:52
return ctx.invoke(self.callback, **ctx.params)

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/click/core.py", line 610, in invoke

16:43:52
return callback(*args, **kwargs)

16:43:52
File "/app/transformervae/cli.py", line 355, in train

16:43:52
model = model_cls(hparams, pretrained_model=pretrained_model_path_or_config)

16:43:52
File "/app/transformervae/models/regression.py", line 35, in __init__

16:43:52
pretrained_model,

16:43:52
File "/app/transformervae/models/finetuning_model.py", line 37, in __init__

16:43:52
self.encoder, self.tokenizer = self.load_pretrained_encoder(pretrained_model)

16:43:52
File "/app/transformervae/models/finetuning_model.py", line 89, in load_pretrained_encoder

16:43:52
pl_model = AutoModel.load(pretrained_model)

16:43:52
File "/app/transformervae/models/automodel.py", line 98, in load

16:43:52
return model_cls.load(path)

16:43:52
File "/app/transformervae/models/base.py", line 229, in load

16:43:52
return cls.load_from_checkpoint(filepath)

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/pytorch_lightning/core/saving.py", line 169, in load_from_checkpoint

16:43:52
model = cls._load_model_state(checkpoint, *args, **kwargs)

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/pytorch_lightning/core/saving.py", line 207, in _load_model_state

16:43:52
model.load_state_dict(checkpoint['state_dict'])

16:43:52
File "/opt/conda/envs/py36/lib/python3.6/site-packages/torch/nn/modules/module.py", line 1045, in load_state_dict

16:43:52
self.__class__.__name__, "\n\t".join(error_msgs)))

16:43:52
RuntimeError: Error(s) in loading state_dict for ElectraLanguageModel:

16:43:52
Missing key(s) in state_dict: "generator_model.electra.embeddings.position_ids", "discriminator_model.electra.embeddings.position_ids".
```

Note, `generator_model.electra` is `ElectraModel`, which uses `BertEmbeddings`.

Can you send me a code snippet so that I can reproduce your error? 


It's a big library. But I can try to recreate in a Colab. One sec.

@patrickvonplaten Colab: https://colab.research.google.com/drive/167CwTImG5T-4c9xeIVEkH9Xrracbn30h?usp=sharing

Let me know if you can access?

It also breaks to me. The attribute embedding.position_ids can't be loaded if the model artifact is trained with v3.0.2. So it will raise an KeyError

Hey @Laksh1997, I can't access the notebook - could you make it public for everybody to see? :-) 

@patrickvonplaten apologies. Here is the script:

```python
!pip install transformers==3.0.2

from transformers import ElectraModel, ElectraConfig
import torch
import transformers

print(transformers.__version__)

model = ElectraModel(ElectraConfig())
state_dict = model.state_dict()
torch.save(state_dict, 'checkpoint.pt')
```

```python
!pip install transformers==3.1.0

from transformers import ElectraModel, ElectraConfig
import torch
import transformers

print(transformers.__version__)

model = ElectraModel(ElectraConfig())
state_dict = torch.load('checkpoint.pt')
model.load_state_dict(state_dict)

```

I encountered the same issue. Old checkpoints (3.0.2) can not be loaded in (3.1.0) due to KeyError.

@Barcavin @easonnie As a temporary fix, I've just reverted back to 3.0.2. @patrickvonplaten I am hoping something can be done !

Hi, while we work on patching this issue, you can still use version v3.1.0 by using the `from_pretrained` method. Taking @Laksh1997's example, you would do:

1. Save the checkpoint in `saved_model_location/pytorch_model.bin`

```py
from transformers import ElectraModel, ElectraConfig
import torch
import transformers

print(transformers.__version__)

model = ElectraModel(ElectraConfig())
state_dict = model.state_dict()
torch.save(state_dict, 'saved_model_location/pytorch_model.bin')
```

2. Load it using the method `.from_pretrained`

```py
from transformers import ElectraModel, ElectraConfig
import transformers

print(transformers.__version__)

model = ElectraModel.from_pretrained("saved_model_location", config=ElectraConfig())
``` 

You can also use the `load_state_dict` method with the `strict` option set to `False`:

```py
model.load_state_dict(state_dict, strict=False)
```

The reason this additional buffer is here now is due to this [PR](https://github.com/huggingface/transformers/pull/5773#issue-449530988). 

Is there a reason why you would use the `load_state_dict` instead of `from_pretrained`, as `from_pretrained` exists in part to prevent such issues from happening?

Hi @LysandreJik 

Thanks for the proposed solution. 

In my case, I am using Pytorch Lightning which has its own saving and loading infrastructure. Thus the `from_pretrained` method can't exactly be used.

The `strict` flag is a good patch for now.

I think, in general, when building on top of the library, for complex projects one cannot rely on `from_pretrained`, especially if using other ecosystems.

Using the `strict` flag can enable a number of errors to go undetected, so I would refrain from using it. I think the best solution is to use version 3.0.2 for already trained models until the fix comes out.

Any update on this @LysandreJik @patrickvonplaten ?

As the `torch.load` method in `strict` mode does not allow unexpected/missing keys, this is an issue that won't be resolved. Three options are available here:
- Use the recommended `from_pretrained` method, which exists specifically to work around this kind of issues
- Use the `torch.load` method with the `strict` flag set to `False`
- Pin to version v3.0.2 if none of these can be applied.

Minor changes in model infrastructure can unfortunately happen as we try to optimize for efficiency, which will lead to this kind of issues. We're internally working on having our models on the hub be versionable, which should solve most of these problems. It's at least a couple of months away, however.

@LysandreJik That is unfortunate that the library will probably have to be pinned, as the first two options are unviable for reasons described in this thread. Especially because pretraining large models is computationally quite expensive (100s of GPU hours)...

You can also use the work-around explained [here](https://github.com/huggingface/transformers/issues/6882#issuecomment-685509938) if you want to convert your weights to the updated architecture.

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.


Just wanted to add that there is another non-trivial reason why `from_pretrained` might not be useful in all cases: fine-tuning. If I fine-tune BERT's weights on a specific dataset, most likely I will have to use `load_state_dict` afterwards to use the new weights, rather than the original weights that `from_pretrained` would load.

@LysandreJik  @Laksh1997  Setting the [persistent flag ](https://pytorch.org/docs/master/generated/torch.jit.ScriptModule.html#torch.jit.ScriptModule.register_buffer)to False when registering the buffer will avoid adding it to the state_dict and can address the BC issue. 

Hello there, 

I encountered the same problem. I was using transformers version 4.7.0; but the checkpoint was trained with transformer 3.0.2. I just did `pip uninstall transformers`, and then `pip install transformers==3.0.2` for running the training. Presumably, you can try: `model.load_state_dict(state_dict, strict=False)` as well. However, I don't feel comfortable with the latter solution since I think that might affect the model performance in abstraction –since `position_ids` **might** be used by the model, and putting some random values when it's not present in pre-trained checkpoint might ruin the performance. So safer way is to down-grade the transformers, in my opinion. 

Hope this helps you out!

Can someone confirm if the `position_ids` are used by the model and by not loading it correctly would it affect the performance of the model in transfer learning or continuing to train or inference? Thank you

I think it's safe to use `model.load_state_dict(state_dict, strict=False)` if the only missing information is the `position_ids` buffer. This tensor is indeed used by the model, but it's just a constant tensor containing a list of integers from 0 to the maximum number of position embeddings. The tensor is first created in the constructor of the `BertEmbeddings` class, in this line:

https://github.com/huggingface/transformers/blob/fcf83011dffce3f2e8aad906f07c1ec14668f877/src/transformers/models/bert/modeling_bert.py#L182

As such, it's not really part of the optimizable parameters of the model. This means that it doesn't matter if `position_ids` is not available when calling `load_state_dict`, because the line above will create it anyway in the constructor with the required values.

> I think it's safe to use `model.load_state_dict(state_dict, strict=False)` if the only missing information is the `position_ids` buffer. This tensor is indeed used by the model, but it's just a constant tensor containing a list of integers from 0 to the maximum number of position embeddings. The tensor is first created in the constructor of the `BertEmbeddings` class, in this line:
> 
> https://github.com/huggingface/transformers/blob/fcf83011dffce3f2e8aad906f07c1ec14668f877/src/transformers/models/bert/modeling_bert.py#L182
> 
> As such, it's not really part of the optimizable parameters of the model. This means that it doesn't matter if `position_ids` is not available when calling `load_state_dict`, because the line above will create it anyway in the constructor with the required values.

Thank you very much @dfdazac for your detailed reply.