Mismatch between sentinel token IDs from T5 data collator and T5 tokenizer
Author: rahuln
Pull request: no

## Environment info

- `transformers` version: 4.10.3
- Platform: Linux-4.18.0-240.el8.x86_64-x86_64-with-glibc2.28
- Python version: 3.9.7
- PyTorch version (GPU?): 1.10.0 (True)
- Tensorflow version (GPU?): 2.6.0 (True)
- Flax version (CPU?/GPU?/TPU?): 0.3.5 (gpu)
- Jax version: 0.2.24
- JaxLib version: 0.1.73
- Using GPU in script?: Yes
- Using distributed or parallel set-up in script?: No

### Who can help

@patil-suraj @patrickvonplaten

## Information

I'm trying to use the [`run_t5_mlm_flax.py`](https://github.com/huggingface/transformers/blob/7db2a79b387fd862ffb0af72f7148e6371339c7f/examples/flax/language-modeling/run_t5_mlm_flax.py) script to do additional pretraining of T5, and I noticed something strange about the way the data collator adds mask/sentinel tokens. In [line 293](https://github.com/huggingface/transformers/blob/7db2a79b387fd862ffb0af72f7148e6371339c7f/examples/flax/language-modeling/run_t5_mlm_flax.py#L293) of `run_t5_mlm_flax.py`, the `create_sentinel_ids` function replaces the masked positions with the corresponding sentinel IDs as `sentinel_ids + self.tokenizer.vocab_size - 1`, which gives values of `32100, 32101, 32102, ...`. However, the sentinel tokens `<extra_id_0>, <extra_id_1>, <extra_id_2>, ...` in the tokenizer for `t5-base` have the token IDs `32099, 32098, 32097, ...`, which I'm getting from running the following:

```python
from transformers import AutoTokenizer
tokenizer = AutoTokenizer.from_pretrained('t5-base')
print(tokenizer.convert_tokens_to_ids(['<extra_id_0>', '<extra_id_1>', '<extra_id_2>', '<extra_id_99>']))
# prints:
# [32099, 32098, 32097, 32000]
```

The larger token IDs seem to work without error because the `T5ForConditionalGeneration` pretrained model has an extra 128 token embeddings (even though `tokenizer.vocab` gives a value of `32100`, which seems to be related to issue #4875), but I'm not sure if these are the same embeddings that were used for the sentinel tokens during the original pretraining. Is the script correct in replacing the mask tokens with token IDs starting from `32100`, even though they don't correspond to the `<extra_id_#>` tokens in the vocabulary?

Here's an example of the behavior of `create_sentinel_ids` alone:

```python
import numpy as np
from transformers import AutoTokenizer

def create_sentinel_ids(mask_indices, tokenizer):
    start_indices = mask_indices - np.roll(mask_indices, 1, axis=-1) * mask_indices
    start_indices[:, 0] = mask_indices[:, 0]
    sentinel_ids = np.where(start_indices != 0, np.cumsum(start_indices, axis=-1), start_indices)
    sentinel_ids = np.where(sentinel_ids != 0, (sentinel_ids + tokenizer.vocab_size - 1), 0)
    sentinel_ids -= mask_indices - start_indices
    return sentinel_ids

tokenizer = AutoTokenizer.from_pretrained('t5-base')
mask_indices = np.array([[0, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1]]).astype(bool)
print(create_sentinel_ids(mask_indices.astype(np.int8), tokenizer))
# prints:
# [[    0 32100     0     0 32101    -1     0 32102    -1    -1     0 32103]]
```

Comments:
Hey @rahuln, 

That's a very good question and thanks a lot for the in-detail issue! 
In short, you are right, there is a mismatch and it would be good if we could correct it together :-) 

What happened here is that when I was writing this script I started from the assumption that people will train their own tokenizer instead of using an official one as explained here: https://github.com/huggingface/transformers/tree/7db2a79b387fd862ffb0af72f7148e6371339c7f/examples/flax/language-modeling#train-tokenizer-2

Now when you follow this example and train your own tokenizer, then the resulting tokenizer has the following property:

```
tokenizer.vocab_size != len(tokenizer)
```

meaning that the `tokenizer`'s vocabulary size does **not** include the sentinel tokens.
You can verify that as follows run the following command:

```python
from transformers import AutoTokenizer

tokenizer = AutoTokenizer.from_pretrained("patrickvonplaten/norwegian-t5-base")
print(f"Vocab size: {tokenizer.vocab_size} vs. Length: {len(tokenizer)}") 
# you should get `Vocab size: 32003 vs. Length: 32103
```

Now if you do the same for an official tokenizer:

```python
from transformers import AutoTokenizer

