New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: FromPretrained to load tokenizer directly from HF #27

Merged

daulet merged 10 commits into daulet:main from berkayersoyy:feature/hf-download

Nov 7, 2024

Contributor

berkayersoyy commented Oct 25, 2024

Benchmark	benchmarks/eafe2ce1db50dcd78b3b7157bf2c669109f4e322.txt	benchmarks/867b67eeade51e570d5150cebddab679b855e885.txt	Change
sec/op
EncodeNTimes-10	12.75µ ± 2%	12.60µ ± 1%	-1.18% (p=0.015 n=6)
EncodeNChars-10	0.3629n ± 19%	0.3589n ± 8%	~ (p=1.000 n=6)
DecodeNTimes-10	4.433µ ± 1%	4.481µ ± 1%	+1.08% (p=0.011 n=6)
DecodeNTokens-10	660.5n ± 2%	662.0n ± 5%	~ (p=0.485 n=6)
Geomean	341.2n	340.3n	-0.25%

Benchmark	benchmarks/eafe2ce1db50dcd78b3b7157bf2c669109f4e322.txt	benchmarks/867b67eeade51e570d5150cebddab679b855e885.txt	Change
B/op
EncodeNTimes-10	232.0 ± 0%	232.0 ± 0%	~ (p=1.000 n=6)¹
EncodeNChars-10	0.000 ± 0%	0.000 ± 0%	~ (p=1.000 n=6)¹
DecodeNTimes-10	96.00 ± 0%	96.00 ± 0%	~ (p=1.000 n=6)¹
DecodeNTokens-10	7.000 ± 0%	7.000 ± 0%	~ (p=1.000 n=6)¹
Geomean			+0.00%²

Benchmark	benchmarks/eafe2ce1db50dcd78b3b7157bf2c669109f4e322.txt	benchmarks/867b67eeade51e570d5150cebddab679b855e885.txt	Change
allocs/op
EncodeNTimes-10	12.00 ± 0%	12.00 ± 0%	~ (p=1.000 n=6)¹
EncodeNChars-10	0.000 ± 0%	0.000 ± 0%	~ (p=1.000 n=6)¹
DecodeNTimes-10	3.000 ± 0%	3.000 ± 0%	~ (p=1.000 n=6)¹
DecodeNTokens-10	0.000 ± 0%	0.000 ± 0%	~ (p=1.000 n=6)¹
Geomean			+0.00%²

¹ All samples are equal
² Summaries must be >0 to compute geomean

Contributor Author

berkayersoyy commented Oct 28, 2024

I added a few things so that the tokenizer can be loaded directly from huggingface, if you could review it that would be great @daulet 🙏

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

Owner

daulet left a comment

Thank you for your contribution! In principle looks good, let's just iron out concurrency stuff.

Contributor Author

berkayersoyy commented Oct 29, 2024

I made the changes you mentioned, can you review them again? 🙏

daulet force-pushed the feature/hf-download branch from 8ec40b9 to ba3a228 Compare

November 5, 2024 01:02

daulet reviewed

View reviewed changes

tokenizer.go Outdated Show resolved Hide resolved

daulet force-pushed the feature/hf-download branch from ba3a228 to 05a1570 Compare

November 5, 2024 01:06

Owner

daulet commented Nov 5, 2024

@berkayersoyy I've made few changes instead of PR comments, lets just add tests and we are good to go

Contributor Author

berkayersoyy commented Nov 5, 2024

looks good 🙏 , I added the tests @daulet

daulet reviewed

View reviewed changes

tokenizer_test.go Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer_test.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer_test.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer_test.go Outdated Show resolved Hide resolved

daulet reviewed

View reviewed changes

tokenizer_test.go Outdated Show resolved Hide resolved

daulet requested changes

View reviewed changes

Owner

daulet left a comment

left comments

Contributor Author

berkayersoyy commented Nov 7, 2024

updated the tests 🙏 @daulet

berkayersoyy and others added 9 commits

November 7, 2024 12:28


          add LoadTokenizerFromHuggingFace function to load tokenizer directly …

ce1fe87

…from huggingface, update README.md


          using channels as unbuffered channel, update channel names and minimi…

9b9e417

…ze some approaches


          fix: rename new func to FromPretrained, improve example

0b1b908


          fix: clean up downloadFile

70dbc18


          fix: concurrency issues in case of an error

2f5c3fa


          fix: make optional params optional

52fc24a


          fix: cache path has to be model specific

2c74710


          add unit tests for FromPretrained

6aae8ce


          migrate to table driven tests, unify/simplify test cases

91ace10

daulet force-pushed the feature/hf-download branch from d8c5cab to 91ace10 Compare

November 7, 2024 20:31


          fix: clean up nits

5d79518

daulet changed the title ~~add LoadTokenizerFromHuggingFace function to load tokenizer directly …~~ feat: FromPretrained to load tokenizer directly from HF

daulet approved these changes

View reviewed changes

daulet merged commit 9c972d9 into daulet:main

1 check passed

Owner

daulet commented Nov 7, 2024

Thank you @berkayersoyy for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet