Gemma and Phi-4 support with vocab-aware splitting and lstrip/rstrip tokens#46
Open
jakehlee wants to merge 13 commits into
Open
Gemma and Phi-4 support with vocab-aware splitting and lstrip/rstrip tokens#46jakehlee wants to merge 13 commits into
jakehlee wants to merge 13 commits into
Conversation
Split each text chunk on newline boundaries ([^\n]+|[\n]+) so Gemma's metaspace pipeline no longer feeds the whole document to BPE as one chunk. This restores the per-line cache and rayon parallelism while preserving parity (BPE never merges across the newline/non-newline boundary). Also removes the leftover debug println in run_merge_loop (serialized worker threads on stdout's lock) and adds an ASCII fast-path char->token lookup. Measured on google/gemma-4-E2B with full HF parity: 3.85x ShareGPT52K, 8.61x LongBench-v2 (was ~1.5x). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace newline splitting with vocab-aware byte-pair splitting. Build a 64 KB bigram bridge table at initialization by scanning all vocab tokens to identify which of 256×256 byte pairs appear in any token. At encode time, split at unbridgeable pairs (checking UTF-8 boundaries to avoid breaking multi-byte chars). For Gemma, 81.6% of byte pairs are unbridgeable, creating fine-grained word-level chunks that restore cache hits and rayon parallelism even on long lines. Provably output-preserving: if no vocab token contains byte pair xy, no BPE merge can span that boundary. Measured on google/gemma-4-E2B with full HF parity: 8.92x ShareGPT52K (up from 3.85x), 27.88x LongBench-v2 (up from 8.61x). The LongBench result approaches the theoretical byte-level ceiling on long documents. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Removed the newline-based splitting function and fallback logic, keeping only the vocabulary-aware bigram splitting approach. This makes the codebase fully model-agnostic for all BPE tokenizers. Changes: - Removed split_on_newlines() function and all related tests - Removed fallback logic to newline splitting in encode path - Updated AGENT.md to reflect current implementation status - Updated comments to emphasize generic, model-agnostic approach Rationale: Vocab-aware splitting is strictly superior (8.92x/27.88x vs 3.85x/8.61x on Gemma) and works for all BPE models by automatically adapting to each model's vocabulary at initialization time. No model-specific logic needed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Gate split_on_unbridgeable_bigrams behind a build-time needs_vocab_splitting flag so it runs only when the pre-tokenizer has no ByteLevel step (metaspace models like Gemma). ByteLevel models already chunk at word boundaries via their regex Split, so the per-byte scan was unnecessary there. The pass is output-preserving, so this is a pure performance/scoping change with no parity impact; benchmarks show it neutral within noise on Qwen3 and unchanged on Gemma. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Gemma Performance Improvement with Vocab-aware Splitting
Delegate ReplaceDecoder to normalizers::Replace so the pattern parsing and literal-replace logic have a single source of truth, and drop the unused BigramBridgeTable::stats/from_array methods. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AddedTokens matched literal content only, ignoring the lstrip/rstrip flags parsed from tokenizer.json. Tokens flagged to strip (e.g. Phi-4's <|im_start|>/<|im_end|>, both lstrip=rstrip=true) left adjacent whitespace as separate text, emitting an extra token vs HuggingFace and desyncing the whole stream on inputs containing chat templates. split() now expands a match span over adjacent Unicode whitespace per the token's flags, mirroring HF's left-to-right resolution when two strip tokens share a run. Tokens without strip flags are unchanged, so other models' output is byte-for-byte identical. Fixes microsoft/phi-4 parity on LongBench-v2 (previously failed on the sample embedding a literal <|im_end|>\n<|im_start|> template). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Implement lstrip/rstrip whitespace absorption for added tokens
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.
First, I did not see contributor guidelines or PR templates, so apologies if this is a long shot to merge.
This started as an effort to support
GemmaTokenizer. While adding theReplacenormalizer, theReplaceandByteFallbackdecoders, andbyte_encodetobpefixes compatibility, performance gains are not realized because Gemma is an SPM/metaspace pipeline. The normalizer rewrites every space (" "→"▁") before a no-op space split pre-tokenizer, meaning the entire document hits the BPE engine. The cache never hits and the parallelism never triggers.The solution is vocabulary-aware bigram splitting, where we build a lookup table of bigram pairs that don't appear in the vocabulary, and only split between pairs that cannot merge. This splits the input back into smaller chunks while maintaining token parity. We add a gate to only apply this to models without
ByteLevelin the pre-tokenizer sequence to avoid unnecessary overhead.The result is Gemma4 compatibility and tokenizer performance gains:
There are minor optimizations included, including an ASCII fast-path, but this is the bulk of the improvement.
In addition, I noticed Phi-4 tests were failing. This is due to Phi-4's added tokens that have
"lstrip": trueand"rstrip": true. Adding these flags restores parity.Merging this (or a variant) would be valuable, especially for vLLM. Feel free to make/request any changes for the merge.
Thank you!