Repository: huggingface/transformers
Pull request: ⚠️⚠️[`T5Tokenize`] Fix T5 family tokenizers⚠️⚠️

# What does this PR do?
Fixes the `T5Tokenizer` (not the fast one yet). (at the same time adresses part of https://github.com/huggingface/transformers/issues/11531)
When converting `UMT5` I created a reproduction snippet for any t5x model form the original repo. I realized that a very very small variation in the input completely changes the output for non-finetuned models. The issue lies with the way we process `<extra_id_xx>`.

Example:
```python
# t5-base tokenizer
>>> tokenizer.encode("<extra_id_0>. Hello", add_special_tokens = False)
[32099, 3, 5, 8774] # ['<extra_id_0>', ' ▁', '.', '▁Hello']
# seqio.SentencePieceVocabulary(vocab_path, extra_ids = 300)
>>> processor.encode("<extra_id_0>. Hello")
[32099, 5, 8774] # ['<extra_id_0>', '.', '▁Hello']

#after fix: 
>>> tokenizer.encode("<extra_id_0>. Hello", add_special_tokens = False)
[32099, 5, 8774] # ['<extra_id_0>', '.', '▁Hello']
``` 

The reason is that t5x wrapps arround `sentencepiece`, and [adds the extra id to the vocab](https://github.com/google/seqio/blob/4d3097973e9e24ec2963319ec3c5ff518811060f/seqio/vocabularies.py#L362), but they are not saved that way. 
We don't add them to the vocab, so when we tokenize, we split on special tokens, thus the sentencepiece model only sees: 
```python
>>> tokenizer.sp_model.encode(". Hello")
[273, 274, 9] 
```
While the original model never sees a `.` (or a lot of  other characters) alone, and thus we add an extra space...

This is a bug fix with regards to training, it is **breaking** in the sense that is should remove the space. 

TODO:
- [x] Extra checks should be added to make sure this does not add anything else (like stripping a ` `. This for example would break: ` tokenizer.encode(". Hello")` as it remove the prefix space that is normally added.


Changed files:
filename: src/transformers/models/t5/tokenization_t5.py
patch: @@ -19,11 +19,15 @@
 import re
 import warnings
 from shutil import copyfile
-from typing import Any, Dict, List, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
 
 import sentencepiece as spm
 
 from ...tokenization_utils import PreTrainedTokenizer
+
+
+if TYPE_CHECKING:
+    from ...tokenization_utils_base import TextInput
 from ...utils import logging
 
 
@@ -51,6 +55,8 @@
     "t5-11b": 512,
 }
 
+SPIECE_UNDERLINE = "▁"
+
 
 class T5Tokenizer(PreTrainedTokenizer):
     """
@@ -294,9 +300,17 @@ def __setstate__(self, d):
         self.sp_model = spm.SentencePieceProcessor(**self.sp_model_kwargs)
         self.sp_model.Load(self.vocab_file)
 
+    def tokenize(self, text: "TextInput", **kwargs) -> List[str]:
+        if not text.startswith(" "):
+            text = " " + text
+        return super().tokenize(text, **kwargs)
+
     def _tokenize(self, text: str) -> List[str]:
         """Take as input a string and return a list of strings (tokens) for words/sub-words"""
-        return self.sp_model.encode(text, out_type=str)
+        tokens = self.sp_model.encode(text, out_type=str)
+        if not text.startswith(" ") and tokens[0] == SPIECE_UNDERLINE:
+            tokens = tokens[1:]
+        return tokens
 
     def _convert_token_to_id(self, token):
         """Converts a token (str) in an id using the vocab."""

filename: tests/models/switch_transformers/test_modeling_switch_transformers.py
patch: @@ -1149,7 +1149,7 @@ def test_small_generate(self):
         model = SwitchTransformersForConditionalGeneration.from_pretrained(
             "google/switch-base-8", torch_dtype=torch.bfloat16
         ).eval()
-        tokenizer = AutoTokenizer.from_pretrained("t5-small")
+        tokenizer = AutoTokenizer.from_pretrained("t5-small", use_fast=False)
         model = model.to(torch_device)
 
         input_ids = tokenizer(
@@ -1160,24 +1160,24 @@ def test_small_generate(self):
         self.assertEqual(output_str, "drink.")
 
         input_ids = tokenizer(
-            "A <extra_id_0> walks into a bar a orders a <extra_id_1> with <extra_id_2> pinch of <extra_id_3>.",
+            "A <extra_id_0> walks into a bar and orders a <extra_id_1> with <extra_id_2> pinch of <extra_id_3>.",
             return_tensors="pt",
         ).input_ids.to(torch_device)
         sequences = model.generate(input_ids)
         output_str = tokenizer.batch_decode(sequences, skip_special_tokens=False)[0]
 
-        EXPECTED_OUTPUT = "<pad><extra_id_0> man<extra_id_1> beer<extra_id_2> a<extra_id_3> salt<extra_id_4>.
"
+        EXPECTED_OUTPUT = "<pad><extra_id_0> man<extra_id_1> beer<extra_id_2> a<extra_id_3> whiskey<extra_id_4>.
"
         self.assertEqual(output_str, EXPECTED_OUTPUT)
 
     def test_small_batch_generate(self):
         BATCH_SIZE = 4
         model = SwitchTransformersForConditionalGeneration.from_pretrained(
             "google/switch-base-8", torch_dtype=torch.bfloat16
         ).eval()
-        tokenizer = AutoTokenizer.from_pretrained("t5-small")
+        tokenizer = AutoTokenizer.from_pretrained("t5-small", use_fast=False)
 
         inputs = [
-            "A <extra_id_0> walks into a bar a orders a <extra_id_1> with <extra_id_2> pinch of <extra_id_3>."
+            "A <extra_id_0> walks into a bar and orders a <extra_id_1> with <extra_id_2> pinch of <extra_id_3>."
         ] * BATCH_SIZE
         encoded_input = tokenizer.batch_encode_plus(inputs, return_tensors="pt")
 

filename: tests/models/t5/test_tokenization_t5.py
patch: @@ -399,3 +399,35 @@ def test_get_sentinel_tokens_for_fasttokenizer(self):
     def test_get_sentinel_token_ids_for_fasttokenizer(self):
         tokenizer = T5TokenizerFast(SAMPLE_VOCAB, extra_ids=10)
         self.assertListEqual(sorted(tokenizer.get_sentinel_token_ids()), sorted(range(1000, 1010)))
+
+    def test_encode_extra_ids(self):
+        tokenizer = T5Tokenizer(SAMPLE_VOCAB, extra_ids=0)
+        tokenizer.add_special_tokens({"additional_special_tokens": ["<extra_id_0>"]})
+        tokenizer._create_trie(tokenizer.all_special_tokens)
+        # TODO ArthurZ the above is necessary as addedTokens / intialization sucks. Trie is not correctly created
+        # So the extra ids are split....
+
+        input_ids = tokenizer.encode(". Hello")
+        self.assertEquals(input_ids, [7, 4, 156, 86, 20, 2])
+        tokens = tokenizer.tokenize(". Hello")
+        self.assertEquals(tokens, ["▁", ".", "▁He", "ll", "o"])
+
+        input_ids = tokenizer.encode(" . Hello")
+        self.assertEquals(input_ids, [7, 4, 156, 86, 20, 2])
+        tokens = tokenizer.tokenize(" . Hello")
+        self.assertEquals(tokens, ["▁", ".", "▁He", "ll", "o"])
+
+        input_ids = tokenizer.encode("Hello, <extra_id_0>I")
+        self.assertEquals(input_ids, [156, 86, 20, 3, 999, 8, 2])
+        tokens = tokenizer.tokenize("Hello, <extra_id_0>I")
+        self.assertEquals(tokens, ["▁He", "ll", "o", ",", "<extra_id_0>", "▁I"])
+
+        input_ids = tokenizer.encode("Hello, <extra_id_0>,")
+        self.assertEquals(input_ids, [156, 86, 20, 3, 999, 3, 2])
+        tokens = tokenizer.tokenize("Hello, <extra_id_0>,")
+        self.assertEquals(tokens, ["▁He", "ll", "o", ",", "<extra_id_0>", ","])
+
+        input_ids = tokenizer.encode(" <extra_id_0> ,")
+        self.assertEquals(input_ids, [999, 3, 2])
+        tokens = tokenizer.tokenize(" <extra_id_0> ,")
+        self.assertEquals(tokens, ["<extra_id_0>", ","])  # spaces are eaten by rstrip / lstrip