Skip to content

Use external hf_tokenizer in llama runner #9112

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
Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Use external hf_tokenizer in llama runner #9112

wants to merge 23 commits into from

Conversation

jackzhxng
Copy link
Contributor

@jackzhxng jackzhxng commented Mar 10, 2025 •
edited
Loading

Summary

Use https://github.com/pytorch-labs/tokenizers huggingface tokenizer in the Llama runner.

Results on Qwen2.5 with extension/llm/tokenizers checked out to pytorch-labs/tokenizers#50:

Once upon a time,  there was a little girl named Lily. She was very happy. She had a big garden in the back of her house. She planted many flowers in it. They were red, yellow and blue. They were very pretty. Lily loved them very much. One day, she was watering them. Suddenly, she heard a noise. It was a noise in the tree. She looked up. There was a big bird in the tree. It was eating one of Lily's flowers. Lily was very angry. She ran to the tree. "Hello!" she said to the bird. "What are you doing in my
I 00:00:08.624959 executorch:runner.cpp:294] RSS after finishing text generation: 2147.121094 MiB (0 if unsupported)
PyTorchObserver {"prompt_tokens":4,"generated_tokens":123,"model_load_start_ms":1744936315023,"model_load_end_ms":1744936318524,"inference_start_ms":1744936318524,"inference_end_ms":1744936323646,"prompt_eval_end_ms":1744936318580,"first_token_ms":1744936318580,"aggregate_sampling_time_ms":274877907025,"SCALING_FACTOR_UNITS_PER_SECOND":1000}
I 00:00:08.625019 executorch:stats.h:106]       Prompt Tokens: 4    Generated Tokens: 123
I 00:00:08.625021 executorch:stats.h:112]       Model Load Time:                3.501000 (seconds)
I 00:00:08.625023 executorch:stats.h:119]       Total inference time:           5.122000 (seconds)               Rate:  24.014057 (tokens/second)
I 00:00:08.625033 executorch:stats.h:129]               Prompt evaluation:      0.056000 (seconds)               Rate:  71.428571 (tokens/second)
I 00:00:08.625038 executorch:stats.h:138]               Generated 123 tokens:   5.066000 (seconds)               Rate:  24.279510 (tokens/second)
I 00:00:08.625045 executorch:stats.h:149]       Time to first generated token:  0.056000 (seconds)
I 00:00:08.625047 executorch:stats.h:155]       Sampling time over 127 tokens:  274877907.025000 (seconds)

Test plan

Build llama runner locally (note the inclusion of -DSUPPORT_REGEX_LOOKAHEAD=ON):

cmake -DPYTHON_EXECUTABLE=python \
    -DCMAKE_INSTALL_PREFIX=cmake-out \
    -DCMAKE_BUILD_TYPE=Release \
    -DEXECUTORCH_BUILD_KERNELS_CUSTOM=ON \
    -DEXECUTORCH_BUILD_KERNELS_OPTIMIZED=ON \
    -DEXECUTORCH_BUILD_XNNPACK=ON \
    -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON \
    -DSUPPORT_REGEX_LOOKAHEAD=ON \
    -Bcmake-out/examples/models/llama \
    examples/models/llama

cmake --build cmake-out/examples/models/llama -j16 --config Release

Run on Qwen2.5:

cmake-out/examples/models/llama/llama_main --model_path=qwen2_5.pte --tokenizer_path ~/hf/models--Qwen--Qwen2.5-1.5B/snapshots/8faed761d45a263340a0528343f099c05c9a4323/tokenizer.json --prompt="Once upon a time" --temperature 0

@jackzhxng jackzhxng requested a review from kirklandsign March 10, 2025 23:34
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Mar 10, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9112

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 4ff5d8b with merge base c5dd476 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 10, 2025
@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from de6b735 to a536432 Compare March 10, 2025 23:53
jackzhxng
jackzhxng commented Mar 11, 2025
View reviewed changes
Copy link
Contributor Author

@jackzhxng jackzhxng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update - will change to use extension/llm/tokenizers/src/hf_tokenizer.cpp as real impl

@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from 08e7b04 to 4d29fda Compare March 14, 2025 23:08
@jackzhxng jackzhxng requested a review from swolchok as a code owner March 14, 2025 23:08
swolchok
swolchok requested changes Mar 17, 2025
View reviewed changes
@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch 4 times, most recently from 9b69efe to 51323bc Compare March 24, 2025 12:00
@jackzhxng jackzhxng changed the title Add stub impl of json tokenizer to llama runner Use external hf_tokenizer in llama runner Mar 24, 2025
jackzhxng
jackzhxng commented Mar 26, 2025
View reviewed changes
Copy link
Contributor Author

@jackzhxng jackzhxng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swolchok This PR changed to directly use the hf tokenizer from https://github.com/pytorch-labs/tokenizers, can you give it a re-review? Most of your comments don't apply anymore as a result, but I addressed the ones that do

@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from 75dd028 to 63b2ed7 Compare March 26, 2025 12:48
swolchok
swolchok approved these changes Apr 2, 2025
View reviewed changes
Copy link
Contributor

@swolchok swolchok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine. sorry for the slow turnaround; feel free to ping me after it's been a day

@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from 63b2ed7 to dc8a31a Compare April 15, 2025 21:15
@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@jackzhxng jackzhxng requested a review from cccclai as a code owner April 28, 2025 04:38
@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from 031d404 to 7fdb8ae Compare April 28, 2025 04:49
@jackzhxng jackzhxng force-pushed the jz/cpp-no-op-tokenizer branch from 7d210be to 846951e Compare April 28, 2025 06:16
@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

guangy10
guangy10 reviewed Apr 29, 2025
View reviewed changes
examples/models/llama/runner/runner.cpp
Comment on lines +43 to +47
auto json_tokenizer = std::make_unique<tokenizers::HFTokenizer>();
if (json_tokenizer->load(tokenizer_path) == ::tokenizers::Error::Ok) {
ET_LOG(Info, "Loaded json tokenizer");
return json_tokenizer;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are custom special tokens, added tokens, etc already included in the tokenizer.json? Personally I haven't validated this yet, but I think we should before we claim HF tokenizer is supported.

@facebook-github-bot
Copy link
Contributor

@jackzhxng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

@guangy10 guangy10 guangy10 left review comments

@swolchok swolchok swolchok approved these changes

@kirklandsign kirklandsign kirklandsign approved these changes

@iseeyuan iseeyuan Awaiting requested review from iseeyuan iseeyuan is a code owner

@larryliu0820 larryliu0820 Awaiting requested review from larryliu0820 larryliu0820 is a code owner

@lucylq lucylq Awaiting requested review from lucylq lucylq is a code owner

@cccclai cccclai Awaiting requested review from cccclai cccclai is a code owner

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jackzhxng @facebook-github-bot @swolchok @larryliu0820 @guangy10 @kirklandsign