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

Add vLLM+HPA support to ChatQnA Helm chart #610

Merged
merged 6 commits into from
Dec 18, 2024

Conversation

eero-t
Copy link
Contributor

@eero-t eero-t commented Nov 25, 2024

Description

Add vLLM + HPA support for ChatQnA Helm charts.

Similarly to how it's already done in Agent component, there's tgi.enabled and vllm.enabled flags for selecting which LLM will be used.

Note: ChatQnA does not yet support using vLLM for embedding & reranking [1], so HF TEI is still used for those.

[1] opea-project/GenAIExamples#1237

Issues

Fixes #608 partially.
Fixes #631.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)

New dependencies

n/a

Tests

Manual testing on top of main HEAD / v1.1 images.

@eero-t eero-t marked this pull request as draft November 25, 2024 18:04
@eero-t
Copy link
Contributor Author

eero-t commented Nov 25, 2024

Setting as draft. I have tested that DocSum with Gaudi vLLM works, and that ChatQnA Helm chart can be installed, but due to v1.1 image pulls currently taking so long in my test node, I haven't been able test ChatQnA with Gaudi vLLM properly yet.

vLLM CPU version testing would also be needed before merging this (I'm hoping somebody else here could check at least DocSum with CPU vLLM).

@eero-t
Copy link
Contributor Author

eero-t commented Nov 26, 2024

CI issues:

  • LLM-uservice: openai.NotFoundError: Error code: 404 - {'object': 'error', 'message': 'The model meta-llama/Meta-Llama-3-8B-Instruct does not exist.', 'type': 'NotFoundError', 'param': None, 'code': 404}
    • Bug already in the CI/repo, pre-existing Helm chart refers to model not present in CI => I can fix it
  • DocSum: 100.83.111.229:5000/opea/llm-docsum-vllm:latest: not found
    • Bug in OPEA image creation, that image is missing from DockerHub & CI => somebody else needs to fix that

@eero-t
Copy link
Contributor Author

eero-t commented Nov 26, 2024

This overlaps partly with #403.

@eero-t
Copy link
Contributor Author

eero-t commented Nov 29, 2024

Added HPA support for ChatQnA / vLLM.

@eero-t
Copy link
Contributor Author

eero-t commented Dec 2, 2024

This bug for llm-uservice, gaudi, ci-vllm-gaudi-values, common is weird:

openai.NotFoundError: Error code: 404 - {'object': 'error', 'message': 'The model `meta-llama/Meta-Llama-3-8B-Instruct` does not exist.', 'type': 'NotFoundError', 'param': None, 'code': 404}

Because neither that CI file, nor values.yaml specifies that model for llm-uservice:

$ git  grep -l '^[^#]*Meta-Llama-3-8B-Instruct'
helm-charts/common/llm-uservice/ci-faqgen-values.yaml
helm-charts/common/llm-uservice/variant_faqgen-values.yaml
helm-charts/faqgen/README.md
helm-charts/faqgen/values.yaml

So CI either invents that model from thin air, or is using wrong CI file (faqgen instead of vllm-gaudi one)!

@eero-t
Copy link
Contributor Author

eero-t commented Dec 10, 2024

Changes:

@eero-t eero-t changed the title WIP: Add vLLM support to ChatQnA + DocSum Helm charts Add vLLM+HPA support to ChatQnA Helm chart Dec 10, 2024
@eero-t
Copy link
Contributor Author

eero-t commented Dec 11, 2024

Rebased to main, and dropped fix for model ID [1].

[1] it's specific to DocSum & FaqGen specific llm-uservice variants. Text generation llm-uservice variant uses different env var name from those (which IMHO is bug, but for code in another repo). I'll add workaround for it in separate PR for DocSum vLLM suport.

eero-t and others added 6 commits December 17, 2024 18:38
For now vLLM replaces just TGI, but as it supports also embedding,
also TEI-embed/-rerank may be replaceable later on.

Signed-off-by: Eero Tamminen <[email protected]>
- Remove --eager-enforce on hpu to improve performance
- Refactor to the upstream docker entrypoint changes

Fixes issue opea-project#631.

Signed-off-by: Lianhao Lu <[email protected]>
@eero-t eero-t marked this pull request as ready for review December 17, 2024 16:39
@eero-t eero-t mentioned this pull request Dec 17, 2024
1 task
@yongfengdu yongfengdu merged commit baed0b5 into opea-project:main Dec 18, 2024
31 of 47 checks passed
@eero-t eero-t deleted the helm-vllm branch December 18, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants