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.

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

Community: LlamaCppEmbeddings embed_documents and embed_query #28827

Merged
merged 12 commits into from
Dec 23, 2024
Merged

Community: LlamaCppEmbeddings embed_documents and embed_query #28827

Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b854425
[fix]: embedding fixed
keenborder786 Dec 19, 2024
7a51ab1
[chore]
keenborder786 Dec 19, 2024
7de9d18
Merge branch 'master' into llama_embedding_corrected
keenborder786 Dec 19, 2024
22379a5
[tests]: Added Tests for LLamaCpp
keenborder786 Dec 20, 2024
d301422
[chore]: Checking if client has already been passed
keenborder786 Dec 20, 2024
1572f3b
[chore]: Annotation Added for Tests LLamaCpp
keenborder786 Dec 20, 2024
fd3771e
[chore]: Comments Added to ignore call-args
keenborder786 Dec 20, 2024
cb16fbc
[chore]
keenborder786 Dec 20, 2024
88c02b5
Merge branch 'master' into llama_embedding_corrected
keenborder786 Dec 20, 2024
6b97c48
[chore]: Raise ValueError if embedding data is empty
keenborder786 Dec 21, 2024
fe30a0d
Merge branch 'master' into llama_embedding_corrected
keenborder786 Dec 21, 2024
cca83b9
update
ccurme Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions libs/community/langchain_community/embeddings/llamacpp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,14 @@ def embed_documents(self, texts: List[str]) -> List[List[float]]:
List of embeddings, one for each text.
"""
embeddings = self.client.create_embedding(texts)
return [list(map(float, e["embedding"])) for e in embeddings["data"]]
if not isinstance(embeddings["data"][0]["embedding"][0], list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible for this line to raise IndexError where it previously would not? e.g., if texts is empty or contains empty strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the confusion. I didn't mean to request that we raise an error where it previously would not.

From an ignorant reading of the code (i.e., no knowledge of the behavior of the client) the update introduced the potential to raise IndexError where it previously wouldn't. I've pushed a change that passes your tests. Let me know if you see any issues with it.

return [list(map(float, e["embedding"])) for e in embeddings["data"]]
else:
final_embeddings = []
for e in embeddings["data"]:
for data in e["embedding"]:
final_embeddings.append(list(map(float, data)))
return final_embeddings

def embed_query(self, text: str) -> List[float]:
"""Embed a query using the Llama model.
Expand All @@ -128,4 +135,7 @@ def embed_query(self, text: str) -> List[float]:
Embeddings for the text.
"""
embedding = self.client.embed(text)
return list(map(float, embedding))
if not isinstance(embedding, list):
return list(map(float, embedding))
else:
return list(map(float, embedding[0]))
Loading