Add support for o200k tokenization #16
Conversation
crates/bpe/src/byte_pair_encoding.rs
Outdated
| if let Some(j) = bytes_hash_to_token.insert(hash_bytes(&token), i as u32) { | ||
| eprintln!("collision: ({i}, {j})"); | ||
| } |
There was a problem hiding this comment.
should the function return an error instead when this happens?
There was a problem hiding this comment.
If this happens, the assertion below the loop will fail. I wonder if we want to provide a way to specify the factor as part of the API, if anyone wants to construct a BPE from their own dictionary. The hard-coded constant might make it harder to reuse this if users bring their own tokens. But I'll save that for a follow-up PR.
| /// 3. Run [`update_token_dicts`] tests below to update data files. | ||
| #[test] | ||
| #[ignore = "run manually to find a suitable hash factor"] | ||
| fn find_hash_factor() { |
crates/bpe/src/byte_pair_encoding.rs
Outdated
| // TODO: Move the generation of the dictionary into some build procedure? | ||
| #[test] | ||
| fn test_serialize() { | ||
| #[ignore = "run manually to update data files"] |
There was a problem hiding this comment.
in principle, we could let this test run normally, since it will fix the broken data file (and one will see in the diff that something has changed)
There was a problem hiding this comment.
Yes, let's do that.
aneubeck
left a comment
aneubeck
left a comment
There was a problem hiding this comment.
What are the benchmark numbers for o200k?
Did things get slower?
Performance characteristics look similar. cl100k o200k |
2 participants
No description provided.