[aarnphm]

This PR adds support to tokenizer_mode in sampling example requests for supporting running this benchmark with Mistral models.

tokenizer_mode=auto doesn't seem to detect mistral tokenizer correctly (which is possibly a bug. However, the Mistral team suggest to set tokenizer_mode explicitly regardless.)

It also refactor --no-structured-outputs to use argparser.OptionalBooleanOptions, which set --no-structured-outputs/--structured-outputs (behaviour unchanged)

Signed-off-by: Aaron Pham contact@aarnphm.xyz

[github-actions]

👋 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.

🚀

[russellb]

can you revert the formatting changes so this just includes the functional parts?

[russellb]

This doesn't work for me

❯ python benchmarks/benchmark_serving_structured_output.py --help
INFO 03-18 15:19:00 [__init__.py:256] Automatically detected platform cuda.
Traceback (most recent call last):
  File "/home/rbryant/vllm/benchmarks/benchmark_serving_structured_output.py", line 843, in <module>
    parser.add_argument("--host", type=str, default="127.0.0.1")
  File "/usr/lib/python3.10/argparse.py", line 1460, in add_argument
    return self._add_action(action)
  File "/usr/lib/python3.10/argparse.py", line 1826, in _add_action
    self._optionals._add_action(action)
  File "/usr/lib/python3.10/argparse.py", line 1662, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "/usr/lib/python3.10/argparse.py", line 1474, in _add_action
    self._check_conflict(action)
  File "/usr/lib/python3.10/argparse.py", line 1611, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "/usr/lib/python3.10/argparse.py", line 1620, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument --host: conflicting option string: --host

[cjackal]

Doesn't this approach (of importing vllm in benchmark script) contradict with #14923?

[aarnphm]

This doesn't work for me

Oh my bad, should be fixed now.

[aarnphm]

Doesn't this approach (of importing vllm in benchmark script) contradict with #14923?

the structured outputs script uses internal utils for some unsupported features atm, so we need to import vllm regardless

[russellb]

to reduce some duplicates parameters

can you be a bit more explicit in the description about what exactly this does? What problem did you observe that led to the change? Was it just duplication? What were the options before this change? and how about after?

[aarnphm]

I updated the description.

[russellb]

However, this will include some additional noop parameters (for example all of the spec_decode args).

I think this is a deal breaker for me. We need to disable/hide stuff that isn't a real option. Maybe sharing this code isn't a good fit as it's written?

[aarnphm]

yeah, I think we can just add support for --tokenizer-mode here instead.

[russellb]

thanks!