Skip to content

Add public SentencePieceTokenizer factory methods for Unigram from vocab list and tokenizer.json#7625

Open
Copilot wants to merge 4 commits into
mainfrom
copilot/add-sentencepiece-tokenizer-api
Open

Add public SentencePieceTokenizer factory methods for Unigram from vocab list and tokenizer.json#7625
Copilot wants to merge 4 commits into
mainfrom
copilot/add-sentencepiece-tokenizer-api

Conversation

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

SentencePieceTokenizer only exposed Create(Stream) requiring a SentencePiece protobuf (.model), making it impossible to load Hugging Face JSON-only Unigram tokenizers that have no .model file.

New public APIs

From in-memory vocab:

SentencePieceTokenizer.Create(
    IEnumerable<(string Piece, float Score)> vocab,
    int unkId,
    bool addBeginningOfSentence = true,
    bool addEndOfSentence = false,
    ReadOnlySpan<byte> precompiledCharsMap = default,
    bool addDummyPrefix = true,
    bool escapeWhiteSpaces = true,
    bool treatWhitespaceAsSuffix = false,
    IReadOnlyDictionary<string, int>? specialTokens = null)

From tokenizer.json:

SentencePieceTokenizer.CreateFromTokenizerJson(
    Stream tokenizerJsonStream,
    bool addBeginningOfSentence = true,
    bool addEndOfSentence = false,
    IReadOnlyDictionary<string, int>? specialTokens = null)

CreateFromTokenizerJson reads model.vocab, model.unk_id, extracts precompiled_charsmap from a Precompiled or Sequence normalizer, and reads Metaspace pre-tokenizer settings (add_prefix_space, replacement, prepend_scheme). It validates model.type == "Unigram".

Internal changes

  • SentencePieceBaseModel: new constructor taking individual config parameters instead of ModelProto
  • SentencePieceUnigramModel: new constructors building vocab from IReadOnlyList<(string, float)>; BOS/EOS/PAD IDs auto-detected by piece name (<s>, </s>, <pad>) with SentencePiece-conventional positional fallbacks

Note on token IDs

HF tokenizer.json typically uses a different special-token ordering than the SentencePiece protobuf (e.g. <s>=0, <pad>=1, </s>=2, <unk>=3 vs. <unk>=0, <s>=1, </s>=2). Piece strings produced are identical; numeric IDs will differ by the vocab offset introduced by the extra special tokens.

…erJson APIs

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copilot AI changed the title [WIP] Add public way to construct SentencePieceTokenizer from tokenizer.json Add public SentencePieceTokenizer factory methods for Unigram from vocab list and tokenizer.json Jun 10, 2026
Copilot AI requested a review from ericstj June 10, 2026 23:42

@ericstj ericstj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this — the in-memory Create(vocab, ...) overload is clean and the ID-preservation (JSON vocab index = token id) is the right call. I implemented this same JSON-only Unigram capability recently against real Hugging Face models, and hit two correctness issues that this PR's single test model happens to mask. Details inline; summary here.

Bugs (untested by the current suite)

  1. BOS/EOS positional fallback corrupts real pieces. FindSpecialTokenId(pieces, "<s>", 1) / ("</s>", 2) fall back to positions 1/2 when the vocab has no piece literally named <s>/</s>. Many HF Unigram tokenizers don't use those names — e.g. minishlab/potion-multilingual-128M (bge-m3 family) has unk_id=1, vocab [0]="[PAD]", [1]="[UNK]", [2]=",". There eosId→2 marks "," as Control and drops it from the Viterbi trie (it can never be emitted), and bosId→1 collides with unkId and clobbers the unknown entry. This is structural (independent of addBos/addEos).

  2. Normalizer steps beyond Precompiled are silently dropped. ExtractPrecompiledCharsMap extracts only the charsmap and discards sibling normalizers. Real Unigram models often have a richer chain (potion/bge-m3: Sequence[Precompiled, Replace(punctuation spacing), Replace("\\s+"->" "), Strip]), which SentencePieceNormalizer cannot reproduce — so CreateFromTokenizerJson silently yields different tokens than HF. Since the charsmap must run before those Replace steps, they can't just be reordered into SP; at minimum this should throw on unrecognized normalizer types rather than silently ignore them.