tokenizer = AutoTokenizer.from_pretrained("t5-base")
print(f"Vocab size: {tokenizer.vocab_size} vs. Length: {len(tokenizer)}") 
# you should get `Vocab size: 32100 vs. Length: 32100`
```

meaning vocab size and length match up! 

The reason for this is that the `tokenizers` library changed quite a bit overtime and that creating a new t5 tokenizer from scratch now does not give the exact same tokenizer anymore as the one that was converted from `t5-base` in the beginning.

So the question is not how we can correct the script to make it work for all kinds of tokenizers - we know that `len(tokenizer)` will always give us the number of all tokens (vocab + sentinel), so we should probably go over this one. We also do know the number of extra tokens as it's saved under `tokenizer._extra_ids`. 

=> So to correct the script (and make it work for both cases), I think we should replace this line:

```python
 sentinel_ids = np.where(sentinel_ids != 0, (sentinel_ids + tokenizer.vocab_size - 1), 0)
```

by 

```
 sentinel_ids = np.where(sentinel_ids != 0, (sentinel_ids + len(tokenizer) - tokenizer._extra_ids - 1), 0)
```

This would correct your mismatch no? 

Would you be interested in opening a PR to solve the issue? :-)

Hi @patrickvonplaten, thanks for getting back to me on this! If I plug the second expression you listed above into the example I wrote, I get the extra token IDs as `32000, 32001, 32002, ...`, which when applying `tokenizer.decode` with the `t5-base` tokenizer gives the sentinel tokens in the reverse order: `<extra_id_99>, <extra_id_98>, <extra_id_97>, ...`. The expression that gives `<extra_id_0>, <extra_id_1>, <extra_id_2>, ...` in that order seems to be

```python
sentinel_ids = np.where(sentinel_ids != 0, (len(tokenizer) - sentinel_ids), 0)
```

Which gives the IDs `32099, 32098, 32097, ...`. However, I'm not sure if this now corresponds to the correct sentinel ID token names but the wrong embeddings (i.e., I'm not sure if embedding `32099` was the embedding originally used for the first sentinel token – `<extra_id_0>` – during pretraining, or if the names are just mismatched for some reason). Just want to make sure to resolve this issue, but happy to submit a PR once it's figured out!

I'm pretty confident that the models where trained with the sentinel tokens placed in ascending order `(32000, 32001, ...)` -> so the tokenizer's decoding is IMO wrong. However it doesn't really matter as the decoder is never used to decode the tokens. 

This being said IMO, the data should be processed as follows:

input: `Hello 32000 my 32001 is 32002 and 32003 have ....`

So I guess you uncovered two issues here: 

1. The T5 preprocessing script is not general enough which can be solved as explained above
2. The T5 tokenizers encode & decode the sentinel tokens in the wrong order. 

Regarding 2.) I'm gently pinging @craffel as well to make sure I'm correct by stating that T5 was pretrained with the sentinel tokens ids placed in ascending order - *i.e.* the word embedding corresponding to index 32000 was used as the first sentinel token, the word embedding corresponding to index 32001 as the second sentinel token, etc...

Hey @patrickvonplaten , the sentinels start from the end of the vocab (i.e. the first sentinel is vocab_size - 1) and descend: https://github.com/google-research/text-to-text-transfer-transformer/blob/main/t5/data/preprocessors.py#L2893

Thanks a lot @craffel! 

Ok in this case I think, we need to just adapt the Flax T5 training example script accordingly @rahuln - the tokenizers seem to be just right then :-)

Sounds good! Submitted a pull request (#14477) with the correct expression for the sentinel token IDs, let me know if I should make any other changes.

@patrickvonplaten Hello, I'm using T5 model for my own task. Considering that I want to explore the mlm rate (masked token rate), I set different mask_rate for T5 model. However, I have a question that the number of mask token for T5 is 100, what if I want to mask tokens that more than 100? 
Thanks and looking forward to your reply~

Hey @kangqiyue,

Please note that we like to keep GitHub related for issues of Transformers. Could you instead try to use the forum for such questions? :-) 

Forum link: https://discuss.huggingface.co/

> Hey @kangqiyue,
> 
> Please note that we like to keep GitHub related for issues of Transformers. Could you instead try to use the forum for such questions? :-)
> 
> Forum link: https://discuss.huggingface.co/

ok, I will do that. Thanks!