Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Introducing Tiktoken Tokenizer - #6981

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken
Feb 6, 2024
Merged

Introducing Tiktoken Tokenizer#6981
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:Titoken

Conversation

@tarekgh

@tarekghtarekgh commented Feb 1, 2024

Copy link
Copy Markdown
Member

This modification introduces support for the Tiktoken tokenizer into the Microsoft ML tokenizers library. The logic is largely derived from the Microsoft Tokenizers Library, and the update includes optimizations and adjustments to the public APIs. Further refinements for the APIs are pending and are being tracked through issue #6982.

Usage

Tokenizertokenizer=awaitTokenizer.CreateByModelNameAsync("gpt-4");// Encoding to Idsstringtext="Hello World";IReadOnlyList<int>encoded=tokenizer.EncodeToIds(text);Assert.Equal(newList<int>(){9906,4435},encoded);Assert.Equal(text,tokenizer.Decode(encoded)!);// Full encoding to tokens, Ids, and offsetsTokenizerResultresult=tokenizer.Encode(text);Assert.Equal(newList<int>(){9906,4435},result.Ids);Assert.Equal(newstring[]{"Hello"," World"},result.Tokens);Assert.Equal(newList<(int,int)>{(0,5),(5,11)},result.Offsets);

APIs changes

namespace Microsoft.ML.Tokenizers
{
public class Tokenizer
{
+ /// <summary>+ /// Encodes input text to object has the tokens list, tokens Ids, tokens offset mapping.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public TokenizerResult Encode(string sequence, bool skipSpecialTokens); // overload adding skipSpecialTokens parameter.+ /// <summary>+ /// Encodes input text to tokens Ids.+ /// </summary>+ /// <param name="sequence">The text to tokenize.</param>+ /// <param name="skipSpecialTokens">Indicate if want to skip the special tokens during the encoding.</param>+ /// <returns>The tokenization result includes the tokens list, tokens Ids, tokens offset mapping.</returns>+ public IReadOnlyList<int> EncodeToIds(string sequence, bool skipSpecialTokens = false);+ /// <summary>+ /// Create tokenizer based on model name+ /// </summary>+ /// <param name="modelName">Model name</param>+ /// <param name="extraSpecialTokens">Extra special tokens other than the built-in ones for the model</param>+ /// <param name="normalizer">To normalize the text before tokenization</param>+ /// <returns>The tokenizer</returns>+ public static async Task<Tokenizer> CreateByModelNameAsync(+ string modelName,+ IReadOnlyDictionary<string, int>? extraSpecialTokens = null,+ Normalizer? normalizer = null)
}
- public class Split : IEquatable<Split>+ public readonly struct Split : IEquatable<Split>
{
- public Split(string token, (int Index, int End) offset)+ public Split(string token, (int Index, int End) offset, bool isSpecialToken = false)+ /// <summary>+ /// Gets if the current Split is a special token.+ /// </summary>+ public bool IsSpecialToken { get; }
}
public abstract class PreTokenizer
{
+ // Primarily focused on optimizing to minimize memory allocations and enable the enumeration of one item at a time,+ // rather than holding a large list in a collection.+ // This change will reflect in all public classes which implementing this interface.- public abstract IReadOnlyLIst<Split> PreTokenize(string sentence);+ public abstract IEnumerable<Split> PreTokenize(string sentence, bool skipSpecialTokens = false);
}
public sealed class TokenizerResult
{
- public TokenizerResult(string originalString, string normalizedString, IReadOnlyList<Split> splits, bool offsetsMappedToOriginalString);+ public TokenizerResult(string originalString, string normalizedString, IEnumerable<Split> splits, bool offsetsMappedToOriginalString);
}
public abstract class Model
{
+ public virtual IReadOnlyList<Token> Tokenize(string sequence, bool isSpecialToken); // overload to add isSpecialToken parameter.+ public virtual bool TokenizeToIds(string sequence, bool isSpecialToken, List<int> accumulatedIds); // To be consumed by Tokenizer.EncodeToIds+ public virtual int? TokenToId(string token, bool skipSpecialTokens); // overload to add isSpecialToken parameter.
}
+ public sealed class Tiktoken : Model+ {+ public Tiktoken(string tikTokenBpeFile, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public Tiktoken(Stream tikTokenBpeFileStream, IReadOnlyDictionary<string, int>? specialTokensEncoder = null, int cacheSize = DefaultCacheSize);+ public IReadOnlyDictionary<string, int>? SpecialTokens { get; }+ // Implement the Model abstract methods+ }+ public sealed class TikTokenPreTokenizer : PreTokenizer+ {+ public TikTokenPreTokenizer(string regexPattern, IReadOnlyDictionary<string, int>? specialTokensEncoder);+ // Implement the Model abstract methods+ }

@ghostghost assigned tarekghFeb 1, 2024
@tarekgh

Copy link
Copy Markdown
MemberAuthor

@codecov

codecovBot commented Feb 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 210 lines in your changes are missing coverage. Please review.

Comparison is base (902102e) 68.80% compared to head (35e2cbc) 68.81%.

❗ Current head 35e2cbc differs from pull request most recent head 4cd96b3. Consider uploading reports for the commit 4cd96b3 to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## main #6981 +/- ##
==========================================
+ Coverage 68.80% 68.81% +0.01% 
==========================================
Files 1249 1256 +7 Lines 249686 250425 +739 Branches 25485 25569 +84 ==========================================
+ Hits 171795 172335 +540 - Misses 71294 71466 +172 - Partials 6597 6624 +27 
FlagCoverage Δ
Debug68.81% <72.62%> (+0.01%)⬆️
production63.28% <66.87%> (+0.01%)⬆️
test88.44% <100.00%> (+0.02%)⬆️

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

FilesCoverage Δ
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/TokenizerResult.cs100.00% <100.00%> (+9.09%)⬆️
...Microsoft.ML.Tokenizers.Tests/PreTokenizerTests.cs95.31% <100.00%> (ø)
test/Microsoft.ML.Tokenizers.Tests/TitokenTests.cs100.00% <100.00%> (ø)
...rc/Microsoft.ML.Tokenizers/PreTokenizer/Roberta.cs57.14% <33.33%> (-19.79%)⬇️
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs95.23% <95.23%> (ø)
...crosoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs83.33% <81.48%> (-7.58%)⬇️
...Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs65.38% <65.38%> (ø)
src/Microsoft.ML.Tokenizers/Model/Model.cs7.69% <7.69%> (ø)
src/Microsoft.ML.Tokenizers/Utils/LruCache.cs66.66% <66.66%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
return true;
}

int[] encodedIds = BytePairEncoder.BytePairEncode(Encoding.UTF8.GetBytes(sequence), _encoder);

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.

It'd be really nice to reduce the overheads here. It can be done separately, but this is a lot of allocation.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
}
}

return utf8Bytes.Count > 0 ? Encoding.UTF8.GetString(utf8Bytes.ToArray()) : string.Empty;

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.

Do we only target netstandard2.0, or do we multitarget and build this for netcoreapp as well? There are newer APIs that make this cheaper.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Tracked through the issue #6989

Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Utils/ByteArrayComparer.cs
return outList;
}

private static T[] Slice<T>(this T[] array, int start, int end)

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.

There looks to be a fair amount of allocation being incurred from all this slicing. That can't be reduced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tracked this in the issue #6989

@tarekghtarekgh mentioned this pull request Feb 5, 2024
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj Outdated
Comment threadsrc/Microsoft.ML.Tokenizers/Microsoft.ML.Tokenizers.csproj
Comment threadsrc/Microsoft.ML.Tokenizers/PreTokenizer/PreTokenizer.cs

@michaelgsharpmichaelgsharp 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.

LGTM. Just that one question about the empty dispose (though I saw it in a couple other places too, my question applies there too)

@tarekgh
tarekgh merged commit 6f55525 into dotnet:mainFeb 6, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 8, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tarekgh@stephentoub@ericstj@michaelgsharp