Why the test stays green

Paraphrase-multilingual-MiniLM-L12-v2 names its specials (<s>=0, </s>=2) so the fallback never fires, and its normalizer is a single Precompiled, so the dropped-sibling path is never hit. Recommend adding fixtures that (a) place specials at non-conventional positions / omit <s>/</s>, and (b) use a Sequence normalizer with Replace/Strip, asserting against HF reference ids.

Minor

  • removeExtraWhitespaces is hard-coded true in both factories rather than derived from the JSON.
  • added_tokens from the JSON aren't auto-wired; correctness depends on the caller passing specialTokens. Worth documenting or reading them.

(Posting as comments only — not an approval or change request.)

Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceUnigramModel.cs Outdated
Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceTokenizer.cs
Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceTokenizer.cs Outdated
@ericstj

ericstj commented Jun 11, 2026

Copy link
Copy Markdown
Member

@copilot please address feedback

…end_scheme handling

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copilot AI requested a review from ericstj June 11, 2026 00:50
Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceTokenizer.cs
Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceTokenizer.cs Outdated
Comment thread src/Microsoft.ML.Tokenizers/Model/SentencePieceUnigramModel.cs Outdated
Comment thread test/Microsoft.ML.Tokenizers.Tests/UnigramTests.cs
@ericstj

ericstj commented Jun 11, 2026

Copy link
Copy Markdown
Member

@copilot address feedback

…tion, and add tests

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copilot AI requested a review from ericstj June 11, 2026 01:23
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.29238% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.61%. Comparing base (548d4d0) to head (7e32c05).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...soft.ML.Tokenizers/Model/SentencePieceTokenizer.cs 69.06% 28 Missing and 15 partials ⚠️
...t.ML.Tokenizers/Model/SentencePieceUnigramModel.cs 86.00% 8 Missing and 6 partials ⚠️
...soft.ML.Tokenizers/Model/SentencePieceBaseModel.cs 71.79% 10 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7625      +/-   ##
==========================================
+ Coverage   69.59%   69.61%   +0.01%     
==========================================
  Files        1484     1484              
  Lines      273606   274013     +407     
  Branches    27949    27999      +50     
