Add dropout to WordpieceTokenizer and BPE
Author: vitaliyradchenko
Pull request: yes
Labels: name: wontfix

We can add dropout not only to model weights but and to a tokenizer. The paper, proposed by Ivan Provilkov (2019, https://arxiv.org/pdf/1910.13267.pdf), describes all benefits from this approach and shows that it's almost always better to use dropout during tokenization. (use only for training, for inference dropout should be equal to 0)

Example:
```
import transformers
tokenizer = transformers.RobertaTokenizer.from_pretrained("roberta-base")
tokenizer.tokenize("Dropout is very important")
# default value is 0
# ['Drop', 'out', 'Ġis', 'Ġvery', 'Ġimportant']
tokenizer.tokenize("Dropout is very important", dropout=0.1)
# ['Drop', 'out', 'Ġis', 'Ġvery', 'Ġimport', 'ant']
```

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=h1) Report
> Merging [#2313](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/81db12c3ba0c2067f43c4a63edf5e45f54161042?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `84%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/2313/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2313      +/-   ##
==========================================
- Coverage   73.54%   73.53%   -0.01%     
==========================================
  Files          87       87              
  Lines       14789    14796       +7     
==========================================
+ Hits        10876    10880       +4     
- Misses       3913     3916       +3
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/tokenization\_gpt2.py](https://codecov.io/gh/huggingface/transformers/pull/2313/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fZ3B0Mi5weQ==) | `96.58% <100%> (ø)` | :arrow_up: |
| [src/transformers/tokenization\_bert.py](https://codecov.io/gh/huggingface/transformers/pull/2313/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fYmVydC5weQ==) | `95.13% <66.66%> (-1.22%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=footer). Last update [81db12c...7472a38](https://codecov.io/gh/huggingface/transformers/pull/2313?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Hi i notice one possible issue in your code.
you use `random.random() > dropout`.
However, according to `Docstring: random() -> x in the interval [0, 1).`
So even with droput=0, the bpe output is not deterministic.

Also, according to orig paper, dropout=1 should output char sequence.
but following unchanged code snippet will make it output the raw input
```python3
if not pairs:
    return token
```

and `self.cache` should be updated iff droput=0

> Hi i notice one possible issue in your code.
> you use `random.random() > dropout`.
> However, according to `Docstring: random() -> x in the interval [0, 1).`
> So even with droput=0, the bpe output is not deterministic.

thanks, fixed

> Also, according to orig paper, dropout=1 should output char sequence.
> but following unchanged code snippet will make it output the raw input
> 
> ```python
> if not pairs:
>     return token
> ```

fixed this too, thanks for pointing

> and `self.cache` should be updated iff droput=0

also fixed, thanks

> > Also, according to orig paper, dropout=1 should output char sequence.
> > but following unchanged code snippet will make it output the raw input
> > ```python
> > if not pairs:
> >     return token
> > ```
> 
> fixed this too, thanks for pointing

This issue is not really fixed, the exact corner case is all merges are dropped at the beginning, not limited to dropout=1.
I think the correct fix is replace
```python
        if dropout != 1:
            pairs = [pair for pair in get_pairs(word) if random.random() >= dropout and pair in self.bpe_ranks]
        else:
            # we should merge space byte with first token char
            new_word = []
            token_index = 0
            while token_index < len(token):
                if token[token_index] != self.byte_encoder[32]:
                    new_word.append(token[token_index])
                    token_index += 1
                else:
                    new_word.append(token[token_index : token_index + 2])
                    token_index += 2

            return " ".join(new_word)


        if not pairs:
            return token


        while True:
```
with
```python
        pairs = [pair for pair in get_pairs(word) if random.random() >= dropout and pair in self.bpe_ranks]

        while pairs:
```

> > > Also, according to orig paper, dropout=1 should output char sequence.
> > > but following unchanged code snippet will make it output the raw input
> > > ```python
> > > if not pairs:
> > >     return token
> > > ```
> > 
> > 
> > fixed this too, thanks for pointing
> 
> This issue is not really fixed, the exact corner case is all merges are dropped at the beginning, not limited to dropout=1.
> I think the correct fix is replace
> 
> ```python
>         if dropout != 1:
>             pairs = [pair for pair in get_pairs(word) if random.random() >= dropout and pair in self.bpe_ranks]
>         else:
>             # we should merge space byte with first token char
>             new_word = []
>             token_index = 0
>             while token_index < len(token):
>                 if token[token_index] != self.byte_encoder[32]:
>                     new_word.append(token[token_index])
>                     token_index += 1
>                 else:
>                     new_word.append(token[token_index : token_index + 2])
>                     token_index += 2
> 
>             return " ".join(new_word)
> 
> 
>         if not pairs:
>             return token
> 
> 
>         while True:
> ```
> 
> with
> 
> ```python
>         pairs = [pair for pair in get_pairs(word) if random.random() >= dropout and pair in self.bpe_ranks]
> 
>         while pairs:
> ```

understood your point. simplified code, thanks

I have one advice that replace the usage of `random.random() >= dropout` with `dropout == 0 or dropout < 1 and dropout <= random.random()`, utilizing the short-circuit operator to prevent consuming unnecessary random number. Otherwise, this side effects may cause existing result rely on `random` unrepeatable.

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.


@vitaliyradchenko good idea to add this feature. are you planning to add the suggestion of @boy2000-007man https://github.com/huggingface/transformers/pull/2313#issuecomment-573192073 and re-fresh this PR so that it will finally get merged?