Implement unk_token, fuse_unk, byte_fallback and ignore_merges - #3
Open
bjo4 wants to merge 2 commits into
Open
Conversation
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) <noreply@anthropic.com>
These four options were stored and serialized but never affected tokenization, so any model relying on them was silently mis-tokenized. Character resolution now runs before merging, which is the order HuggingFace uses and not an implementation detail: the tokens resolution produces take part in merges themselves. With <0xE4><0xB8> in the vocabulary and a merge over its two halves, 中 tokenizes as ['<0xE4><0xB8>', '<0xAD>']. tokenize() therefore splits into _resolve_symbols() followed by _apply_merges(), whose signature changes from a string to a sequence of already-resolved symbols. The merge loop body is unchanged. Behaviour, all measured against huggingface/tokenizers rather than derived from reading its source: - unk_token: out-of-vocabulary characters become the unk token. Without one they are dropped, which is what HuggingFace does; previously they reached Tokenizer._encode_single and became id -1. - fuse_unk: consecutive unresolvable characters collapse into a single unk. - byte_fallback: an out-of-vocabulary character becomes its UTF-8 bytes as <0xXX> tokens, uppercase and zero-padded. All-or-nothing per character: one missing byte token sends the whole character to unk instead of emitting a partial expansion. - ignore_merges: a piece already present in the vocabulary is emitted whole. Two behaviours are worth calling out. A pending unk is *not* flushed by a successful byte expansion, so with <0xE4> missing, '中é' gives ['<0xC3>', '<0xA9>', '<unk>']. That is probably an upstream quirk, but byte-parity with HuggingFace is what this package is for, so it is reproduced rather than corrected, and it is covered by fixtures. And byte_fallback is encode-only: the ByteLevel decoder renders <0xE4> as the ASCII characters of its own name, so round-tripping it needs a ByteFallback decoder that does not exist yet. Setting unk_token to a string absent from the vocabulary stays legal at construction and raises ModelError only when unresolvable input actually arrives, matching HuggingFace's laziness. Expectations come from tests/fixtures/hf_bpe_flags.json, recorded by scripts/gen_hf_flag_fixtures.py against a pinned tokenizers version. The test module reads that recording with the standard library only, so the suite still runs with no third-party package installed. The generator is the sole consumer of the new dev extra. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1
These four options were stored and serialized but never affected tokenization,
so any model relying on them was silently mis-tokenized.
Every expectation here was measured against
huggingface/tokenizers==0.23.1,not derived from reading its source. Two of the findings are ones I got wrong on
the first attempt, so they are worth stating explicitly.
Resolution runs before merging
This is the part that is not an implementation detail. HuggingFace resolves each
character onto vocabulary tokens first, and the tokens that resolution produces
then take part in merges:
Generated unks merge too: with
<unk><unk>in vocab and a merge over("<unk>","<unk>"),'azza'gives['a', '<unk><unk>', 'a'].So
tokenize()now splits into_resolve_symbols()followed by_apply_merges(), and_apply_merges()'s signature changes from a string to asequence of already-resolved symbols. The merge loop body itself is unchanged —
the only edits inside it are the signature line and
tuple(piece)→tuple(symbols).Behaviour implemented
unk_tokenTokenizer._encode_singleand became id-1fuse_unkbyte_fallback<0xXX>tokens, uppercase and zero-padded. All-or-nothing per character: one missing byte token sends the whole character to unk rather than emitting a partial expansionignore_mergesSetting
unk_tokento a string absent from the vocabulary stays legal atconstruction and raises
ModelErroronly when unresolvable input actuallyarrives, matching HuggingFace's laziness.
Two things I want to flag rather than bury
A pending unk is not flushed by a successful byte expansion. With
<0xE4>missing,
'中é'gives['<0xC3>', '<0xA9>', '<unk>']— the unk for中landsafter the bytes of
é. The consistent rule is a single pending-unk slot that anin-vocab character flushes, a byte-fallback hit does not, a second unk flushes
unless
fuse_unkcoalesces, and end-of-piece flushes.This is probably an upstream quirk. I reproduced it because byte-parity with
HuggingFace is what this package is for, and deviating silently would be worse
than either choice made openly. It has a comment at the implementation site and
fixtures pinning it. If you would rather emit unks in position, say so — the
fixtures make either choice easy to lock down, but they cannot both hold.
byte_fallbackis encode-only.ByteLevel.decodemaps each character throughthe byte table, so it renders
<0xE4>as the six ASCII characters of its ownname:
Round-tripping needs a
ByteFallbackdecoder, which does not exist yet. I wouldrather add it in a follow-up than grow this PR; flagging it so the limitation is
not a surprise.
Known divergence
ignore_mergesdiverges from HuggingFace here, because this repo inserts addedtokens into
model.vocab(tokenizer.py:140,:518) where HuggingFace does not.An added token can therefore spuriously trigger the whole-piece short-circuit.
Fixing it needs a model/
Tokenizercontract change, so I left it documentedrather than in scope. Tell me if you would rather it be fixed here.
Relatedly, the
-1fallback attokenizer.py:305becomes dead code for everypath except
unk_tokenset but missing from the vocabulary, which now raisesModelError. I left the line alone to keep this diff focused.Testing
Ground truth lives in
tests/fixtures/hf_bpe_flags.json— 17 cases, 91 inputs —recorded by
scripts/gen_hf_flag_fixtures.pyagainst a pinnedtokenizers.tests/test_bpe_flags.pyreads it using only the standard library, so thesuite still runs with no third-party package installed. I verified that by
hiding
tokenizersviaPYTHONPATHand re-running: 174 passed either way.The generator is deterministic (same SHA-256 across runs) and is the sole
consumer of the new
devextra.dependencies = []is untouched. A missingfixture file is a hard failure rather than a skip, so the parity coverage cannot
silently disappear.
Cases include deliberate gate-keepers: a
merge_over_byte_tokens_rank_ordercasewhose output only differs if resolution precedes merging; an
ignore_merges_off_control/ignore_merges_discriminatingpair over a vocabwhere merges cannot reach the whole word, plus a test asserting the two disagree
so the coverage cannot become vacuous; the full pending-unk slot table; and low
bytes (
\t,\n,\x01,\x0f), which are the only inputs that catchuppercase-but-not-zero-padded hex.
Full suite: 174 passed (53 before this work, 25 from PR 1, 96 here). No
existing test changed.