From 5be7acaff15cedc826963deca04b7f004d7b6bff Mon Sep 17 00:00:00 2001 From: JuiHsuanLee0303 Date: Tue, 28 Jul 2026 14:51:10 +0800 Subject: [PATCH] Reject unimplemented BPE options instead of ignoring them silently BPE and BpeTrainer accepted a set of HuggingFace-compatible options, stored them, and in the model's case serialized them back out, without ever acting on any of them. A caller who set one got wrong tokenization with no signal, and a round-tripped tokenizer.json looked unchanged, so nothing appeared wrong. Options that would change tokenization now raise UnsupportedFeatureError: dropout in (0, 1], and non-empty continuing_subword_prefix / end_of_word_suffix. A dropout outside [0, 1] raises ModelError instead, since that is invalid rather than unimplemented, matching HuggingFace's own rejection at construction. Default values stay silent. None, 0.0 and "" are genuine no-ops in HuggingFace too, so files that tokenize correctly today keep loading -- GPT-2-lineage exports ship "" for both affix options. BpeTrainer gets the same treatment for its two affix options, which it also stored and never read. Fixing only the model would have left half of the same defect in place. cache_capacity was being discarded entirely, without even an attribute assignment. It is now retained, but not rejected: a cache is a pure optimization, so its absence cannot change output. HuggingFace does not serialize it into tokenizer.json, so to_dict() stays faithful by omitting it. The rejection rules live in a shared module because they must stay identical in the model and the trainer. Co-Authored-By: Claude Opus 5 (1M context) --- docs/components.md | 21 +++++ src/ubi_tokenizer/_validation.py | 54 +++++++++++++ src/ubi_tokenizer/errors.py | 9 +++ src/ubi_tokenizer/models/bpe.py | 11 +++ src/ubi_tokenizer/trainers/bpe.py | 2 + tests/test_unsupported_flags.py | 123 ++++++++++++++++++++++++++++++ 6 files changed, 220 insertions(+) create mode 100644 src/ubi_tokenizer/_validation.py create mode 100644 tests/test_unsupported_flags.py diff --git a/docs/components.md b/docs/components.md index 77c10a9..5583e48 100644 --- a/docs/components.md +++ b/docs/components.md @@ -61,3 +61,24 @@ Reconstructs text from tokens. - `BpeTrainer(vocab_size, min_frequency, special_tokens=None, ...)` — in-memory greedy trainer. For large/distributed training use UbiTok; this is for small corpora and tests. + +## Unimplemented options + +`BPE` and `BpeTrainer` accept the full HuggingFace option set so that a +`tokenizer.json` round-trips unchanged. Options that are not implemented raise +`UnsupportedFeatureError` when set to a value that would change tokenization, +rather than being accepted and ignored: + +| Option | Accepted values | Rejected values | +| --- | --- | --- | +| `dropout` | `None`, `0.0` | anything in `(0, 1]` — outside `[0, 1]` raises `ModelError` | +| `continuing_subword_prefix` | `None`, `""` | any non-empty string | +| `end_of_word_suffix` | `None`, `""` | any non-empty string | + +The accepted values are genuine no-ops in HuggingFace as well, so files that +tokenize correctly today keep loading — GPT-2-lineage exports ship `""` for both +affix options. + +`cache_capacity` is accepted and retained but not yet used. It is not rejected +because a cache is a pure optimization: its absence cannot change output. +HuggingFace does not serialize it into `tokenizer.json`, so `to_dict()` omits it. diff --git a/src/ubi_tokenizer/_validation.py b/src/ubi_tokenizer/_validation.py new file mode 100644 index 0000000..7ab5629 --- /dev/null +++ b/src/ubi_tokenizer/_validation.py @@ -0,0 +1,54 @@ +"""Shared rejection rules for recognized-but-unimplemented options. + +The BPE model and the BPE trainer both accept options that they store, and in +the model's case serialize, without ever acting on them. Silently accepting +such an option hands the caller wrong tokenization with no signal, so a value +that would change behaviour is rejected instead. + +The rule has to stay identical in both places, so it lives here rather than +being duplicated. +""" + +from __future__ import annotations + +from typing import Optional + +from ubi_tokenizer.errors import ModelError, UnsupportedFeatureError + +_HINT = ( + "recognized but not implemented by ubi_tokenizer. Drop the option, or use " + "huggingface/tokenizers if you need it." +) + + +def reject_unimplemented_affixes( + continuing_subword_prefix: Optional[str], + end_of_word_suffix: Optional[str], +) -> None: + """Reject affix options that would change tokenization. + + ``None`` and ``""`` are genuine no-ops in HuggingFace and stay accepted -- + GPT-2-lineage ``tokenizer.json`` files ship ``""`` for both, and rejecting + those would break loading files that tokenize correctly today. + """ + for name, value in ( + ("continuing_subword_prefix", continuing_subword_prefix), + ("end_of_word_suffix", end_of_word_suffix), + ): + if value: + raise UnsupportedFeatureError(f"{name}={value!r} is {_HINT}") + + +def reject_unimplemented_dropout(dropout: Optional[float]) -> None: + """Reject BPE dropout. + + ``None`` and ``0.0`` both mean "no dropout" and stay accepted. Values + outside ``[0, 1]`` are invalid rather than unimplemented, which is why they + raise :class:`ModelError`; HuggingFace rejects them at construction too. + """ + if dropout is None: + return + if not 0.0 <= dropout <= 1.0: + raise ModelError(f"dropout={dropout!r} must be between 0 and 1, inclusive") + if dropout > 0.0: + raise UnsupportedFeatureError(f"dropout={dropout!r} is {_HINT}") diff --git a/src/ubi_tokenizer/errors.py b/src/ubi_tokenizer/errors.py index b72daaf..6907d75 100644 --- a/src/ubi_tokenizer/errors.py +++ b/src/ubi_tokenizer/errors.py @@ -13,3 +13,12 @@ class DeserializationError(UbiTokenizerError): class ModelError(UbiTokenizerError): """Raised for invalid model state (missing tokens, bad merges, etc.).""" + + +class UnsupportedFeatureError(UbiTokenizerError): + """Raised when a requested option is recognized but not implemented. + + Distinct from :class:`ModelError`: the request is well-formed, it simply + cannot be honoured. Raised eagerly so the caller finds out when the + tokenizer is built rather than getting silently wrong tokenization. + """ diff --git a/src/ubi_tokenizer/models/bpe.py b/src/ubi_tokenizer/models/bpe.py index 4be8077..16b1a86 100644 --- a/src/ubi_tokenizer/models/bpe.py +++ b/src/ubi_tokenizer/models/bpe.py @@ -10,6 +10,11 @@ from typing import Any, Optional +from ubi_tokenizer._validation import ( + reject_unimplemented_affixes, + reject_unimplemented_dropout, +) + class Model: """Abstract base for tokenization models.""" @@ -47,8 +52,14 @@ def __init__( byte_fallback: bool = False, ignore_merges: bool = False, ) -> None: + reject_unimplemented_dropout(dropout) + reject_unimplemented_affixes(continuing_subword_prefix, end_of_word_suffix) self.vocab: dict[str, int] = dict(vocab) if vocab else {} self.merges: list[tuple[str, str]] = [tuple(m) for m in (merges or [])] + # Accepted rather than rejected: a cache is a pure optimization, so its + # absence cannot change output. HuggingFace does not serialize it into + # tokenizer.json either, so to_dict() stays faithful by omitting it. + self.cache_capacity = cache_capacity self.dropout = dropout self.unk_token = unk_token self.continuing_subword_prefix = continuing_subword_prefix diff --git a/src/ubi_tokenizer/trainers/bpe.py b/src/ubi_tokenizer/trainers/bpe.py index 3fc3fa6..a021631 100644 --- a/src/ubi_tokenizer/trainers/bpe.py +++ b/src/ubi_tokenizer/trainers/bpe.py @@ -14,6 +14,7 @@ from collections.abc import Iterable, Iterator from typing import Optional +from ubi_tokenizer._validation import reject_unimplemented_affixes from ubi_tokenizer.byte_level import BYTE_ENCODER, encode_token_bytes from ubi_tokenizer.models.bpe import BPE from ubi_tokenizer.pre_tokenizers.byte_level import ByteLevel @@ -40,6 +41,7 @@ def __init__( continuing_subword_prefix: Optional[str] = None, end_of_word_suffix: Optional[str] = None, ) -> None: + reject_unimplemented_affixes(continuing_subword_prefix, end_of_word_suffix) self.vocab_size = vocab_size self.min_frequency = min_frequency self.special_tokens = list(special_tokens or []) diff --git a/tests/test_unsupported_flags.py b/tests/test_unsupported_flags.py new file mode 100644 index 0000000..89879d7 --- /dev/null +++ b/tests/test_unsupported_flags.py @@ -0,0 +1,123 @@ +"""Unimplemented BPE / trainer options must fail loudly instead of silently. + +``BPE`` and ``BpeTrainer`` accept several HuggingFace-compatible options that are +stored and serialized but never acted on. Accepting them silently means a caller +gets wrong tokenization with no signal, so a non-default value now raises. +Default values stay silent: they are true no-ops in HuggingFace too, and +GPT-2-lineage ``tokenizer.json`` files ship ``""`` for the affix options. +""" + +import pytest + +from ubi_tokenizer.errors import ModelError, UbiTokenizerError, UnsupportedFeatureError +from ubi_tokenizer.models.bpe import BPE +from ubi_tokenizer.trainers.bpe import BpeTrainer + + +def test_unsupported_feature_error_is_a_ubi_tokenizer_error(): + assert issubclass(UnsupportedFeatureError, UbiTokenizerError) + + +# -- dropout --------------------------------------------------------------- + + +@pytest.mark.parametrize("value", [None, 0.0]) +def test_no_op_dropout_is_accepted(value): + # HuggingFace treats both as "no dropout", so they must not raise. + assert BPE(dropout=value).dropout == value + + +@pytest.mark.parametrize("value", [0.1, 0.5, 1.0]) +def test_active_dropout_is_rejected(value): + with pytest.raises(UnsupportedFeatureError, match="dropout"): + BPE(dropout=value) + + +@pytest.mark.parametrize("value", [-0.1, 1.5]) +def test_out_of_range_dropout_is_rejected_as_invalid_model_state(value): + # HuggingFace rejects these at construction as invalid rather than + # unimplemented, so they are ModelError and not UnsupportedFeatureError. + with pytest.raises(ModelError, match="dropout"): + BPE(dropout=value) + + +# -- affix options -------------------------------------------------------- + + +@pytest.mark.parametrize("option", ["continuing_subword_prefix", "end_of_word_suffix"]) +@pytest.mark.parametrize("value", [None, ""]) +def test_no_op_affix_options_are_accepted(option, value): + assert getattr(BPE(**{option: value}), option) == value + + +@pytest.mark.parametrize( + ("option", "value"), + [("continuing_subword_prefix", "##"), ("end_of_word_suffix", "")], +) +def test_active_affix_options_are_rejected(option, value): + with pytest.raises(UnsupportedFeatureError, match=option): + BPE(**{option: value}) + + +@pytest.mark.parametrize( + ("option", "value"), + [("continuing_subword_prefix", "##"), ("end_of_word_suffix", "")], +) +def test_trainer_rejects_active_affix_options(option, value): + # The trainer stores these two and never reads them either; fixing only the + # model would leave half of the same defect in place. + with pytest.raises(UnsupportedFeatureError, match=option): + BpeTrainer(**{option: value}) + + +@pytest.mark.parametrize("option", ["continuing_subword_prefix", "end_of_word_suffix"]) +@pytest.mark.parametrize("value", [None, ""]) +def test_trainer_accepts_no_op_affix_options(option, value): + assert getattr(BpeTrainer(**{option: value}), option) == value + + +def test_trainer_defaults_are_accepted(): + trainer = BpeTrainer(vocab_size=64, min_frequency=1) + assert trainer.continuing_subword_prefix is None + assert trainer.end_of_word_suffix is None + + +# -- message quality ------------------------------------------------------- + + +def test_rejection_message_names_the_option_and_says_it_is_unimplemented(): + with pytest.raises(UnsupportedFeatureError) as excinfo: + BPE(end_of_word_suffix="") + message = str(excinfo.value) + assert "end_of_word_suffix" in message + assert "not implemented" in message.lower() + + +# -- cache_capacity is accepted, not rejected ------------------------------ + + +def test_cache_capacity_is_retained(): + # A cache is a pure optimization: its absence cannot change output, so this + # is accepted rather than rejected. It was previously discarded entirely. + assert BPE(cache_capacity=1234).cache_capacity == 1234 + + +def test_cache_capacity_stays_out_of_serialization(): + # HuggingFace does not serialize cache_capacity into tokenizer.json, so + # omitting it keeps to_dict() faithful rather than lossy. + assert "cache_capacity" not in BPE(cache_capacity=1234).to_dict() + + +# -- deserialization path -------------------------------------------------- + + +def test_unsupported_option_in_tokenizer_json_is_rejected_on_load(): + from ubi_tokenizer.tokenizer import Tokenizer + + payload = { + "version": "1.0", + "added_tokens": [], + "model": {"type": "BPE", "vocab": {"a": 0}, "merges": [], "dropout": 0.5}, + } + with pytest.raises(UnsupportedFeatureError, match="dropout"): + Tokenizer.from_dict(payload)