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

Bump llama_tokenize APIs to latest specs #730

Closed
wants to merge 3 commits into from

Conversation

manishshettym
Copy link

@manishshettym manishshettym commented Sep 18, 2023

The llama_tokenize and llama_tokenize_with_model APIs are out-dated with respect to the equivalent APIs in llama.cpp (as shown below):

  1. https://github.com/ggerganov/llama.cpp/blob/7ddf185537b712ea0ccbc5f222ee92bed654914e/llama.h#L374
  2. https://github.com/ggerganov/llama.cpp/blob/7ddf185537b712ea0ccbc5f222ee92bed654914e/llama.h#L382

ISSUE: Particularly, they are missing an argument text_len which denotes the length of the text/prompt being tokenized. This causes segmentation faults, as the wrong arguments get passed on to the C++ library.

This PR adds the required arguments to llama_cpp.py. cc: @abetlen

@manishshettym
Copy link
Author

Ref to the relevant change in llama.cpp's API (two days ago): ggerganov/llama.cpp#3170

@manishshettym
Copy link
Author

closing this PR since the changes were made directly by @abetlen! Thank you.

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.

1 participant