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

[FEATURE] Add Xinference implementation #1179 #25

Merged
merged 21 commits into from
Dec 5, 2024

Conversation

alvinlee518
Copy link
Contributor

Issue

Contributes to langchain4j/langchain4j#1179

Change

Support for Xorbits Inference.

  • Added a XinferenceChatModel
  • Added a XinferenceEmbeddingModel
  • Added a XinferenceImageModel
  • Added a XinferenceLanguageModel
  • Added a XinferenceScoringModel
  • Added a XinferenceStreamingChatModel
  • Added a XinferenceStreamingLanguageModel

General checklist

  • There are no breaking changes
  • I have added unit and integration tests for my change
  • I have manually run all the unit tests in all modules, and they are all green
  • I have manually run all integration tests in the module I have added/changed, and they are all green

Checklist for adding new maven module

  • I have added my new module in the root pom.xml and langchain4j-community-bom/pom.xml

@Martin7-1 Martin7-1 added enhancement New feature or request P3 Medium priority theme: model Issues/PRs related to model labels Nov 27, 2024
@Martin7-1
Copy link
Collaborator

@alvinlee518 Hi! Thank you for your contribution. Will review it asap.

BTW, it looks like XInference support docker container. Would it possible to use testcontainers in IT?

@Martin7-1
Copy link
Collaborator

Martin7-1 commented Nov 28, 2024

BTW, could you make all static method call by using import static? (e.g. InternalXinferenceHelper.toTools -> toTools). Just for consistency with other modules.

And, could you please add tests about ChatModelListener like OllamaChatModelListenerIT?

@alvinlee518
Copy link
Contributor Author

BTW, could you make all static method call by using import static? (e.g. InternalXinferenceHelper.toTools -> toTools). Just for consistency with other modules.

And, could you please add tests about ChatModelListener like OllamaChatModelListenerIT?

Okay, got it.

@alvinlee518
Copy link
Contributor Author

@alvinlee518 Hi! Thank you for your contribution. Will review it asap.

BTW, it looks like XInference support docker container. Would it possible to use testcontainers in IT?

It's a bit tricky for me to test the docker container locally, but I can try adding it.

@alvinlee518
Copy link
Contributor Author

@Martin7-1 The Xinferenrece testcontainers require GPU support. How can I configure it in GitHub Actions to ensure the unit tests pass?

@Martin7-1
Copy link
Collaborator

Since we're using the free tier, Github Actions do not support GPU... Are there any ways that Xinference can run on CPU (such as latest-cpu version). BTW, could we use smaller models (I don't know how much time you test with GPU locally, but if we change it to CPU, it may cost much time, so we need to make the model as small as possible) to reduce IT cost?

If the test takes too long to run with CPU, maybe I'll consider disabling Xinference's IT in Github Actions and just running it locally.

Xinference's ITs are like Ollama's ITs in that they both take a long time to execute tests, and I'm also thinking about and trying to figure out how to optimise them and get them working perfectly in Github Actions. You can try the ideas I suggested above, but if it doesn't work that's fine, I'd run those test cases locally. I think I will review it next week :)

@alvinlee518
Copy link
Contributor Author

@Martin7-1 I have modified the image to the CPU version. When executed in GitHub Actions, the entire process takes approximately 10 minutes.
ImageModel, VisionModel, and tool call streaming functions rely on GPUs and can be validated locally.

@Martin7-1
Copy link
Collaborator

@Martin7-1 I have modified the image to the CPU version. When executed in GitHub Actions, the entire process takes approximately 10 minutes. ImageModel, VisionModel, and tool call streaming functions rely on GPUs and can be validated locally.

Thank you! Will try to review it this week.

Copy link
Collaborator

@Martin7-1 Martin7-1 left a comment

Choose a reason for hiding this comment

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

@alvinlee518 Thank you!

@alvinlee518 alvinlee518 requested a review from Martin7-1 December 4, 2024 05:58
Copy link
Collaborator

@Martin7-1 Martin7-1 left a comment

Choose a reason for hiding this comment

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

@alvinlee518 Thank you! About IT: maybe we should give up comitting new image, because so much images will exceed Github Actions disk space limit. WDYT?

Copy link
Collaborator

@Martin7-1 Martin7-1 left a comment

Choose a reason for hiding this comment

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

Thank you! Would you mind removing all final in method level if it's not really needed?

@alvinlee518
Copy link
Contributor Author

@alvinlee518 Thank you! About IT: maybe we should give up comitting new image, because so much images will exceed Github Actions disk space limit. WDYT?
@Martin7-1
The new image is intended to avoid re-downloading the model. If you're concerned about the disk space limit, I can remove it.

lixw and others added 2 commits December 5, 2024 10:04
2.remove final on local variable and parameter
Copy link
Collaborator

@Martin7-1 Martin7-1 left a comment

Choose a reason for hiding this comment

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

@alvinlee518 Thank you! LGTM. Could you please add docs in langchain4j main repo?

@alvinlee518
Copy link
Contributor Author

@alvinlee518 Thank you! LGTM. Could you please add docs in langchain4j main repo?

Okay, I'll add it later.

@Martin7-1 Martin7-1 merged commit 481a1e2 into langchain4j:main Dec 5, 2024
4 checks passed
langchain4j pushed a commit to langchain4j/langchain4j that referenced this pull request Jan 8, 2025
> ## Issue
> Closes [langchain4j-community
#25](langchain4j/langchain4j-community#25)
> 
> ## Change
> Add `langchain4j-community-xinference` document.
> 
> ## General checklist
> * [x]  There are no breaking changes
> * [ ]  I have added unit and integration tests for my change
> * [ ] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
> * [ ] I have manually run all the unit and integration tests in the
[core](https://github.com/langchain4j/langchain4j/tree/main/langchain4j-core)
and
[main](https://github.com/langchain4j/langchain4j/tree/main/langchain4j)
modules, and they are all green
> 
> * [x] I have added/updated the
[documentation](https://github.com/langchain4j/langchain4j/tree/main/docs/docs)
> * [ ] I have added an example in the [examples
repo](https://github.com/langchain4j/langchain4j-examples) (only for
"big" features)
> * [ ] I have added/updated [Spring Boot
starter(s)](https://github.com/langchain4j/langchain4j-spring) (if
applicable)

Co-authored-by: lixw <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P3 Medium priority theme: model Issues/PRs related to model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants