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 vllm openai api endpoint #49

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

joennlae
Copy link
Contributor

@joennlae joennlae commented Mar 18, 2024

First of all: Cool project 💯

This PR adds vllm based endpoints.

Just use:

 VLLM_BASE_URL="http://localhost:8000/v1/" EVAL_LLM="vllm_Qwen/Qwen1.5-14B-Chat-GPTQ-Int8" python ...

And your vllm deployed model should work :-)


Ellipsis 🚀 This PR description was created by Ellipsis for commit 284c0e0.

Summary:

This PR adds support for vllm models in the LLMFactory class and modifies the _llm_response method to handle cases where the api_key is 'unused'.

Key points:

  • Added support for vllm models in LLMFactory class in continuous_eval/llm_factory.py.
  • The vllm model is initialized with a base URL from the VLLM_BASE_URL environment variable.
  • Modified _llm_response method to handle cases where api_key is 'unused'.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me!

  • Reviewed the entire pull request up to 284c0e0
  • Looked at 30 lines of code in 1 files
  • Took 1 minute and 17 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 2 additional comments because they didn't meet confidence threshold of 50%.
1. continuous_eval/llm_factory.py:106:
  • Assessed confidence : 80%
  • Grade: 0%
  • Comment:
    The OpenAI client is instantiated with a hardcoded 'unused' API key. This might cause issues if the OpenAI client requires a valid API key to function, even if the base_url is different. Please confirm if this is the intended behavior.
  • Reasoning:
    The PR adds support for a new type of model, 'vllm_', and it seems to be implemented correctly. However, there's a potential issue with the way the OpenAI client is instantiated for this new model type. The api_key is hardcoded as 'unused', which might cause issues if the OpenAI client actually requires a valid API key to function, even if the base_url is different. I need to check the OpenAI client's documentation or source code to confirm this.
2. continuous_eval/llm_factory.py:120:
  • Assessed confidence : 80%
  • Grade: 0%
  • Comment:
    The condition to check if the API key is not 'unused' might cause issues if other model types also use the 'unused' API key. Please confirm if this is the intended behavior.
  • Reasoning:
    The PR adds a condition to check if the API key is not 'unused' before making a request in JSON mode. This seems to be a workaround for the hardcoded 'unused' API key for the 'vllm_' model type. However, this might cause issues if other model types also use the 'unused' API key. I need to check if other parts of the codebase also use the 'unused' API key.

Workflow ID: wflow_4NvnotezU58pN1QT


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

@pantonante
Copy link
Contributor

Thank you for contributing, this looks great to me!

@pantonante pantonante merged commit 6fca8ab into relari-ai:main Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants