Skip to content

fix(python): forward unk_id in SentencePieceUnigramTokenizer - #2393

Open
Con-Benksl wants to merge 1 commit into
huggingface:mainfrom
Con-Benksl:fix/sentencepiece-unigram-unk-id
Open

fix(python): forward unk_id in SentencePieceUnigramTokenizer#2393
Con-Benksl wants to merge 1 commit into
huggingface:mainfrom
Con-Benksl:fix/sentencepiece-unigram-unk-id

Conversation

@Con-Benksl

Copy link
Copy Markdown

Fixes #1576.

Even with <unk> in a custom vocabulary, SentencePieceUnigramTokenizer cannot encode unknown characters because its constructor does not expose the underlying Unigram model's unk_id. Add an optional trailing parameter and forward it to the existing model constructor, preserving the default behavior and positional arguments. Also pass unk_id by keyword in the parity checker, where it was incorrectly bound to replacement.

Validation on Python 3.13.13 with the native extension built from this checkout:

  • PYTHONPATH=py_src HF_HUB_OFFLINE=1 HF_DATASETS_OFFLINE=1 python -m pytest -q tests/implementations/test_sentencepiece.py: 7 passed; before the fix, the two new cases failed and the five existing cases passed. The regression covers zero/nonzero IDs, default behavior, positional compatibility, and missing-vocabulary validation.
  • Ran the actual parity checker against a tokenizer trained locally on three short sentences, with unk_id=2 and an unseen character in the input: all three encodings matched SentencePiece. The original call failed with a TypeError.
  • make check-style passed; final ruff checks, formatting checks, and the repository's ty check also passed after excluding unrelated generated stub changes from the diff.

No model files were downloaded. The full network-dependent test suite was not run.

AI assistance: OpenAI Codex implemented and tested this change; a separate Codex agent reviewed the diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with SentencePieceUnigramTokenizer Handling Unknown Tokens

1 participant