BertTokenizer.save_pretrained() ignores do_lower_case
Author: yoptar
Pull request: no
Labels: name: wontfix

name: Core: Tokenization

# 🐛 Bug

## Information

When saving a tokenizer with the purpose of sharing, `init` arguments are not saved to a config.

## To reproduce

Steps to reproduce the behavior:

Initialize a tokenizer with `do_lower_case=False`, save pretrained, initialize from pretrained. The default `do_lower_case=True` will not be overwritten and further tokenization will be incorrect.

```python3
In[1]: import transformers
In[2]: tokenizer = transformers.BertTokenizer('my/model/vocab.txt', do_lower_case=False)
In[3]: tokenizer.basic_tokenizer.do_lower_case
Out[3]: False
In[4]: tokenizer.save_pretrained('dumped/model/')
Out[4]: 
('dumped/model/vocab.txt',
 'dumped/model/special_tokens_map.json',
 'dumped/model/added_tokens.json')
In[5]: tokenizer = transformers.BertTokenizer.from_pretrained('dumped/model/')
In[6]: tokenizer.basic_tokenizer.do_lower_case
Out[6]: True
```

## Expected behavior

```python3
In[1]: import transformers
In[2]: tokenizer = transformers.BertTokenizer('my/model/vocab.txt', do_lower_case=False)
In[3]: tokenizer.basic_tokenizer.do_lower_case
Out[3]: False
In[4]: tokenizer.save_pretrained('dumped/model/')
Out[4]: 
('dumped/model/vocab.txt',
 'dumped/model/special_tokens_map.json',
 'dumped/model/added_tokens.json')
In[5]: tokenizer = transformers.BertTokenizer.from_pretrained('dumped/model/')
In[6]: tokenizer.basic_tokenizer.do_lower_case
Out[6]: False
```

## Environment info
<!-- You can run the command `python transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->
     
- `transformers` version: 2.5.0


Comments:
When I copy/paste your code, I get this in `dumped/model/`:
`vocab.txt`, `tokenizer_config.json` and `special_tokens_map.json`

The tokenizer config does save the lowercase, and the output of the code is two `False`.

Below the code snippet I had to actually run it:

```py
import transformers
import os

tokenizer = transformers.BertTokenizer.from_pretrained('bert-base-cased', do_lower_case=False)
print(tokenizer.basic_tokenizer.do_lower_case)
os.makedirs("~/dumped/model", exist_ok=True)
tokenizer.save_pretrained('~/dumped/model')
tokenizer = transformers.BertTokenizer.from_pretrained('~/dumped/model')
print(tokenizer.basic_tokenizer.do_lower_case)

```

Hi @LysandreJik,

It does work when you initialize the model `.from_pretrained('bert-base-cased')`, because the pretrained tokenizer already has a configuration that is saved afterwards.

I was talking about a case when you do not have a config and load only the local `vocab.txt` file:
```python3
import os

from transformers import BertTokenizer

# Download the `bert-base-cased` tokenizer and initialized from pretrained
tokenizer = BertTokenizer.from_pretrained('bert-base-cased')
# A configuration is already there, `do_lower_case` is `False`
print(tokenizer.basic_tokenizer.do_lower_case)
os.makedirs("~/dumped/model", exist_ok=True)
# Save it locally
tokenizer.save_pretrained('~/dumped/model')

# We can see that the config file has data
with open("~/dumped/model/tokenizer_config.json") as f:
  print(f.read())

# Initialize as if we only have a local `vocab.txt` which is my case
tokenizer = BertTokenizer('~/dumped/model/vocab.txt', do_lower_case=False)
print(tokenizer.basic_tokenizer.do_lower_case)
tokenizer.save_pretrained('~/dumped/model')

# After saving the config is empty
with open("~/dumped/model/tokenizer_config.json") as f:
  print(f.read())

# And after initializing from pretrained `do_lower_case` is `True`
tokenizer = BertTokenizer.from_pretrained('~/dumped/model')
print(tokenizer.basic_tokenizer.do_lower_case)
```

Hmm, that makes sense. Indeed, that seems problematic. Thanks for opening this issue, we're looking into it!

@LysandreJik Looked a bit into it quickly, and here is the deal:

`.save_pretrained` from `BertTokenizer` is inherited from `PreTrainedTokenizer`, and save config based on the `self.init_kwargs` dict.

However, `do_lower_case` [is not passed to the `super().__init__()`](https://github.com/huggingface/transformers/blob/e9e6efdc452b74947d40a5a2e8af2fc444c63b5b/src/transformers/tokenization_bert.py#L177) in the Bert tokenizer.

Adding it to the `kwargs` passed to `super()` should do the trick. I can send a PR if that helps!




@RaphaelMeudec, it would not help: the config is empty even though `unt_token`, `sep_token`, etc., are passed to `super().__init__()`.
Also, I've tried it :slightly_smiling_face: 

@yoptar Indeed! What I can't explain is that `self.init_kwargs` is initialized [here](https://github.com/huggingface/transformers/blob/e9e6efdc452b74947d40a5a2e8af2fc444c63b5b/src/transformers/tokenization_utils.py#L327) but remains untouched in the rest of the code? 

I've been able to make your code work as expected by initializing `self.init_kwargs = kwargs` (instead of `{}`) and passing `do_lower_case=do_lower_case` in BertTokenizer `super()` resulting in:

```
False
{"do_lower_case": false, "max_len": 512}
False
{"unk_token": "[UNK]", "sep_token": "[SEP]", "pad_token": "[PAD]", "cls_token": "[CLS]", "mask_token": "[MASK]", "do_lower_case": false}
False
```

@LysandreJik Do you have more insights on how `self.init_kwargs` is modified in the current code?

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.