run-llama / llama_index

LlamaIndex is a data framework for your LLM applications
https://docs.llamaindex.ai
MIT License
35.86k stars 5.09k forks source link

[Feature Request]: Batch asynchronous text embedding for LangchainEmbedding #12709

Closed NiklasLue closed 2 months ago

NiklasLue commented 5 months ago

Feature Description

When using the LangchainEmbedding class to import langchain embeddings, the asynchronous calls are not batched, but just defaulted to a chunk size of 1, as the _aget_query_embedding function is not implemented, as it is for example the case in the OpenAIEmbedding class in LlamaIndex.

Reason

I only tested an implementation using the LangChain AzureOpenAIEmbeddings class. In that case the following code worked well, similar to the _aget_text_embedding implementation.

async def _aget_text_embeddings(self, texts: List[str]) -> List[List[float]]:
    try:
        embeds = await self._langchain_embedding.aembed_documents(texts)
        return embeds
    except NotImplementedError:
        # Warn the user that sync is being used
        self._async_not_implemented_warn_once()
        return self._get_text_embeddings(texts)

Value of Feature

Without batching the asynchronous embedding functions from LangChain embedders are not useable for large collections of texts, hence this bugfix would be important for their integration.

logan-markewich commented 5 months ago

Fair point, I remember there being some reason this wasn't implemented but I can look again.

Just curious though, what embeddings are you using that we don't have? We support azure embeddings, along with a ton of others

NiklasLue commented 5 months ago

Thanks!

There was some issue with using Azure Entra ID for authentication. I'm not 100% sure anymore, but could it be that your implementation does not support using a bearer token provider (yet)? I tried to extend the current Embedding class but I didn't get very far.

This was the quickest solution for now, but I'd be happy to try the Llama-Index class again in the future.