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

Jump to bottom

community: make LocalAIEmbeddings comaptible with openai>1.0 #17154

Closed
wants to merge 11 commits into from
Closed

community: make LocalAIEmbeddings comaptible with openai>1.0 #17154

wants to merge 11 commits into from

Conversation

liunux4odoo
Copy link
Contributor

I run the bge-large embedding model by xinference locally, but the latest LocalAIEmbeddings can only work with openai<1.0, not comaptible with the latestopenai`.

I upgrade the LocalAIEmbeddings referencing to ChatOpenAI to make it working with openai>1.0.

@vercel Vercel
Copy link

vercel bot commented Feb 7, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Apr 19, 2024 2:36am

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. Ɑ: embeddings Related to text embedding models module 🤖:improvement Medium size change to existing code to handle new use-cases 🔌: openai Primarily related to OpenAI integrations labels Feb 7, 2024
@shell-nlp
Copy link

I run the bge-large embedding model by xinference locally, but the latest LocalAIEmbeddings can only work with openai<1.0, not comaptible with the latestopenai`.

I upgrade the LocalAIEmbeddings referencing to ChatOpenAI to make it working with openai>1.0.

langchain_community/embeddings/localai.py:1:1: I001 Import block is un-sorted or un-formatted

@eahova
Copy link
Contributor

eahova commented Feb 24, 2024

these changes work great for me using localai embeddings with news openai - thanks!

submitted a pr to your fork for the lint errors

@bdqfork
Copy link

bdqfork commented Feb 29, 2024

hope we can merge as soon as possible. I have encountered the same problem.

@eahova eahova mentioned this pull request Mar 8, 2024
Closed
@eahova
Copy link
Contributor

eahova commented Mar 8, 2024

I moved this to #18799 and got it to pass the CI tests

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Mar 19, 2024
@liunux4odoo
Copy link
Contributor Author

@eahova I have merge your pr for lint fixing.

@imClumsyPanda
Copy link

This PR helps a lot! Plz review asap @baskaryan @eyurtsev @hwchase17

baskaryan
baskaryan reviewed Mar 28, 2024
View reviewed changes
libs/community/langchain_community/embeddings/localai.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we make this compatible with both openai v0 and v1, the way langchain_community.embedding.OpenAIEmbeddings is? or if that's too hard we should at least add an explicit version check of the openai sdk and raise an informative error if it's less than 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

surely it has been done.

baskaryan
baskaryan reviewed Mar 28, 2024
View reviewed changes
libs/community/langchain_community/embeddings/localai.py
Comment on lines +55 to +59
retry_if_exception_type(openai.Timeout)
| retry_if_exception_type(openai.APIError)
| retry_if_exception_type(openai.APIConnectionError)
| retry_if_exception_type(openai.RateLimitError)
| retry_if_exception_type(openai.InternalServerError)
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't think this is v0 compatible

baskaryan
baskaryan reviewed Mar 28, 2024
View reviewed changes
libs/community/langchain_community/embeddings/localai.py
Comment on lines +295 to +302
embed_with_retry(
self,
input=[text],
**self._invocation_params,
)
.data[0]
.embedding
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't think this is v0 compatible

Copy link
Collaborator

Choose a reason for hiding this comment

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

bump

Copy link
Collaborator

Choose a reason for hiding this comment

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

same below

@baskaryan baskaryan self-assigned this Mar 29, 2024
@ChristianSch
Copy link

ChristianSch commented Apr 18, 2024 •
edited
Loading

Hey @baskaryan @liunux4odoo any updates here? Anything you need help with? I'm interested in getting this released asap

@ccurme ccurme added the community Related to langchain-community label Jun 18, 2024
@liunux4odoo
Copy link
Contributor Author

I would like to close this PR because latest langchain-openai works well with locally served embeddings by xinference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baskaryan baskaryan baskaryan left review comments

Assignees

@baskaryan baskaryan

Labels
community Related to langchain-community Ɑ: embeddings Related to text embedding models module 🤖:improvement Medium size change to existing code to handle new use-cases 🔌: openai Primarily related to OpenAI integrations size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@liunux4odoo @shell-nlp @eahova @bdqfork @imClumsyPanda @ChristianSch @baskaryan @ccurme