Repository navigation
unicode : add the K2-Horizon pre-tokenizer splitter - #1
aaryamonvikram merged 4 commits into
Conversation
The K2-Horizon regex had no arm in unicode_regex_split_custom and fell through to the
general std::regex fallback, which fails two ways.
On MSVC std::regex rejects \p{...}, so no K2-Horizon GGUF loads on Windows at all:
llama-quantize, llama-imatrix and llama-perplexity all abort with
regex_error(error_escape) before a token is produced.
Where the fallback does compile it is still wrong. unicode_regex_split collapses each
codepoint to a single byte naming its Unicode category before matching, and U+200C/U+200D
are category Control, which has no entry in k_ucat_cpt, so both become the 0xD0 fallback
byte. The literal and alternatives in K2's regex can then never match and
every ZWNJ or ZWJ ends a letter run.
The splitter is the existing llama3 one with a single rule widened, since K2's regex
differs from llama3's only in that a letter run also takes marks, ZWNJ and ZWJ.
tests/test-unicode.cpp gains a case for this: it fails before the change with
[Amy] [ZWNJ khaham] and passes after with the run intact.
65a1362 to
69d3a4e
Compare
WestWaters
changed the title
|
I used an ai agent to help me get this working with my tool im building out Pollard-weights. If there is anything else needed please feel free to reach out. |
|
I independently ended up testing the same K2-Horizon Unicode path and found the runtime implementation here matches the behavior I needed. I opened a test-only PR against @WestWaters' branch with broader regression coverage; it makes no runtime changes and passes against the current PR #1 implementation: WestWaters#1 |
|
Independently hit this exact bug loading a Mova/K2-Horizon GGUF (`Failed to process regex` / `regex_error(error_escape)`) and root-caused it the same way — arrived at the same fix (identical `unicode_regex_split_custom_k2_horizon`, same widened letter-run predicate) before finding this PR. For what it's worth, a couple of things I verified independently that corroborate this:
Closed my duplicate (#3) in favor of this one — the real-world Persian ZWNJ test case here is better than my synthetic cases. Would be good to see this merged so Mova loads out of the box again. |
tests: expand K2 Horizon unicode splitter coverage
Assisted-by: Codex
|
Thanks for the PR! |
4 participants
Overview
Additional information
Requirements