Skip to content

Conversation

@stephentoub
Copy link
Member

This updates two of the regexes to match the changes made in openai/tiktoken@9f7f69d.

On .NET Core, these changes are mostly nops, as the main thing they're doing is changing some loops to be atomic, and the auto-atomicity logic in the regex optimizer was already noticing that could be done and doing it automatically. On .NET Framework, it's a bigger deal, as those loops will now be atomic where they weren't previously.

If nothing else, it keeps the regexes in sync with the reference implementation.

@codecov
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.78%. Comparing base (be1e428) to head (14fff61).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7255   +/-   ##
=======================================
  Coverage   68.77%   68.78%           
=======================================
  Files        1462     1463    +1     
  Lines      272261   272288   +27     
  Branches    28176    28177    +1     
=======================================
+ Hits       187254   187297   +43     
+ Misses      77764    77748   -16     
  Partials     7243     7243           
Flag Coverage Δ
Debug 68.78% <ø> (+<0.01%) ⬆️
production 63.28% <ø> (+<0.01%) ⬆️
test 89.04% <ø> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...Microsoft.ML.Tokenizers/Model/TiktokenTokenizer.cs 77.91% <ø> (+0.08%) ⬆️

... and 10 files with indirect coverage changes

@stephentoub
Copy link
Member Author

/ba-g unrelated torchsharp crash

@stephentoub stephentoub requested a review from ericstj October 5, 2024 16:16
@michaelgsharp michaelgsharp merged commit 3f042f6 into dotnet:main Oct 7, 2024
23 of 25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2024
Sign up for free to 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.

3 participants