Skip to content
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.

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

add gpt-tokenizer to comparison #3

Merged

Conversation

niieani
Copy link
Contributor

@niieani niieani commented Jun 1, 2023

Thanks for the nudge @transitive-bullshit. I've fixed the default import issue you encountered in niieani/gpt-tokenizer#11.

┌─────────┬───────────────────────────────────┬───────────────────┬───────────────┐
│ (index) │             Task Name             │ Average Time (ms) │ Variance (ms) │
├─────────┼───────────────────────────────────┼───────────────────┼───────────────┤
│    0    │         'gpt3-tokenizer'          │       33615       │    126310     │
│    1    │          'gpt-3-encoder'          │       19330       │    105469     │
│    2    │       'gpt-tokenizer gpt2'        │       12978       │     1539      │
│    3    │ 'gpt-tokenizer text-davinci-003'  │       12401       │      578      │
│    4    │       '@dqbd/tiktoken gpt2'       │       4411        │     1215      │
│    5    │ '@dqbd/tiktoken text-davinci-003' │       4295        │      69       │
│    6    │       'tiktoken-node gpt2'        │       3822        │      56       │
│    7    │ 'tiktoken-node text-davinci-003'  │       3824        │      57       │
└─────────┴───────────────────────────────────┴───────────────────┴───────────────┘

@socket-security
Copy link

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

🚮 Removed packages: @trivago/prettier-plugin-sort-imports@4.1.1, tsup@6.6.3

@transitive-bullshit
Copy link
Owner

transitive-bullshit commented Jun 1, 2023

This looks awesome @niieani. Thanks! 🙏

The test failures appear to be unrelated to this PR and related to upgrading to latest pnpm. Will merge and update readme shortly.

@transitive-bullshit transitive-bullshit merged commit 6b297c1 into transitive-bullshit:main Jun 2, 2023
1 of 6 checks passed
@transitive-bullshit
Copy link
Owner

Thanks again @niieani. Just updated readme w/ the latest stats and output from my local tests.

I'm still getting some import errors when running via tsx vs compiling and running the node.js binaries directly, and I know first-hand how tricky all the different combos of formats and imports can be to get right. Attached is a screenshot if it's helpful that should be pretty easy to reproduce (npx tsx src/index.ts which should "just work" if the package exports are formatted correctly).

CleanShot 2023-06-01 at 22 24 10@2x

@niieani
Copy link
Contributor Author

niieani commented Jun 2, 2023

Thanks for flagging @transitive-bullshit. This looks like an issue with tsx, since the imports work correctly when running native node. I'll submit an issue in their repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants