Address the feedback on the tokenizer's library - #7024

Merged
tarekgh merged 23 commits into
dotnet:mainfrom
tarekgh:MiscFixes
Feb 26, 2024
Merged

Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom
tarekgh:MiscFixes

Conversation

@tarekgh

@tarekghtarekgh commented Feb 23, 2024

Copy link
Copy Markdown
Member

@tarekgh

tarekgh commented Feb 23, 2024

Copy link
Copy Markdown
MemberAuthor

CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud

@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov

codecovBot commented Feb 23, 2024•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.

Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).

Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️

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

FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more

... and 7 files with indirect coverage changes

stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
{
var mergeRanks = new Dictionary<(string, string), int>();
var mergeRanks = new Cache<(string, string), int>(60_000);

@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?

@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.

stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
/// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
/// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
/// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>

@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.

@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)

stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs Outdated
stephentoub
stephentoub reviewed Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs Outdated
@tarekgh
tarekgh merged commit d0aa2c2 into dotnet:mainFeb 26, 2024
@ericstjericstj mentioned this pull request Feb 12, 2024
Closed
4 tasks
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub
, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub

, '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
Address the feedback on the tokenizer's library - #7024
Merged
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Feb 26, 2024
Merged
Address the feedback on the tokenizer's library#7024
tarekgh merged 23 commits into
dotnet:mainfrom 
tarekgh:MiscFixes
Conversation
@tarekgh
@tarekghtarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

Member
@tarekgh
tarekgh
commented
Feb 23, 2024

Copy link


Copy Markdown

MemberAuthor
CC @ericstj@michaelgsharp@luisquintanilla@LittleLittleCloud
@tarekgh
Fix cache when calling EncodeToIds
f6e32f5
@tarekgh
Make EnglishRoberta _mergeRanks thread safe
0553922
@tarekgh
Delete Trainer
a4cb1f5
@tarekgh
Remove the setters on the Bpe properties
6a13025
@tarekgh
Remove Roberta and Tiktoken special casing in the Tokenizer and suppo…
3278aff
…rt the cases in the Model abstraction
@tarekgh
Support text-embedding-3-small/large embedding
b5f7fa2
@tarekgh
Remove redundant TokenToId abstraction and keep the one with the extr…
a11f4e0
…a parameters
@tarekgh
Enable creating Tiktoken asynchronously or directly using the tokeniz…
865068a
…er data
@tarekgh
Add cancellationToken support in CreateAsync APIs
4077de0
@tarekgh
Rename sequence to text and Tokenize to Encode
5aaf849
@tarekgh
Rename skipSpecialTokens to considerSpecialTokens
b5e0927
@tarekgh
Rename TokenizerResult to EncodingResult
5e26b3e
@tarekgh
Make Token publicly immutable
985de8a
@tarekgh
Change offset tuples from (Index, End) to (Index, Length)
b551e7d
@tarekgh
Rename NormalizedString method's parameters
7ea7f70
@tarekgh
Rename Model's methods to start with verb
b0c8244
@tarekgh
Convert Model.GetVocab() method to a Vocab property
450418a
@tarekgh
Some method's parameters and variable renaming
6f53de8
@tarekgh
Remove Vocab and VocabSize from the abstraction
62334c6
@tarekgh
Cleanup normalization support
d48b32d
@tarekgh
Minor Bpe cleanup
191ab03
@tarekgh
Resolve rebase change
b9b0f58
@codecov
codecovBot
commented
Feb 23, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 76.65953% with 109 lines in your changes are missing coverage. Please review.
Project coverage is 68.79%. Comparing base (4b89d98) to head (1ad157f).
Additional details and impacted files
@@ Coverage Diff @@## main #7024 +/- ##
==========================================
- Coverage 68.83% 68.79% -0.04% 
==========================================
Files 1258 1254 -4 Lines 250672 250204 -468 Branches 25615 25529 -86 ==========================================
- Hits 172547 172125 -422 + Misses 71493 71468 -25 + Partials 6632 6611 -21 
FlagCoverage Δ
Debug68.79% <76.65%> (-0.04%)⬇️
production63.22% <66.24%> (-0.05%)⬇️
test88.50% <98.66%> (-0.07%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
FilesCoverage Δ
src/Microsoft.ML.Tokenizers/EncodingResult.cs98.41% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Model/Word.cs58.75% <100.00%> (-25.63%)⬇️
...ft.ML.Tokenizers/Normalizer/LowerCaseNormalizer.cs100.00% <100.00%> (ø)
...ft.ML.Tokenizers/Normalizer/UpperCaseNormalizer.cs100.00% <100.00%> (ø)
...ML.Tokenizers/PreTokenizer/TikTokenPreTokenizer.cs90.24% <100.00%> (ø)
...Microsoft.ML.Tokenizers/PreTokenizer/Whitespace.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Tokenizers/Token.cs100.00% <100.00%> (ø)
...c/Microsoft.ML.Tokenizers/Utils/BytePairEncoder.cs88.23% <ø> (ø)
...ft.ML.TorchSharp/Extensions/TokenizerExtensions.cs87.50% <100.00%> (ø)
src/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs91.10% <100.00%> (ø)
... and 14 more
... and 7 files with indirect coverage changes
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Cache.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
 private Cache<(string, string), int> GetMergeRanks(Stream mergeStream)
 {
 var mergeRanks = new Dictionary<(string, string), int>();
 var mergeRanks = new Cache<(string, string), int>(60_000);
@stephentoubstephentoubFeb 23, 2024

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.

Where does this 60k come from?
@tarekghtarekghFeb 23, 2024

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.

The loaded data from the merge file is 50K. I give it 10K more to grow.
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BpeTrainer.cs
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
 /// <param name="continuingSubwordPrefix">The prefix to attach to sub-word units that don’t represent a beginning of word.</param>
 /// <param name="endOfWordSuffix">The suffix to attach to sub-word units that represent an end of word.</param>
 public Bpe(string vocabFile, string? mergesFile, string? unknownToken = null, string? continuingSubwordPrefix = null, string? endOfWordSuffix = null) :
 /// <param name="fuseUnknownTokens">Indicate whether allowing multiple unknown tokens get fused.</param>
@stephentoubstephentoubFeb 23, 2024

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.

I'm having trouble understanding what this means.
@tarekghtarekghFeb 24, 2024

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.

If encoding text with Bpe model, for the tokens that the model doesn't recognize, it uses the unknown token for it. Most users uses [Unk] for the unknow token. It is possible to get multiple [Unk] tokens next to each others in the result. Settings fuseUnknownTokens to true cause all [Unk] sequence to collapse into one [Ukn]. Fuse term is used by Huggingface and users of Bpe are familiar with that. If you have better explanation we can use here I'll be happy to use it :-)
stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/BPE.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/EnglishRoberta.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadtest/Microsoft.ML.Tokenizers.Tests/Microsoft.ML.Tokenizers.Tests.csproj
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Model.cs
Outdated

stephentoub
stephentoub
reviewed
Feb 23, 2024
View reviewed changes
Comment threadsrc/Microsoft.ML.Tokenizers/Model/Tiktoken.cs
Outdated

@tarekgh
tarekgh
merged commit d0aa2c2
into
dotnet:mainFeb 26, 2024
@ericstjericstj
mentioned this pull request
Feb 12, 2024

Closed

4 tasks
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 28, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@tarekghtarekgh

Labels

Tokenizers

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@tarekgh@stephentoub