* fix: patch CONTROL type for special tokens in sentencepiece GGUF export (fixes #5070) When converting a Gemma 3 fine-tune to GGUF via save_pretrained_gguf, tokens like <start_of_turn> (id=105) and <end_of_turn> (id=106) are already present in the sentencepiece model but are typed as NORMAL (1) instead of CONTROL (3). llama.cpp only recognises CONTROL tokens when parse_special=True is active, so these tokens get BPE-split during chat inference and the model produces garbage output. fix_sentencepiece_gguf now reads tokenizer.json's added_tokens list and, for any token with "special": true whose ID falls within the existing sentencepiece vocabulary, updates its type from NORMAL to CONTROL before writing the patched tokenizer.model to disk. The same CONTROL type is also applied when new tokens are appended for the out-of-range case, so both code paths are consistent. * Wire fix_sentencepiece_gguf into tokenizer save path and guard np.diff - save.py: call fix_sentencepiece_gguf inside unsloth_tokenizer_save_pretrained after _preserve_sentencepiece_tokenizer_assets. The helper was previously unreferenced in the repo, so the PR's CONTROL-type patch never actually ran during save_pretrained_gguf. - tokenizer_utils.py: add an early-return guard for len(added_tokens_ids) < 2 before the existing np.diff contiguity check. np.diff on a single-element array returns [] and .min() raises ValueError, which would discard the new in-vocab CONTROL patch; the guard flushes tokenizer.model first. Guard is inserted before the existing lines (diff = np.diff(...) and the min/max check) so their blame is unchanged. Dropped the separate refactor to fold the four duplicated "if patched > 0: write tokenizer.model" blocks into a helper because doing so re-indents lines whose blame is "Formatting & bug fixes"; the duplication remains the author's pattern. * Fix review findings: negative token_id guard and np.diff single-element - tokenizer_utils.py:481: add 0 <= lower bound to the special_token_ids bounds check. Previously a negative token_id from tokenizer.json passed 'token_id < sentence_piece_size' and Python's negative indexing wrapped tokenizer_file.pieces[-1] to silently corrupt the last piece to CONTROL. - tokenizer_utils.py:513: replace the loop-1 'if len < 2: return' guard (which was too broad: it silently skipped vocab extension for single-entry added_tokens.json) with a pre-pass that substitutes a trivially-contiguous 2-element sentinel for the contiguity check, then restores the original array before the append loop. Lines 519 ('diff = np.diff(added_tokens_ids)') and 520-529 (min/max/boundary checks and early-return write blocks) are left literally unchanged so blame remains intact. * Restore real added_tokens_ids before min boundary check Move the '_real_added_tokens_ids' restore above the 'added_tokens_ids.min() != sentence_piece_size' check. With the previous order the sentinel [sentence_piece_size, sentence_piece_size + 1] was still in scope when the min check ran, so any single-entry added_tokens .json with an out-of-range start id (e.g. 99 when sentence_piece_size=2) bypassed the boundary check and fell through to the append loop. * Scope fix_sentencepiece_gguf to GGUF export path only Previously wired fix_sentencepiece_gguf into unsloth_tokenizer_save_pretrained, which is the generic monkey-patch replacement for every tokenizer.save_pretrained call. That caused the GGUF-specific mutation (and the unconditional protobuf import in fix_sentencepiece_gguf) to run on every LoRA / merged 16-bit / push_to_hub / torchao save, where it has no purpose and can abort the entire save if the protobuf runtime is unavailable. - save.py: remove fix_sentencepiece_gguf call from unsloth_tokenizer_save_pretrained. - save.py: add the call inside unsloth_save_pretrained_gguf immediately before save_to_gguf, wrapped in try/except so a protobuf import failure logs a warning and lets GGUF conversion proceed rather than aborting the save. * Broaden special-token retag to USER_DEFINED and narrow save.py except - tokenizer_utils.py:483: the in-vocab retag previously only promoted NORMAL pieces to CONTROL, but the real Gemma tokenizer (e.g. unsloth/functiongemma -270m-it) stores <start_of_turn>/<end_of_turn> as USER_DEFINED (type 4). Extend the predicate to cover both NORMAL and USER_DEFINED so tokens marked "special": true in tokenizer.json are promoted regardless of their current sentencepiece type. Only tokens explicitly flagged special are touched, so non-special USER_DEFINED pieces are unchanged; already-CONTROL pieces stay unchanged. The warning message is generalised accordingly. - save.py:2294: narrow the except clause from Exception to ImportError. The loop-3 try/except was added to tolerate a missing protobuf runtime; leaving it broad also swallows OSError/PermissionError mid-write, which would ship a corrupted tokenizer.model to save_to_gguf. ImportError still covers the protobuf case while letting I/O errors propagate to the outer save handler. * Harden fix_sentencepiece_gguf: widen except, protobuf fallback, revert USER_DEFINED widen, guard entry id - save.py:2294: widen except from ImportError back to Exception. The loop-4 narrowing let JSONDecodeError / KeyError / OSError / PermissionError from fix_sentencepiece_gguf abort the entire GGUF export, a regression vs pre-PR behavior. The outer save_to_gguf try/except still covers GGUF-side failures; any fix-side failure now logs a typed warning and lets conversion proceed. - tokenizer_utils.py:445: the direct 'from transformers.utils import sentencepiece_model_pb2' raises TypeError ("Descriptors cannot be created directly") on modern protobuf runtimes. Prepend a sys.modules.setdefault pre-population using transformers.convert_slow_tokenizer.import_protobuf() so the subsequent from-import finds a compatible module via the module cache. The original import line is left verbatim at its place as the final resolver. - tokenizer_utils.py:483: revert loop-4 widening; retag only NORMAL pieces to CONTROL. Retagging USER_DEFINED pieces caused a concrete tokenization regression where an intentionally-USER_DEFINED in-vocab special token had its sentencepiece encoding broken ('<user> hello' changed from [11, 3, 8] to [11, 0, 12, 21, 0, 8]). The PR's stated scope is the NORMAL->CONTROL Gemma case; USER_DEFINED handling is deferred. - tokenizer_utils.py:475: defensive guard around entry["id"]. A malformed added_tokens entry missing the "id" field or with a non-int id is now skipped rather than raising KeyError / inserting garbage. * Add review tests for sentencepiece GGUF fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: octo-patch <octo-patch@github.com> Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
132 lines
4.3 KiB
Python
132 lines
4.3 KiB
Python
import ast
|
|
import json
|
|
import os
|
|
|
|
os.environ.setdefault("PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION", "python")
|
|
|
|
from transformers.utils import sentencepiece_model_pb2
|
|
|
|
from unsloth.tokenizer_utils import fix_sentencepiece_gguf
|
|
|
|
|
|
NORMAL, CONTROL, USER_DEFINED = 1, 3, 4
|
|
|
|
_SAVE_PY = os.path.abspath(
|
|
os.path.join(os.path.dirname(__file__), "..", "..", "unsloth", "save.py")
|
|
)
|
|
_TOK_PY = os.path.abspath(
|
|
os.path.join(os.path.dirname(__file__), "..", "..", "unsloth", "tokenizer_utils.py")
|
|
)
|
|
|
|
|
|
def _build(pieces):
|
|
m = sentencepiece_model_pb2.ModelProto()
|
|
for piece, score, typ in pieces:
|
|
p = m.pieces.add()
|
|
p.piece = piece
|
|
p.score = score
|
|
p.type = typ
|
|
return m.SerializeToString()
|
|
|
|
|
|
def _read(path):
|
|
m = sentencepiece_model_pb2.ModelProto()
|
|
with open(path, "rb") as f:
|
|
m.ParseFromString(f.read())
|
|
return [(p.piece, p.type) for p in m.pieces]
|
|
|
|
|
|
def test_user_defined_special_piece_is_not_retyped(tmp_path):
|
|
pieces = [
|
|
("<s>", 0.0, CONTROL),
|
|
("a", -1.0, NORMAL),
|
|
("<ud_special>", -1.0, USER_DEFINED),
|
|
]
|
|
(tmp_path / "tokenizer.model").write_bytes(_build(pieces))
|
|
(tmp_path / "tokenizer.json").write_text(
|
|
json.dumps(
|
|
{"added_tokens": [{"id": 2, "content": "<ud_special>", "special": True}]}
|
|
)
|
|
)
|
|
fix_sentencepiece_gguf(str(tmp_path))
|
|
got = dict(_read(str(tmp_path / "tokenizer.model")))
|
|
assert got["<ud_special>"] == USER_DEFINED
|
|
|
|
|
|
def test_malformed_entry_missing_id_does_not_raise(tmp_path):
|
|
pieces = [("<s>", 0.0, CONTROL), ("a", -1.0, NORMAL), ("<sot>", -1.0, NORMAL)]
|
|
(tmp_path / "tokenizer.model").write_bytes(_build(pieces))
|
|
(tmp_path / "tokenizer.json").write_text(
|
|
json.dumps(
|
|
{
|
|
"added_tokens": [
|
|
{"content": "no_id_entry", "special": True},
|
|
{"id": 2, "content": "<sot>", "special": True},
|
|
]
|
|
}
|
|
)
|
|
)
|
|
fix_sentencepiece_gguf(str(tmp_path))
|
|
got = dict(_read(str(tmp_path / "tokenizer.model")))
|
|
assert got["<sot>"] == CONTROL
|
|
|
|
|
|
def test_entry_with_non_int_id_is_skipped(tmp_path):
|
|
pieces = [("<s>", 0.0, CONTROL), ("a", -1.0, NORMAL)]
|
|
(tmp_path / "tokenizer.model").write_bytes(_build(pieces))
|
|
(tmp_path / "tokenizer.json").write_text(
|
|
json.dumps({"added_tokens": [{"id": "oops", "content": "x", "special": True}]})
|
|
)
|
|
before = (tmp_path / "tokenizer.model").read_bytes()
|
|
fix_sentencepiece_gguf(str(tmp_path))
|
|
after = (tmp_path / "tokenizer.model").read_bytes()
|
|
assert before == after
|
|
|
|
|
|
def test_save_py_except_clause_is_broad_exception():
|
|
with open(_SAVE_PY) as f:
|
|
tree = ast.parse(f.read())
|
|
for node in ast.walk(tree):
|
|
if (
|
|
isinstance(node, ast.FunctionDef)
|
|
and node.name == "unsloth_save_pretrained_gguf"
|
|
):
|
|
for subnode in ast.walk(node):
|
|
if isinstance(subnode, ast.Try):
|
|
body_src = "\n".join(ast.unparse(s) for s in subnode.body)
|
|
if "fix_sentencepiece_gguf(" not in body_src:
|
|
continue
|
|
handler = subnode.handlers[0]
|
|
assert handler.type is not None
|
|
assert isinstance(handler.type, ast.Name)
|
|
assert handler.type.id == "Exception"
|
|
return
|
|
raise AssertionError(
|
|
"fix_sentencepiece_gguf try block not found in unsloth_save_pretrained_gguf"
|
|
)
|
|
|
|
|
|
def test_tokenizer_utils_uses_import_protobuf_fallback_pattern():
|
|
with open(_TOK_PY) as f:
|
|
src = f.read()
|
|
tree = ast.parse(src)
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.FunctionDef) and node.name == "fix_sentencepiece_gguf":
|
|
fn_src = ast.unparse(node)
|
|
assert "import_protobuf" in fn_src
|
|
return
|
|
raise AssertionError("fix_sentencepiece_gguf not found in tokenizer_utils.py")
|
|
|
|
|
|
def test_all_special_tokens_are_gated_by_tokenizer_json_not_by_type(tmp_path):
|
|
pieces = [
|
|
("<s>", 0.0, CONTROL),
|
|
("a", -1.0, NORMAL),
|
|
("<n1>", -1.0, NORMAL),
|
|
("<u1>", -1.0, USER_DEFINED),
|
|
]
|
|
(tmp_path / "tokenizer.model").write_bytes(_build(pieces))
|
|
fix_sentencepiece_gguf(str(tmp_path))
|
|
got = dict(_read(str(tmp_path / "tokenizer.model")))
|
|
assert got["<n1>"] == NORMAL
|
|
assert got["<u1>"] == USER_DEFINED
|