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

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.