dotnet / dotnet/machinelearning

BPE Tokenizer doesn't allow reading carriage return symbol

Open
#6,800 0 comments 0 reactions 0 assignees View on GitHub
area-Tokenizers untriaged
Dominant language
C#
Stars
9.4k
Forks
2k
Avg merge
2d 20h
Merged PRs (30d)
11

Description

**System Information (please complete the following information):**
- OS & Version: windows 11
- ML.NET Version: [0.20.1](https://www.nuget.org/packages/Microsoft.ML.Tokenizers/0.20.1)
- .NET Version: .NET 7

**Describe the bug**
BPE Tokenizer doesn't allow reading carriage return symbol.
https://github.com/dotnet/machinelearning/blob/077a6b81966dc2c514572568917f36cb94e08ac4/src/Microsoft.ML.Tokenizers/Model/BPE.cs#L297
Reading text with ReadLines causes inability to read the carriage return symbol in merges.

**To Reproduce**
Steps to reproduce the behavior:
```csharp
using System.Text.Json;
using Microsoft.ML.Tokenizers;

var vocabFilePath = @"vocab.json";
var mergeFilePath = @"merges.txt";
try
{
var tokenizer = new Tokenizer(new Bpe(vocabFilePath, mergeFilePath, ""));

var input = " Test String ";

var tokenizerEncodedResult = tokenizer.Encode(input);
Console.WriteLine(JsonSerializer.Serialize(tokenizerEncodedResult.Ids));
var tokenizerDecodedResult = tokenizer.Decode(tokenizerEncodedResult.Ids);
Console.WriteLine(tokenizerDecodedResult);
}
catch (Exception e)
{
Console.WriteLine(e);
}
```

```
System.InvalidOperationException: Invalid merger file format at line: 3869
at Microsoft.ML.Tokenizers.Bpe.ConvertMergesToHashmap(String mergesFile)
at Microsoft.ML.Tokenizers.Bpe.ReadFile(String vocab, String merges)
at Microsoft.ML.Tokenizers.Bpe..ctor(String vocabFile, String mergesFile, String unknownToken, String continuingSubwordPrefix, String endOfWordSuffix)
at Program.$(String[] args) in D:\dev\personal\ai\Program.cs:line 8
```

**Expected behavior**
BPE should process merges without exception

**Simple solution**
Read a json instead
```csharp

internal static Vec<(string, string)> ConvertMergesToHashmap(string? mergesFile)
{
if (mergesFile is null)
{
return new Vec<(string, string)>();
}

Vec<(string, string)> merges = new(1000);

int lineNumber = 0;
List mergesList;
using (Stream stream = File.OpenRead(mergesFile))
{
mergesList = JsonSerializer.Deserialize>(stream);
}
foreach (string line in mergesList)
{
lineNumber++;
if (line.StartsWith("#version", StringComparison.Ordinal) || line.Length == 0)
{
continue;
}
int index = line.IndexOf(' ');
if (index < 0 || index == line.Length - 1 || line.IndexOf(' ', index + 1) >= 0)
{
throw new InvalidOperationException($"Invalid merger file format at line: {lineNumber}");
}
merges.Push((line.Substring(0, index), line.Substring(index + 1)));
}

return merges;
}
```

**Additional context**
I was able to fix the exception. However, BPE algorithm generates different results in comparison to google's sentencepiece that is widely used in ML.

Contributor guide

Open the contributing guide

Research direction

Start in src/Microsoft.ML.Tokenizers/Model/BPE.cs at ConvertMergesToHashmap, around line 297, and reproduce the failure with the Windows-style merges.txt described in the issue. Verify that carriage-return-containing merge entries are processed without the reported invalid-format exception, then check tokenization results against the expected behavior and the noted SentencePiece difference.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.