Breaking-change behavior in BERT tokenizer when stripping accents
Author: bryant1410
Pull request: no
Labels: name: wontfix

name: Core: Tokenization

# 🐛 Bug

## Information

Model I am using (Bert, XLNet ...): Bert (could happen with other ones, don't know)

Language I am using the model on (English, Chinese ...): English

The problem arises when using:
* [ ] the official example scripts: (give details below)
* [x] my own modified scripts: (give details below)

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: (give the name)
* [x] my own task or dataset: (give details below)

## To reproduce

```python
from transformers import AutoTokenizer

pretrained_model_name = "bert-base-cased"

fast_tokenizer = AutoTokenizer.from_pretrained(pretrained_model_name)
slow_tokenizer = AutoTokenizer.from_pretrained(pretrained_model_name, use_fast=False)

text = "naïve"

assert fast_tokenizer.encode(text) == slow_tokenizer.encode(text)
```

With the slow, it only strips accents if lowercase is enabled (maybe a bug?):

https://github.com/huggingface/transformers/blob/e67676424191e5935362e5fe7e04b5c317d706a9/src/transformers/tokenization_bert.py#L346

With the fast one, it'd never strip accents:

https://github.com/huggingface/tokenizers/blob/python-v0.5.0/bindings/python/tokenizers/implementations/bert_wordpiece.py#L23

https://github.com/huggingface/transformers/blob/e67676424191e5935362e5fe7e04b5c317d706a9/src/transformers/tokenization_bert.py#L557-L565

I'd be cool to have that flag also, in both tokenizers.

Finally, this warning seems odd for the simple code from above:

```pycon
>>> assert fast_tokenizer.encode(text) == slow_tokenizer.encode(text)
Disabled padding because no padding token set (pad_token: [PAD], pad_token_id: 0).
To remove this error, you can add a new pad token and then resize model embedding:
	tokenizer.pad_token = '<PAD>'
	model.resize_token_embeddings(len(tokenizer))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError
```

Maybe here the `if pad_to_max_length` should be nesting the rest of the if?

https://github.com/huggingface/transformers/blob/e67676424191e5935362e5fe7e04b5c317d706a9/src/transformers/tokenization_utils.py#L80-L95

Didn't check in the other transformer models.

## Expected behavior

1. The 2 tokenizer outputs (slow and fast) should be the same.
2. The tokenizers should allow you to choose if to strip accents or not.
3. That warning shouldn't appear, IMHO.

## Environment info

- `transformers` version: 2.5.0
- Platform: Linux-4.15.0-76-generic-x86_64-with-debian-buster-sid
- Python version: 3.7.4
- PyTorch version (GPU?): 1.4.0 (True)
- Tensorflow version (GPU?): 2.0.0 (False)
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No

Comments:
Yeah, I found the same problem in my code. The "encode" won't add padding even "pad_to_max_length = True".

HI @bryant1410, 

Thanks for reporting the issue. The parameter `strip_accents` was indeed enabled on `BertTokenizerFast`. 

I've a PR exposing the missing parameters https://github.com/huggingface/transformers/pull/2921, it will land soon on master and will be included in the first maintenance release of 2.5 

I see, thanks! There's an incompatibility still though, which is that you can choose if to strip accents in the fast tokenizers but you can't control that in the previous tokenizers. I believe this should be fixed as well.

And be aware that, IIRC, this is still a breaking change, because in the previous tokenizers you would get stipped accents by default in one way but now it seems to behave in a different way by default.

I don't know if this also the case for the other params added in #2921, and for other models apart from BERT.

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.


Please don't close it as this is an important issue.

Same one reported by @stefan-it, @n1t0 ?

Yes same one. Stripping accents is happening only when `do_lower_case=True` for slow tokenizers, and there is no way at the moment to change this behavior.

We can probably add an explicit option for this on slow tokenizers, and specify the default values in the configs.

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.


Don't close it!! I want to have control of striping accents when tokenizing