Add tokenize_batch method
#14
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
michael-p
force-pushed
the
tokenize-batch
branch
from
1435509 to
b055c73
Compare
| regex = "1.10.2" | ||
|
|
||
| [dev-dependencies] | ||
| criterion = "0.5.1" | ||
|
|
||
| [[bench]] | ||
| name = "bench" | ||
| required-features = ["openai-vocabulary-file"] | ||
| required-features = ["ndarray", "openai-vocabulary-file"] |
There was a problem hiding this comment.
We should avoid this and only guard the tokenize_batch_small benchmark on this.
There was a problem hiding this comment.
I moved the benchmark which requires the (now optional) ndarray feature to a second benchmark harness.
There was a problem hiding this comment.
Why is this better than just guarding the single benchmark?
There was a problem hiding this comment.
I'm not sure guarding a single benchmark is even possible in Criterion without much effort. I didn't find anything in the documentation, and tried several combinations while defining the criterion_group!(...) and criterion_main!(...) but it looks like there is no easy way to feature-gate individual benchmarks (except probably duplicating both criterion_group!(...) and criterion_main!(...)).
In addition, it's maybe a bit more discoverable. I tend to just run cargo bench on projects I'm not familiar with (instead of cargo bench --all-features) and would hence miss those benchmarks. By having two benchmark .rs files I might realize that there are actually more benchmarks I can run.
michael-p
force-pushed
the
tokenize-batch
branch
from
b055c73 to
6fb71dd
Compare
michael-p
force-pushed
the
tokenize-batch
branch
from
6fb71dd to
ca501b2
Compare
| regex = "1.10.2" | ||
|
|
||
| [dev-dependencies] | ||
| criterion = "0.5.1" | ||
|
|
||
| [[bench]] | ||
| name = "bench" | ||
| required-features = ["openai-vocabulary-file"] | ||
| required-features = ["ndarray", "openai-vocabulary-file"] |
There was a problem hiding this comment.
Why is this better than just guarding the single benchmark?
The Python bindings provide this method as a convenience function. I think this is useful for users of the Rust library as well, so I implemented it there. This means we have to take on a dependency on
ndarray, but I think this is justified given how widely used this crate is in the ML ecosystem.