diff --git a/src/transformers/convert_slow_tokenizer.py b/src/transformers/convert_slow_tokenizer.py index 2637db428a11..7ac68b713162 100644 --- a/src/transformers/convert_slow_tokenizer.py +++ b/src/transformers/convert_slow_tokenizer.py @@ -110,13 +110,8 @@ def import_protobuf(error_message=""): def _get_prepend_scheme(add_prefix_space: bool, original_tokenizer) -> str: - if add_prefix_space: - prepend_scheme = "always" - if not getattr(original_tokenizer, "legacy", True): - prepend_scheme = "first" - else: - prepend_scheme = "never" - return prepend_scheme + # "first" avoids the spurious "▁" prefix on chunks following a user-added token (#28218). + return "first" if add_prefix_space else "never" def generate_merges(vocab, vocab_scores, skip_tokens: Collection[str] | None = None): @@ -1316,7 +1311,8 @@ def unk_id(self, proto): return proto.trainer_spec.unk_id + self.original_tokenizer.offset def pre_tokenizer(self, replacement, add_prefix_space): - prepend_scheme = _get_prepend_scheme(add_prefix_space, self.original_tokenizer) + # WhitespaceSplit + Metaspace needs "always" so each split word gets the "▁" marker. + prepend_scheme = "always" if add_prefix_space else "never" return pre_tokenizers.Sequence( [ pre_tokenizers.WhitespaceSplit(), diff --git a/src/transformers/models/xlnet/tokenization_xlnet.py b/src/transformers/models/xlnet/tokenization_xlnet.py index 0b682564bffd..dee73617198d 100644 --- a/src/transformers/models/xlnet/tokenization_xlnet.py +++ b/src/transformers/models/xlnet/tokenization_xlnet.py @@ -16,7 +16,6 @@ from tokenizers import AddedToken, Regex, Tokenizer, decoders, normalizers, pre_tokenizers, processors from tokenizers.models import Unigram -from ...tokenization_utils_base import _get_prepend_scheme from ...tokenization_utils_tokenizers import TokenizersBackend from ...utils import logging @@ -144,7 +143,8 @@ def __init__( self._tokenizer.normalizer = normalizers.Sequence(list_normalizers) add_prefix_space = True - prepend_scheme = _get_prepend_scheme(add_prefix_space, self) + # WhitespaceSplit + Metaspace needs "always" so each split word gets the "▁" marker. + prepend_scheme = "always" if add_prefix_space else "never" self._tokenizer.pre_tokenizer = pre_tokenizers.Sequence( [ pre_tokenizers.WhitespaceSplit(), diff --git a/src/transformers/tokenization_utils_base.py b/src/transformers/tokenization_utils_base.py index 86162ae6b01c..02a45862c95f 100644 --- a/src/transformers/tokenization_utils_base.py +++ b/src/transformers/tokenization_utils_base.py @@ -3680,13 +3680,8 @@ def load_vocab_and_merges(pretrained_model_name_or_path, **kwargs): def _get_prepend_scheme(add_prefix_space: bool, original_tokenizer) -> str: - if add_prefix_space: - prepend_scheme = "always" - if not getattr(original_tokenizer, "legacy", True): - prepend_scheme = "first" - else: - prepend_scheme = "never" - return prepend_scheme + # "first" avoids the spurious "▁" prefix on chunks following a user-added token (#28218). + return "first" if add_prefix_space else "never" def generate_merges(vocab, vocab_scores: dict[str, float] | None = None, skip_tokens: Collection[str] | None = None): diff --git a/src/transformers/tokenization_utils_tokenizers.py b/src/transformers/tokenization_utils_tokenizers.py index d7c9744e5e11..68c58567cd83 100644 --- a/src/transformers/tokenization_utils_tokenizers.py +++ b/src/transformers/tokenization_utils_tokenizers.py @@ -488,6 +488,10 @@ def __init__(self, *args, **kwargs): if self._should_update_post_processor: self.update_post_processor() + # Construction (incl. checkpoint-restore of added tokens) is done; later non-special adds are + # genuine user edits and may realign the Metaspace prepend_scheme in `_add_tokens` (#28218). + self._prepend_scheme_can_flip = True + @property def is_fast(self) -> bool: return True @@ -727,6 +731,24 @@ def _add_tokens(self, new_tokens: list[str | AddedToken], special_tokens=False) if special_tokens: return self._tokenizer.add_special_tokens(new_tokens) + # #28218: adding a genuinely new non-special token to a standalone Metaspace("always") leaves a + # spurious "▁" on the next chunk; realign to "first". Gated on a finished tokenizer and a not-yet- + # present token, so restoring a checkpoint's own added tokens (during or after init) is left alone. + if getattr(self, "_prepend_scheme_can_flip", False): + pre_tokenizer = self._tokenizer.pre_tokenizer + if ( + isinstance(pre_tokenizer, pre_tokenizers_fast.Metaspace) + and pre_tokenizer.prepend_scheme == "always" + and any( + (isinstance(t, str) or not t.special) and self._tokenizer.token_to_id(str(t)) is None + for t in new_tokens + ) + ): + self._tokenizer.pre_tokenizer = pre_tokenizers_fast.Metaspace( + replacement=pre_tokenizer.replacement, + prepend_scheme="first", + split=pre_tokenizer.split, + ) return self._tokenizer.add_tokens(new_tokens) def num_special_tokens_to_add(self, pair: bool = False) -> int: diff --git a/tests/tokenization/test_tokenization_fast.py b/tests/tokenization/test_tokenization_fast.py index 48f6f75a16eb..c4121d183561 100644 --- a/tests/tokenization/test_tokenization_fast.py +++ b/tests/tokenization/test_tokenization_fast.py @@ -20,7 +20,7 @@ import unittest from tokenizers import Tokenizer, decoders, pre_tokenizers, trainers -from tokenizers.models import BPE, WordLevel +from tokenizers.models import BPE, Unigram, WordLevel from transformers import AutoTokenizer, PreTrainedTokenizerFast from transformers.testing_utils import require_tokenizers @@ -217,6 +217,53 @@ def test_class_after_save_and_reload(self): tokenizer = AutoTokenizer.from_pretrained(temp_dir, use_fast=True) self.assertIsInstance(tokenizer, PreTrainedTokenizerFast) + def test_added_token_does_not_get_metaspace_prefix(self): + # #28218: a non-special add_tokens call on a SentencePiece-style fast tokenizer + # must not leave a spurious "▁" prefix on the chunk after the added token. + vocab = [ + ("", 0.0), + ("▁", -10.0), + ("gym", 0.0), + ("▁gym", 0.0), + ("▁Hi", 0.0), + ("▁world", 0.0), + ] + backend = Tokenizer(Unigram(vocab=vocab, unk_id=0)) + backend.pre_tokenizer = pre_tokenizers.Metaspace(replacement="▁", prepend_scheme="always", split=True) + fast = PreTrainedTokenizerFast(tokenizer_object=backend, unk_token="") + self.assertEqual(fast._tokenizer.pre_tokenizer.prepend_scheme, "always") + + fast.add_tokens(["abcd"]) + + self.assertEqual(fast._tokenizer.pre_tokenizer.prepend_scheme, "first") + self.assertEqual(fast.tokenize("abcdgym"), ["abcd", "gym"]) + self.assertEqual(fast.tokenize("abcd gym"), ["abcd", "▁gym"]) + self.assertEqual( + fast.tokenize("Hi abcdgym world"), + ["▁Hi", "▁", "abcd", "gym", "▁world"], + ) + + def test_checkpoint_restore_keeps_metaspace_prepend_scheme(self): + # #28218 follow-up: restoring a checkpoint that legitimately baked prepend_scheme="always" + # together with its own non-special added tokens must NOT flip the scheme to "first". The + # realignment only applies to genuine post-init user add_tokens, never to load-time restore. + vocab = [("", 0.0), ("▁", -10.0), ("gym", 0.0), ("▁gym", 0.0)] + backend = Tokenizer(Unigram(vocab=vocab, unk_id=0)) + backend.pre_tokenizer = pre_tokenizers.Metaspace(replacement="▁", prepend_scheme="always", split=True) + backend.add_tokens(["abcd"]) # a non-special token baked into the checkpoint + fast = PreTrainedTokenizerFast(tokenizer_object=backend, unk_token="") + self.assertEqual(fast._tokenizer.pre_tokenizer.prepend_scheme, "always") + + with tempfile.TemporaryDirectory() as tmp_dir: + fast.save_pretrained(tmp_dir) + reloaded = PreTrainedTokenizerFast.from_pretrained(tmp_dir) + + # Restoring "abcd" while loading must leave the scheme untouched... + self.assertEqual(reloaded._tokenizer.pre_tokenizer.prepend_scheme, "always") + # ...while a genuine post-load user add still realigns it. + reloaded.add_tokens(["efgh"]) + self.assertEqual(reloaded._tokenizer.pre_tokenizer.prepend_scheme, "first") + def test_bpe_tokenizer_skips_clean_up_tokenization_spaces(self): """BPE tokenizers should not apply clean_up_tokenization even when the flag is True.