==========================================
+ Hits       190410   190745     +335     
- Misses      75832    75885      +53     
- Partials     7364     7383      +19     
Flag Coverage Δ
Debug 69.61% <83.29%> (+0.01%) ⬆️
production 63.85% <75.53%> (+0.01%) ⬆️
test 89.65% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
test/Microsoft.ML.Tokenizers.Tests/UnigramTests.cs 94.52% <100.00%> (+1.55%) ⬆️
...soft.ML.Tokenizers/Model/SentencePieceBaseModel.cs 79.81% <71.79%> (-0.63%) ⬇️
...t.ML.Tokenizers/Model/SentencePieceUnigramModel.cs 63.13% <86.00%> (+2.74%) ⬆️
...soft.ML.Tokenizers/Model/SentencePieceTokenizer.cs 77.62% <69.06%> (-14.88%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ericstj ericstj marked this pull request as ready for review June 11, 2026 03:27
Copilot AI review requested due to automatic review settings June 11, 2026 03:27
@ericstj ericstj requested a review from tarekgh June 11, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends SentencePieceTokenizer to support Hugging Face JSON-only Unigram tokenizers by adding new public factory APIs that can construct a Unigram tokenizer from either an in-memory vocab list or a tokenizer.json stream, avoiding the current requirement for a SentencePiece .model protobuf.

Changes:

  • Add SentencePieceTokenizer.Create(IEnumerable<(string Piece, float Score)> vocab, ...) for constructing a Unigram tokenizer directly from a vocab list.
  • Add SentencePieceTokenizer.CreateFromTokenizerJson(Stream tokenizerJsonStream, ...) for parsing HF tokenizer.json (Unigram) including vocab, unk_id, precompiled charsmap, and Metaspace settings.
  • Add internal constructors/refactoring to build a SentencePieceUnigramModel from vocab pieces and config values, plus new tests covering these creation paths.
Show a summary per file
File Description
test/Microsoft.ML.Tokenizers.Tests/UnigramTests.cs Adds unit tests for vocab-based and tokenizer.json-based Unigram construction and behavior parity checks.
src/Microsoft.ML.Tokenizers/Model/SentencePieceUnigramModel.cs Adds Unigram model constructors that build vocab/trie from (piece, score) inputs and detect special tokens by name.
src/Microsoft.ML.Tokenizers/Model/SentencePieceTokenizer.cs Adds public factories for vocab and tokenizer.json, plus JSON parsing helpers for normalizer/pre-tokenizer extraction.
src/Microsoft.ML.Tokenizers/Model/SentencePieceBaseModel.cs Adds a new base-model constructor taking explicit config/token IDs instead of ModelProto.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 4

Comment on lines +72 to +80
AddBeginningOfSentence = addBos;
AddEndOfSentence = addEos;
BeginningOfSentenceToken = bosToken;
BeginningOfSentenceId = Math.Max(0, bosId);
EndOfSentenceToken = eosToken;
EndOfSentenceId = Math.Max(0, eosId);
UnknownToken = unkToken;
UnknownId = Math.Max(0, unkId);
AddDummyPrefix = addDummyPrefix;
Comment on lines +488 to +491
/// The beginning-of-sentence and end-of-sentence token IDs are auto-detected by looking for pieces
/// named <c>&lt;s&gt;</c> and <c>&lt;/s&gt;</c> in <paramref name="vocab"/>. If not found, positions 1 and 2
/// are used as fallbacks (the SentencePiece convention). Similarly, a <c>&lt;pad&gt;</c> piece is
/// detected automatically if present.
Comment on lines +558 to +568
// Validate model type
if (!root.TryGetProperty("model", out JsonElement modelElement))
{
throw new InvalidDataException("The tokenizer.json does not contain a 'model' property.");
}

if (modelElement.TryGetProperty("type", out JsonElement modelTypeElement) &&
!string.Equals(modelTypeElement.GetString(), "Unigram", StringComparison.OrdinalIgnoreCase))
{
throw new InvalidDataException($"Expected model type 'Unigram' but found '{modelTypeElement.GetString()}'.");
}
Comment on lines +610 to +616
// Extract pre_tokenizer settings
bool escapeWhiteSpaces = true;
bool treatWhitespaceAsSuffix = false;
if (root.TryGetProperty("pre_tokenizer", out JsonElement preTokenizerElement))
{
ExtractMetaspaceSettings(preTokenizerElement, ref addDummyPrefix, ref escapeWhiteSpaces, ref treatWhitespaceAsSuffix);
}
@tarekgh

tarekgh commented Jun 11, 2026

Copy link
Copy Markdown
Member

CreateFromTokenizerJson ignores model.byte_fallback

The new JSON parser never reads the byte_fallback flag from the model section. In the protobuf path, SentencePieceBaseModel sets ByteFallback = modelProto.TrainerSpec.ByteFallback (SentencePieceBaseModel.cs:36), but the new Unigram constructor passes byteFallback: false as a hardcoded literal:

: base(addBos, addEos,
       ...,
       addDummyPrefix, escapeWhiteSpaces, treatWhitespaceAsSuffix, byteFallback: false,
       precompiledCharsmap, removeExtraWhitespaces, specialTokens)

Hugging Face Unigram tokenizers serialize this as a top-level model.byte_fallback boolean, for example:

"model": {
  "type": "Unigram",
  "unk_id": 0,
  "byte_fallback": true,
  "vocab": [ ... ]
}

For any model that enables byte fallback, hardcoding false means unknown characters will be emitted as the unknown token instead of being decomposed into <0x00>..<0xFF> byte pieces, which changes both the produced token IDs and the decoded output.

Suggested fix: read the flag in CreateFromTokenizerJson and thread it through to the model constructor:

bool byteFallback = modelElement.TryGetProperty("byte_fallback", out JsonElement bf) && bf.GetBoolean();

defaulting to false when the property is absent, then pass byteFallback instead of the literal.

@tarekgh

tarekgh commented Jun 11, 2026

Copy link
Copy Markdown
Member

CreateFromTokenizerJson ignores the top-level added_tokens array

The parser determines special tokens in two ways: it auto-detects only the literal strings <s>, </s>, and <pad> via FindSpecialTokenId, and otherwise relies on the caller passing a specialTokens dictionary by hand. The added_tokens array in tokenizer.json, which is the authoritative source for special tokens and their IDs, is never read.

A typical tokenizer.json declares them like this:

"added_tokens": [
  { "id": 0, "content": "<s>",    "special": true },
  { "id": 1, "content": "<pad>",  "special": true },
  { "id": 2, "content": "</s>",   "special": true },
  { "id": 3, "content": "<unk>",  "special": true },
  { "id": 250001, "content": "<mask>", "special": true }
]

Two consequences:

  1. Models that do not use the exact <s>/</s>/<pad> strings (for example [CLS], [SEP], [MASK], [PAD], [UNK], or <bos>) get none of their special tokens recognized. These are exactly the JSON-only Unigram layouts this PR targets (the issue mentions [PAD]=0, [UNK]=1). Such tokens stay classified as Normal pieces, so they are not treated as special during encoding and considerSpecialTokens has nothing to act on.
  2. Callers are forced to re-supply information that already exists in the file, and to keep those IDs in sync manually.

Suggested direction: read added_tokens (content plus id, optionally filtered by "special": true) and use it to populate the special-token map and to reclassify those pieces as Control, falling back to the explicit specialTokens parameter only as an override. This also makes BOS/EOS/PAD detection robust instead of depending on hardcoded literal names.

@tarekgh

tarekgh commented Jun 11, 2026

Copy link
Copy Markdown
Member

ExtractPrecompiledCharsMap throws on legitimate normalizer steps inside a Sequence

The normalizer walk handles only Precompiled and Sequence; every other type hits the else branch and throws:

else
{
    throw new NotSupportedException($"Normalizer type '{type}' is not supported. Only 'Precompiled' and 'Sequence' normalizers are supported.");
}

Because the Sequence branch calls ExtractPrecompiledCharsMap on each inner normalizer, any sibling step that is not Precompiled triggers the throw. Real Unigram tokenizers commonly nest the precompiled map alongside other steps, for example:

"normalizer": {
  "type": "Sequence",
  "normalizers": [
    { "type": "Nmt" },
    { "type": "Precompiled", "precompiled_charsmap": "..." },
    { "type": "Replace", "pattern": { "Regex": " {2,}" }, "content": " " }
  ]
}

With this input the method throws on the Nmt (or Replace) entry and the whole load fails, even though the precompiled_charsmap it needs is present and extractable. This rejects many valid tokenizer.json files.

The correct behavior here is to not throw: when walking a Sequence, skip inner steps that are not Precompiled rather than aborting, and only extract the precompiled map.

Should we have a tracking issue to support the normalizers that we currently not supporting yet?

@tarekgh

tarekgh commented Jun 11, 2026

Copy link
Copy Markdown
Member

removeExtraWhitespaces is hardcoded to true

CreateFromTokenizerJson always passes removeExtraWhitespaces: true to the model constructor. The protobuf path instead reads it from the model (SentencePieceBaseModel.cs:56, modelProto.NormalizerSpec.RemoveExtraWhitespaces), so the two entry points can disagree for the same underlying model.

Hugging Face tokenizer.json has no single field that maps directly to SentencePiece's remove_extra_whitespaces; the behavior is usually expressed through normalizer steps (for example a Strip or a Replace collapsing runs of spaces) and through the Metaspace pre-tokenizer. So unconditionally forcing true is an assumption rather than a value taken from the file.

For models where extra whitespace should be preserved, this changes normalization and therefore the resulting tokens. Two reasonable options:

  1. Keep true as the default but document it explicitly as an assumption of this loader, and consider exposing it as a parameter so callers can override.
  2. Infer it from the normalizer steps present in the JSON (for example, presence/absence of a whitespace-collapsing Replace/Strip).

@tarekgh

tarekgh commented Jun 11, 2026

Copy link
Copy Markdown
Member

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.

[API] Public way to construct SentencePieceTokenizer (Unigram) from tokenizer.json / in-memory pieces+scores

4 participants