You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "MrJs133 (via GitHub)" <gi...@apache.org> on 2025/05/22 16:56:17 UTC
[PR] fix(llm): ollama batch embedding bug [incubator-hugegraph-ai]
MrJs133 opened a new pull request, #250:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/250
when using ollama embedding models, `update_vid_embedding` bug
**gradio.exceptions.Error: "'Client' object has no attribute 'embed'"**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(llm): ollama batch embedding bug [incubator-hugegraph-ai]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #250:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/250#discussion_r2104254237
##########
hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py:
##########
@@ -19,57 +19,41 @@
from typing import List
import ollama
+
from .base import BaseEmbedding
class OllamaEmbedding(BaseEmbedding):
- def __init__(
- self,
- model: str,
- host: str = "127.0.0.1",
- port: int = 11434,
- **kwargs
- ):
+ def __init__(self, model: str, host: str = "127.0.0.1", port: int = 11434, **kwargs):
self.model = model
self.client = ollama.Client(host=f"http://{host}:{port}", **kwargs)
self.async_client = ollama.AsyncClient(host=f"http://{host}:{port}", **kwargs)
self.embedding_dimension = None
- def get_text_embedding(
- self,
- text: str
- ) -> List[float]:
- """Comment"""
- return list(self.client.embed(model=self.model, input=text)["embeddings"][0])
+ def get_text_embedding(self, text: str) -> List[float]:
+ """Get embedding for a single text."""
+ return self.get_texts_embeddings([text])[0]
- def get_texts_embeddings(
- self,
- texts: List[str]
- ) -> List[List[float]]:
+ def get_texts_embeddings(self, texts: List[str]) -> List[List[float]]:
"""Get embeddings for multiple texts in a single batch.
-
- This method efficiently processes multiple texts at once by leveraging
- Ollama's batching capabilities, which is more efficient than processing
- texts individually.
-
- Parameters
- ----------
- texts : List[str]
- A list of text strings to be embedded.
-
+
Returns
-------
List[List[float]]
A list of embedding vectors, where each vector is a list of floats.
The order of embeddings matches the order of input texts.
"""
- response = self.client.embed(model=self.model, input=texts)["embeddings"]
- return [list(inner_sequence) for inner_sequence in response]
-
- async def async_get_text_embedding(
- self,
- text: str
- ) -> List[float]:
- """Comment"""
+ if hasattr(self.client, "embed"):
Review Comment:
If `client.embed` is missing, the code raises an `AttributeError` instead of using a synchronous fallback (e.g., `client.embeddings` via `run_in_executor`). Implement a real sync fallback or document that batch embedding is unsupported by older client versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(llm): ollama batch embedding bug [incubator-hugegraph-ai]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #250:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/250#discussion_r2103998263
##########
hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py:
##########
@@ -63,8 +96,20 @@ def get_texts_embeddings(
A list of embedding vectors, where each vector is a list of floats.
The order of embeddings matches the order of input texts.
"""
- response = self.client.embed(model=self.model, input=texts)["embeddings"]
- return [list(inner_sequence) for inner_sequence in response]
+ if hasattr(self.client, "embed"): # pylint: disable=no-else-return
+ response = self.client.embed(model=self.model, input=texts)["embeddings"]
+ return [list(inner_sequence) for inner_sequence in response]
+ elif hasattr(self.client, "embeddings"):
+ embeddings_list = []
+ for text_item in texts:
+ response_item = self.client.embeddings(model=self.model, prompt=text_item)
+ embeddings_list.append(list(response_item["embedding"]))
+ return embeddings_list
Review Comment:
[nitpick] When using the 'embeddings' method, the code makes individual API calls for each text. Consider checking if a batch API is available or batching these calls to reduce potential network overhead.
```suggestion
response = self.client.embeddings(model=self.model, prompt=texts)
try:
return [list(embedding) for embedding in response["embeddings"]]
except KeyError as e:
raise RuntimeError(
"Failed to extract embeddings from Ollama client 'embeddings' response. "
f"Response: {response}. Error: {e}"
) from e
```
##########
hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py:
##########
@@ -39,8 +39,41 @@ def get_text_embedding(
self,
text: str
) -> List[float]:
- """Comment"""
- return list(self.client.embed(model=self.model, input=text)["embeddings"][0])
+ """Get embedding for a single text.
+
+ This method handles different Ollama client API versions by checking for
+ the presence of 'embed' or 'embeddings' methods.
+ """
+ if hasattr(self.client, "embed"):
+ response = self.client.embed(model=self.model, input=text)
+ try:
+ # First, try the structure typically seen for single embeddings
+ # or newer batch responses that might return a single "embedding" key.
+ return list(response["embedding"])
+ except KeyError:
+ # Fallback for older batch-like response for single item,
+ # or if "embeddings" is a list with one item.
+ try:
+ return list(response["embeddings"][0])
+ except (KeyError, IndexError) as e:
+ raise RuntimeError(
+ "Failed to extract embedding from Ollama client 'embed' response. "
+ f"Response: {response}. Error: {e}"
+ ) from e
+ elif hasattr(self.client, "embeddings"):
+ response = self.client.embeddings(model=self.model, prompt=text)
+ try:
+ return list(response["embedding"])
+ except KeyError as e:
+ raise RuntimeError(
+ "Failed to extract embedding from Ollama client 'embeddings' response. "
+ f"Response: {response}. Error: {e}"
+ ) from e
Review Comment:
[nitpick] The nested try/except blocks could be refactored by first checking the existence of keys in the response, which would improve clarity and reduce reliance on exception handling.
```suggestion
# Check for the presence of "embedding" key in the response
if "embedding" in response:
return list(response["embedding"])
# Fallback for older batch-like response with "embeddings" key
elif "embeddings" in response and isinstance(response["embeddings"], list) and len(response["embeddings"]) > 0:
return list(response["embeddings"][0])
else:
raise RuntimeError(
"Failed to extract embedding from Ollama client 'embed' response. "
f"Response: {response}. Ensure the response structure is correct."
)
elif hasattr(self.client, "embeddings"):
response = self.client.embeddings(model=self.model, prompt=text)
# Check for the presence of "embedding" key in the response
if "embedding" in response:
return list(response["embedding"])
else:
raise RuntimeError(
"Failed to extract embedding from Ollama client 'embeddings' response. "
f"Response: {response}. Ensure the response structure is correct."
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(llm): ollama batch embedding bug [incubator-hugegraph-ai]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #250:
URL: https://github.com/apache/incubator-hugegraph-ai/pull/250
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org