-
-
Notifications
You must be signed in to change notification settings - Fork 4k
feat(vllm): add grammar and structured output support #8806
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
base: master
Are you sure you want to change the base?
Changes from all commits
0a26cf6
bbb32ac
3617e2a
7a4d0a6
b19d7f2
480bcd8
c703a03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| import asyncio | ||
| from concurrent import futures | ||
| import argparse | ||
| import json | ||
| import signal | ||
| import sys | ||
| import os | ||
|
|
@@ -22,6 +23,21 @@ | |
| from vllm.engine.arg_utils import AsyncEngineArgs | ||
| from vllm.engine.async_llm_engine import AsyncLLMEngine | ||
| from vllm.sampling_params import SamplingParams | ||
|
|
||
| # vLLM renamed GuidedDecodingParams to StructuredOutputsParams in newer versions. | ||
| # The corresponding SamplingParams field also changed from guided_decoding to structured_outputs. | ||
| try: | ||
| from vllm.sampling_params import StructuredOutputsParams | ||
| _structured_output_cls = StructuredOutputsParams | ||
| _structured_output_field = "structured_outputs" | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto here - I'm not sure what are we doing here, just importing , adding to a var.. and not used? |
||
| except ImportError: | ||
| try: | ||
| from vllm.sampling_params import GuidedDecodingParams | ||
| _structured_output_cls = GuidedDecodingParams | ||
| _structured_output_field = "guided_decoding" | ||
| except ImportError: | ||
| _structured_output_cls = None | ||
| _structured_output_field = None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a fallback? We usually pin the upstream version.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I checked and vLLM is actually not pinned to a specific version — That said, if the project plans to pin vLLM to a specific version, I'm happy to drop the fallback and target whichever API is current. Let me know which you'd prefer.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, when you say newer versions, how new? If it's a very recent change then maybe we need this, otherwise we probably don't
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rename happened in vLLM v0.8.x → latest. Since vLLM isn't pinned ( Also in the latest push: I've refactored to use the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just remove the fallback and if it passes CI we don't need to worry |
||
| from vllm.utils import random_uuid | ||
| from vllm.transformers_utils.tokenizer import get_tokenizer | ||
| from vllm.multimodal.utils import fetch_image | ||
|
|
||
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.
where this is consumed?