Simplify (and fix) passing of guided decoding backend options #17008

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.

Sign up for GitHub

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

Jump to bottom

Merged

russellb merged 26 commits into vllm-project:main from hmellor:split-guided-decoding-backend-options

Apr 29, 2025

Member

hmellor commented Apr 22, 2025 •

edited

Loading

Fixes #17002


          Split guided_decoding_backend into guided_decoding_backend and `g…

360026f

…uided_decoding_backend_options`

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

github-actions bot commented Apr 22, 2025

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

mergify bot added documentation structured-output v1 labels

github-project-automation bot added this to Structured Output

hmellor mentioned this pull request

[Bug]: Guided Decoding Backend options with the OpenAI server recently broken #17002

Closed

1 task


          Typo

0390b6f

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

tjohnson31415 reviewed

View reviewed changes

vllm/engine/arg_utils.py Outdated Show resolved Hide resolved

hmellor added 2 commits

April 24, 2025 10:29


          Merge branch 'main' into split-guided-decoding-backend-options

dc9f57b

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Merge branch 'main' into split-guided-decoding-backend-options

49b89e5

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Member Author

hmellor commented Apr 28, 2025

This should merge after #17303 to leverage the improved conversion from dataclass configs to argparse arguments

hmellor added 4 commits

April 28, 2025 14:28


          Merge branch 'main' into split-guided-decoding-backend-options

8a1ed06

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Merge branch 'main' into split-guided-decoding-backend-options

7a660b0


          Fix typo

592ff3d

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Use bool flags instead of cramming many flags into a dict

2979c31

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

hmellor marked this pull request as ready for review

April 28, 2025 18:14

hmellor requested review from WoosukKwon, alexm-redhat, comaniac, mgoin, njhill, robertgshaw2-redhat, russellb, youkaichao, ywang96 and zhuohan123 as code owners

April 28, 2025 18:14

hmellor added 3 commits

April 28, 2025 20:28


          Fix word order in args

8a367ba

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Fix missing arg

b9d4074

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Enforce disable additional properties only works for guidance

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Fix incorrect attribute in xgrammar

a34a434

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Member Author

hmellor commented Apr 28, 2025 •

edited

Loading

I've fixed all the issues locally, but encounter an issue in the entrypoints test which requires a decision.

In my local branch (which fixes the errors you see in CI) I see the following error:

$ pytest tests/entrypoints/llm/test_guided_generate.py -vs -k test_guided_number_range_json_completion
...
ValueError: Request-level structured output backend selection is
no longer supported. The request specified 'outlines', but vLLM
was initialised with 'auto'. This error can be resolved by removing
backend selection from the request.

This error is supposed to throw when we do this so I'm not sure what to do about it.

hmellor added 2 commits

April 29, 2025 09:10


          Fix test parameters

ae2f1c3

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Merge Literals with Literal not Union

666ba39

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Member

russellb commented Apr 29, 2025

@hmellor that file only works with V0. CI runs it with VLLM_USE_V1=0. It would be better to edit the file to force it to do that on its own, but if it passes for you with V0, you're good to go.

Member Author

hmellor commented Apr 29, 2025

I've found an issue with unions of literals that I'm going to make a separate pr for. Then I'll update this test to only run for v0. Then all the tests should pass and we should be good to merge!

hmellor added 5 commits

April 29, 2025 15:11


          Enforce that Literals are merged with Literal not Union

6df5dbb

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Add tests for config decorator

08d5e20

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Create new helper function to handle sequences of literals

92300b6

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Add test for literal to kwarg

88c1479

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Add test cases for list[Literal] and Literal[Literal, Literal]

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Member Author

hmellor commented Apr 29, 2025

Here is the union of literals PR I mentioned #17391

hmellor added 4 commits

April 29, 2025 15:46


          Fix pre-commit

0ecf76e

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Respond to comment

8beb8df

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Merge branch 'main' into split-guided-decoding-backend-options

e44135c

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>


          Merge branch 'config-literal-handling' into split-guided-decoding-bac…

008b037

…kend-options

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Member Author

hmellor commented Apr 29, 2025

Looks like I can't use the monkeypatch fixture to instantiate the LLM with VLLM_USE_V1=0.

I'm going to leave it as is because I'd rather the use the llm fixture and run faster.


          Merge branch 'main' into split-guided-decoding-backend-options

39557d7

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

russellb merged commit a6977db into vllm-project:main

48 checks passed

github-project-automation bot moved this to Done in Structured Output

hmellor deleted the split-guided-decoding-backend-options branch

April 29, 2025 19:10

lk-chen pushed a commit to lk-chen/vllm that referenced this pull request


          Simplify (and fix) passing of guided decoding backend options (vllm-p…

8a6ace7

…roject#17008)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

lk-chen reviewed

View reviewed changes

vllm/model_executor/guided_decoding/xgrammar_decoding.py

    
                                     f"`guided_backend=xgrammar:disable-any-whitespace` "

                                     f"to prevent runaway generation of whitespaces.")

                              logger.info_once(msg)

                              logger.info_once(

Collaborator

lk-chen Apr 29, 2025

This is causing number of arguments mismatch for me https://github.com/vllm-project/vllm/blob/main/vllm/logger.py#L75

Member Author

hmellor Apr 29, 2025

Sorry about that, fix in #17416

radeksm pushed a commit to radeksm/vllm that referenced this pull request


          Simplify (and fix) passing of guided decoding backend options (vllm-p…

6fe6a42

…roject#17008)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request


          Simplify (and fix) passing of guided decoding backend options (vllm-p…

26aed9c

…roject#17008)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>

zzzyq pushed a commit to zzzyq/vllm that referenced this pull request


          Simplify (and fix) passing of guided decoding backend options (vllm-p…

3fa7e0c

…roject#17008)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Yuqi Zhang <yuqizhang@google.com>

DarkLight1337 mentioned this pull request

[Deprecation] Remove inputs arg fallback in Engine classes #18799

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

russellb russellb approved these changes

mgoin Awaiting requested review from mgoin mgoin is a code owner

WoosukKwon Awaiting requested review from WoosukKwon

robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

njhill Awaiting requested review from njhill

ywang96 Awaiting requested review from ywang96

comaniac Awaiting requested review from comaniac

alexm-redhat Awaiting requested review from alexm-redhat

zhuohan123 Awaiting requested review from zhuohan123

youkaichao Awaiting requested review from youkaichao

+2 more reviewers

lk-chen lk-chen left review comments

tjohnson31415 tjohnson31415 left review comments

Labels

documentation ready structured-output v1