System Information (please complete the following information):
- OS & Version: Windows 11
- ML.NET Version: Microsoft.ML.Tokenizers 3.0.0-preview.26457.2, same code on main at 681cfb6
- .NET Version: .NET 10.0
Describe the bug
SentencePieceTokenizer.Create(Stream) throws IndexOutOfRangeException for a Unigram model trained without a BOS token, such as T5 (bos_id = -1), whatever addBeginningOfSentence is. Building a Unigram tokenizer from a vocabulary or a tokenizer.json already treats a missing BOS as absent, and only throws ArgumentException when it is requested.
To Reproduce
using Stream stream = File.OpenRead("spiece.model"); // https://huggingface.co/google-t5/t5-small
SentencePieceTokenizer tokenizer = SentencePieceTokenizer.Create(stream, addBeginningOfSentence: false, addEndOfSentence: true);System.IndexOutOfRangeException: Index was outside the bounds of the array.
at Microsoft.ML.Tokenizers.SentencePieceUnigramModel..ctor(ModelProto modelProto, Boolean addBos, Boolean addEos, IReadOnlyDictionary`2 specialTokens)
at Microsoft.ML.Tokenizers.SentencePieceTokenizer..ctor(ModelProto modelProto, Boolean addBos, Boolean addEos, IReadOnlyDictionary`2 specialTokens)
at Microsoft.ML.Tokenizers.SentencePieceTokenizer.Create(Stream modelStream, Boolean addBeginningOfSentence, Boolean addEndOfSentence, IReadOnlyDictionary`2 specialTokens)
Every Unigram model trained without BOS fails the same way. Of these models from the Hugging Face hub, all but the two with a BOS fail:
Model (spiece.model / sentencepiece.bpe.model) |
bos_id |
SentencePieceTokenizer.Create |
google-t5/t5-small |
-1 |
IndexOutOfRangeException |
google-t5/t5-base |
-1 |
IndexOutOfRangeException |
google/t5-v1_1-small |
-1 |
IndexOutOfRangeException |
google/flan-t5-small |
-1 |
IndexOutOfRangeException |
google/flan-t5-base |
-1 |
IndexOutOfRangeException |
google/mt5-small |
-1 |
IndexOutOfRangeException |
google/mt5-base |
-1 |
IndexOutOfRangeException |
google/long-t5-tglobal-base |
-1 |
IndexOutOfRangeException |
google/pegasus-xsum |
-1 |
IndexOutOfRangeException |
google/umt5-small |
2 |
loads |
FacebookAI/xlm-roberta-base |
1 |
loads |
Expected behavior
As in SentencePiece 0.2.2: the model loads and Translate English to German: That is good. encodes to 30355, 15, 1566, 12, 2968, 10, 466, 19, 207, 5, 1, while asking for a BOS fails, since the model defines none ("BOS token is not defined as a control symbol in this model").
Additional context
The ModelProto constructor of SentencePieceUnigramModel writes _vocabReverse[BosId] with no check (Debug.Assert(BosId >= 0)), and SentencePieceBaseModel turns a missing id into 0 with Math.Max(0, BosId), which for T5 is <pad>. The constructor that builds the model from a vocabulary keeps an absent BOS at -1 through CheckSpecialId and DefaultAffix. I have a fix that does the same for a ModelProto, with tests built from a synthetic model, and would like to contribute it.
System Information (please complete the following information):
Describe the bug
SentencePieceTokenizer.Create(Stream)throwsIndexOutOfRangeExceptionfor a Unigram model trained without a BOS token, such as T5 (bos_id = -1), whateveraddBeginningOfSentenceis. Building a Unigram tokenizer from a vocabulary or atokenizer.jsonalready treats a missing BOS as absent, and only throwsArgumentExceptionwhen it is requested.To Reproduce
Every Unigram model trained without BOS fails the same way. Of these models from the Hugging Face hub, all but the two with a BOS fail:
spiece.model/sentencepiece.bpe.model)bos_idSentencePieceTokenizer.Creategoogle-t5/t5-smallIndexOutOfRangeExceptiongoogle-t5/t5-baseIndexOutOfRangeExceptiongoogle/t5-v1_1-smallIndexOutOfRangeExceptiongoogle/flan-t5-smallIndexOutOfRangeExceptiongoogle/flan-t5-baseIndexOutOfRangeExceptiongoogle/mt5-smallIndexOutOfRangeExceptiongoogle/mt5-baseIndexOutOfRangeExceptiongoogle/long-t5-tglobal-baseIndexOutOfRangeExceptiongoogle/pegasus-xsumIndexOutOfRangeExceptiongoogle/umt5-smallFacebookAI/xlm-roberta-baseExpected behavior
As in SentencePiece 0.2.2: the model loads and
Translate English to German: That is good.encodes to30355, 15, 1566, 12, 2968, 10, 466, 19, 207, 5, 1, while asking for a BOS fails, since the model defines none ("BOS token is not defined as a control symbol in this model").Additional context
The
ModelProtoconstructor ofSentencePieceUnigramModelwrites_vocabReverse[BosId]with no check (Debug.Assert(BosId >= 0)), andSentencePieceBaseModelturns a missing id into 0 withMath.Max(0, BosId), which for T5 is<pad>. The constructor that builds the model from a vocabulary keeps an absent BOS at -1 throughCheckSpecialIdandDefaultAffix. I have a fix that does the same for aModelProto, with tests built from a synthetic model, and would like to contribute it.