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 Embedding Related Functionality #133

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Conversation

Hirtol
Copy link
Contributor

@Hirtol Hirtol commented Mar 5, 2024

Recently llama.cpp has added the ability to generate embeddings using BERT related models. There have been some issues, namely differences between the embeddings generated in Python and the ones coming from llama.cpp, but with ggerganov/llama.cpp#5796 being merged it seems to finally be stable.

This PR implements the functionality to allow a user to make use of these new features, it adds an example based loosely on the llama.cpp embeddings example. Most of the code was translated from either llama.cpp or llama-cpp-python (specifically, the add_sequence method in LLamaBatch).

A few additional changes snuck in as well, replacing a println! call with a tracing call to allow users to filter it out, alongside a fix for a failing doc-test.

Copy link
Contributor

@MarcusDunn MarcusDunn left a comment

Choose a reason for hiding this comment

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

this looks good. one question about consolidating some unsafe.

llama-cpp-2/src/llama_batch.rs Outdated Show resolved Hide resolved
llama-cpp-2/src/llama_backend.rs Show resolved Hide resolved
@Hirtol
Copy link
Contributor Author

Hirtol commented Mar 5, 2024

Hm, the doc-test that I 'fixed' fails here. llama_token_type seems to get translated to c_uint on CI, but c_int for me locally.

EDIT: Ah, seems to be a Windows vs. Linux issue: rust-lang/rust-bindgen#1966
(as a side-note: pprof currently prevents the running of tests on Windows by default unless manually commented out)

@MarcusDunn
Copy link
Contributor

fix that and I'll add windows to the test CI so I don't break it again. (as llama_token_type where needed should work fine)

@MarcusDunn MarcusDunn merged commit 5e6a677 into utilityai:main Mar 5, 2024
5 checks passed
@MarcusDunn
Copy link
Contributor

thanks for the quick turnaround. Just cut a new release.

@MarcusDunn MarcusDunn mentioned this pull request Mar 5, 2024
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.

2 participants