-
Notifications
You must be signed in to change notification settings - Fork 17
bump minimum vLLM requirement to v0.10.0, drop prompt adapters #269
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
Conversation
WalkthroughThis update removes all support for prompt tuning adapters from the codebase. It eliminates related code, tests, configuration files, and the CLI tool for converting PyTorch prompt tuning models. The dependency on "vllm" is updated, and the script entry point for prompt adapter conversion is removed from the project configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (removed)
participant AdapterValidator
participant AdapterStore
participant ModelHandler
User->>CLI (removed): Run convert_pt_to_prompt (No longer available)
Note over CLI (removed): CLI tool for prompt adapter conversion is removed
User->>AdapterValidator: Request with adapter type "PROMPT_TUNING"
AdapterValidator->>AdapterValidator: Immediately raise unsupported adapter error
Note over AdapterValidator: No prompt adapter validation or handling remains
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
||
|
||
@pytest.mark.asyncio | ||
async def test_cache_handles_concurrent_loads(vllm_model_handler): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test would probably be good to keep, the second half of it is all using lora adapters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(But also not a huge deal if we're deleting this repo soon 😉 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally lgtm, ship it when tests pass 🚀
@joerunde should this section be removed as well https://github.com/opendatahub-io/vllm-tgis-adapter/pull/269/files#diff-044c034e586927eab0938dfbd552ed6e6cdc3a05168bae2e69fae45bd3ac54d7L191-L200 ? Since the src/vllm_tgis_adapter/tgis_utils/convert_pt_to_prompt.py is deleted. |
So, I investigated this a bit. It looks like CPU vllm is broken as of v0.10.0 (maybe from earlier versions as well since I've seen our CI fail for quite a while before this). I tested this branch on a GPU host and both endpoints work (http and grpc) when using I used this script https://github.com/opendatahub-io/vllm-tgis-adapter/tree/main/examples (inference.py) to hit the grpc endpoint. We don't have the capacity to get CPU vllm work again, we'll just merge this as is for now. |
PromptAdapterRequest
and everything relatedfixes #268
Summary by CodeRabbit
Chores
Bug Fixes
Tests
Refactor