🚨🚨 🚨🚨 [Tokenizer] attemp to fix add_token issues🚨🚨 🚨🚨

[ArthurZucker]

What does this PR do?

Adresses a lot of issues related to add_tokens, also adds more refine testing to make sure this does not happen again.

• Adding a token with add_tokens ignores the arguments if the token is an AddedToken. reported in AddedToken 's argument are ignored when called in add_tokens 's method of slow tokenizers #20734 and Adding new tokens to various models changes tokenization of adjacent elements in strings #14770, PreTrainedTokenizer (slow) strip tokens that are around unique_no_split_tokens #21120, T5Tokenizer Fast and Slow give different results with AddedTokens #16334
• Adding a token does not automatically adds it to the unique_no_split_token. Reported in LLaMATokenizerFast works abnormally #23818 , [Bug]? how does the tokenizer encode the special tokens?  #23851, Adding custom tokens makes the T5Tokenizer always strip spaces #11531 but also skip_special_tokens has different behavior between slow and fast tokenizer #23250. Also linked to Add UDOP #22940 , should allow us to re-factor the way T5 tokenizes the inputs (convert_token_to_ids should not have a bunch of regex for special tokens) (also mT5 additional_special_tokens seems not work #9747)
• Adding a token that is already in the vocabulary does not add it. This is debatable, but if someone explicitly adds it, it means he does not want to split it. Reported in ”never_split“ not working on BertTokenizer #23459
• There is no support for single_word in slow. Reported in Adding new tokens to various models changes tokenization of adjacent elements in strings #14770
• Initialising a model from a vocab file does not initialize the Trie. from_pretrained calls added_tokens = tokenizer.sanitize_special_tokens() which is when the tokens are added to no_unique_split. reported in Two tokenizer initialization methods result in inconsistent segmentation results for special words #23930

Fixes #20734, fixes #21120, fixes #16334, fixes #23818 , fixes #23851, fixes #11531 , fixes #9747, fixes #23459 , fixes #14770 , fixes #22935, fixes #23930, fixes #23250, fixes #7901, fixes #19873, fixes #25232, fixes #22414,

Spirit of the refactoring

The main idea is that the PreTrainedTokenizer's __init__ function is responsible for adding all the additional_special tokens, eos_token, etc and creating the token_trie that will be used for splitting the tokens.

• All tokens that are added are stored in their AddedToken format in the added_tokens_decoder which becomes the only way to interact with them. The added_tokens_encoder cannot be modified, it is just a conversion of the added_tokens_decoder. The trie is only created based on the added_tokens_decoder. One possible addition is to keep unique_no_split tokens, but I am currently against.

• All the added token information now lies in the tokenizer_config.json. Nuking the special_tokens_map.json and added_tokens.json.

• Support for lstrip, rstrip and single_word is added. This is only possible because we store the AddedTokens and not only the strings.

• Information on which tokens were added is also available for the Fast tokenizers. This is just a representation convenience but was not possible before.

• add_special_tokens's replace_additional_special_tokens argument now works.

• Remove some of the available surface functions (self.added_tokens_encoder, self.added_tokens_decoder, self.unique_no_split_tokens, etc) and more to come here especial for special tokens,

🚨🚨 Breaking changes 🚨🚨:

• unique_no_split_tokens attribute removed and not used in the internal logic
• sanitize_special_tokens() follows a deprecation cycle and does nothing
• All attributes in SPECIAL_TOKENS_ATTRIBUTES are stored as AddedTokens and no strings.
• loading a slow from a fast or a fast from a slow will no longer raise and error if the tokens added don't have the correct index. This is because they will always be added following the order of the added_tokens but will correct mistakes in the saved vocabulary if there are any. (And there are a lot in old format tokenizers)
• the length of a tokenizer is now max(set(self.get_vocab().keys())) accounting for holes in the vocab. The vocab_size no longer takes into account the added vocab for most of the tokenizers (as it should not). Mostly breaking for T5
• Adding a token using tokenizer.add_tokens([AddedToken("hey", rstrip=False, normalized=True)]) now takes into account rstrip, lstrip, normalized information.
• added_tokens_decoder holds AddedToken, not strings.
• add_tokens() for both fast and slow will always be updated if the token is already part of the vocab, allowing for custom stripping.
• initializing a tokenizer form scratch will now add missing special tokens to the vocab.
• stripping is not always done for special tokens! 🚨 Only if the AddedToken has lstrip=True and rstrip=True
• fairseq_ids_to_tokens attribute removed for Barthez (was not used)

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[ArthurZucker]

TODO: the test should be for all the TokenMixin rather than comparing slow and fast as the behaviour should work in any case.

[ArthurZucker]

This highlights how bad the API is:

• you can't modify the content of the AddedToken
• if you only add the content to the unique no split:
  • when decoding you will have a problem,since decoding uses _additional_special_tokens
  • when encoding, the rstrip and lstrip etc logic will be ignored
• Adding a token does not necessarily update the unique_no_split etc

[stephantul]

Is work on this still going on? I'm interested in working on tokenization.

[ArthurZucker]

It is! Had to focus on a new model for a while but this is close to being over

[stephantul]

Cool! So, I think the best way to do this is that I fork your branch, and then do PRs there? Any changes you like would then propagate to this PR? Is that the preferred way of collaborating?

[ArthurZucker]

Haha sorry what I meant is that I’ll take care of this one probably today, tomorrow so no need to dive !
Otherwise forking transformers is always the best solution IMO, then add others as remotes.

[stephantul]

Ah ok, my bad! I totally misunderstood. Good luck with the PR 😸

[LoicDagnas]

@ArthurZucker I am not sure to understand the whole scope of this PR, but does it means that maintainer of such model have the responsibility to update the added_token.json file to still have their tokenizer usable with AutoTokenizer.from_pretrained?

In other words, is AutoTokenizer.from_pretrained("iarfmoose/t5-base-question-generator") supposed to fail now here

transformers/src/transformers/models/t5/tokenization_t5.py

Line 171 in e4dad4f

raise ValueError(

?

[ArthurZucker]

No it’s not! It might right now but the goal is to keep forward compatibility.
However long term wise authors should update the repo especially if they want to have the same behavior between fast and slow

[LoicDagnas]

Hello @ArthurZucker, about my comment above, with the latest release 4.34.1 it doesn't fail anymore in tokenization_t5.py but here

transformers/src/transformers/models/t5/tokenization_t5_fast.py

Line 127 in e4dad4f

raise ValueError(

Do you want me to open an issue?

[amyeroberts]

Hi @LoicDagnas, @ArthurZucker is off for this week so won't be able to address until then. Yes, please create a separate issue, linking to this one with details on what's happening and a code snippet to reproduce.

[ArthurZucker]

I answered on the issue but it's fixed and part of the latest release! 🤗