adding additional additional_special_tokens to tokenizer has inconsistent behavior
Author: andifunke
Pull request: no
Labels: name: wontfix

## Environment info

- `transformers` version: 3.1.0
- Platform: Linux-5.4.0-45-generic-x86_64-with-debian-bullseye-sid
- Python version: 3.6.10
- PyTorch version (GPU?): 1.6.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: no
- Using distributed or parallel set-up in script?: no

### Who can help

@mfuntowicz

## Information

affected: all tokenizers based on `SpecialTokensMixin`

The behavior of the `add_special_tokens()` method seems irregular to me, when adding `additional_special_tokens` to a tokenizer that already holds a list of `additional_special_tokens`. In this case the value of `self._additional_special_tokens` will simply be replaced, while the previous additional special tokens still remain in `PreTrainedTokenizer.added_tokens_encoder` and `PreTrainedTokenizer.added_tokens_decoder`.


## To reproduce

Steps to reproduce the behavior:

```python
from transformers import GPT2Tokenizer

def print_special_tokens():
    print(tokenizer.all_special_tokens)
    print(tokenizer.all_special_ids)
    print(tokenizer.additional_special_tokens)
    print(tokenizer.additional_special_tokens_ids)
    print(tokenizer.special_tokens_map)
    print(tokenizer.added_tokens_encoder)
    print(tokenizer.added_tokens_decoder)

tokenizer = GPT2Tokenizer.from_pretrained(
    'gpt2',
    pad_token='[PAD]',
    additional_special_tokens=['<A>', '<B>', '<C>']
)
print_special_tokens()
tokenizer.add_special_tokens({
    'cls_token': '[CLS]',
    'additional_special_tokens': ['<B>', '<X>', '<X>']
})
print('-'*50)
print_special_tokens()
```

Output:

```
['
', '
', '
', '[PAD]', '<A>', '<B>', '<C>']
[50256, 50256, 50256, 50257, 50258, 50259, 50260]
['<A>', '<B>', '<C>']
[50258, 50259, 50260]
{'bos_token': '
', 'eos_token': '
', 'unk_token': '
', 'pad_token': '[PAD]', 'additional_special_tokens': "['<A>', '<B>', '<C>']"}
{'[PAD]': 50257, '<A>': 50258, '<B>': 50259, '<C>': 50260}
{50257: '[PAD]', 50258: '<A>', 50259: '<B>', 50260: '<C>'}
--------------------------------------------------
['
', '
', '
', '[PAD]', '[CLS]', '<B>', '<X>']
[50256, 50256, 50256, 50257, 50261, 50259, 50262]
['<B>', '<X>', '<X>']
[50259, 50262, 50262]
{'bos_token': '
', 'eos_token': '
', 'unk_token': '
', 'pad_token': '[PAD]', 'cls_token': '[CLS]', 'additional_special_tokens': "['<B>', '<X>', '<X>']"}
{'[PAD]': 50257, '<A>': 50258, '<B>': 50259, '<C>': 50260, '[CLS]': 50261, '<X>': 50262}
{50257: '[PAD]', 50258: '<A>', 50259: '<B>', 50260: '<C>', 50261: '[CLS]', 50262: '<X>'}
```


## Expected behavior

Additional special tokens added by `add_special_tokens()` should be appended to the existing `_additional_special_tokens` list and not replace them. Also, there should be some deduplication logic.

The following code change in `SpecialTokensMixin.add_special_tokens()` will do exactly this:

```python
        for key, value in special_tokens_dict.items():
            assert key in self.SPECIAL_TOKENS_ATTRIBUTES, f"Key {key} is not a special token"

            if key == "additional_special_tokens":
                assert isinstance(value, (list, tuple)) and all(
                    isinstance(t, (str, AddedToken)) for t in value
                ), f"Tokens {value} for key {key} should all be str or AddedToken instances"

                if self.verbose:
                    logger.info("Adding %s to `additional_special_tokens`", value)

                for token in value:
                    if token not in self.additional_special_tokens:
                        self._additional_special_tokens.append(token)

                added_tokens += self.add_tokens(value, special_tokens=True)
            else:
                assert isinstance(
                    value, (str, AddedToken)
                ), f"Token {value} for key {key} should be a str or an AddedToken instance"

                if self.verbose:
                    logger.info("Assigning %s to the %s key of the tokenizer", value, key)
                setattr(self, key, value)

                added_tokens += self.add_tokens([value], special_tokens=True)
```

Now, when running the above code example the output is as expected (imho):

```
['
', '
', '
', '[PAD]', '<A>', '<B>', '<C>']
[50256, 50256, 50256, 50257, 50258, 50259, 50260]
['<A>', '<B>', '<C>']
[50258, 50259, 50260]
{'bos_token': '
', 'eos_token': '
', 'unk_token': '
', 'pad_token': '[PAD]', 'additional_special_tokens': "['<A>', '<B>', '<C>']"}
{'[PAD]': 50257, '<A>': 50258, '<B>': 50259, '<C>': 50260}
{50257: '[PAD]', 50258: '<A>', 50259: '<B>', 50260: '<C>'}
--------------------------------------------------
['
', '
', '
', '[PAD]', '[CLS]', '<A>', '<B>', '<C>', '<X>']
[50256, 50256, 50256, 50257, 50261, 50258, 50259, 50260, 50262]
['<A>', '<B>', '<C>', '<X>']
[50258, 50259, 50260, 50262]
{'bos_token': '
', 'eos_token': '
', 'unk_token': '
', 'pad_token': '[PAD]', 'cls_token': '[CLS]', 'additional_special_tokens': "['<A>', '<B>', '<C>', '<X>']"}
{'[PAD]': 50257, '<A>': 50258, '<B>': 50259, '<C>': 50260, '[CLS]': 50261, '<X>': 50262}
{50257: '[PAD]', 50258: '<A>', 50259: '<B>', 50260: '<C>', 50261: '[CLS]', 50262: '<X>'}
```

I could open a PR if you agree that this is indeed the expected behavior.

Comments:
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.