Skip to content

Conversation

@ai-jz
Copy link
Contributor

@ai-jz ai-jz commented Oct 31, 2025

Summary

This PR enables benchmark_serving_multi_turn.py to accept HuggingFace model names directly (e.g., meta-llama/Llama-2-7b-hf), in addition to local file paths.

Problem

Previously, the script would fail when given a HuggingFace model name:

python benchmark_serving_multi_turn.py --model meta-llama/Llama-2-7b-hf ...
# Error: OSError: Path does not exist: meta-llama/Llama-2-7b-hf

Solution

Remove the path validation check and let AutoTokenizer.from_pretrained() handle both local paths and HuggingFace model names. This is safe because AutoTokenizer already provides clear, informative error messages for invalid inputs.

Changes

- if not os.path.exists(args.model):
-     raise OSError(f"Path does not exist: {args.model}")
  logger.info("Loading tokenizer")
  tokenizer = AutoTokenizer.from_pretrained(args.model)

AutoTokenizer Error Messages

Testing shows that AutoTokenizer provides excellent error messages for all invalid inputs:

Non-existent absolute path:

Error: HFValidationError: Repo id must be in the form 'repo_name' or 'namespace/repo_name': '/nonexistent/model/path'

Non-existent relative path:

Error: HFValidationError: Repo id must use alphanumeric chars, '-', '_' or '.': './nonexistent_model'

Non-existent simple name:

Error: OSError: nonexistent_model is not a local folder and is not a valid model identifier listed on 'https://huggingface.co/models'

Invalid HuggingFace model name:

Error: OSError: invalid/model-name-12345 is not a local folder and is not a valid model identifier listed on 'https://huggingface.co/models'

These error messages clearly distinguish between local paths and HuggingFace model names, making additional validation unnecessary.

Testing

This change can be tested with:

# With HuggingFace model name (now works)
python benchmarks/multi_turn/benchmark_serving_multi_turn.py \
  --model meta-llama/Llama-2-7b-hf \
  --input-file input.json \
  --url http://localhost:8000

# With local path (still works as before)
python benchmarks/multi_turn/benchmark_serving_multi_turn.py \
  --model /path/to/local/model \
  --input-file input.json \
  --url http://localhost:8000

@mergify mergify bot added the performance Performance-related issues label Oct 31, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the benchmark_serving_multi_turn.py script by enabling it to accept HuggingFace model names in addition to local file paths. This is achieved by removing an explicit os.path.exists check and instead relying on the AutoTokenizer.from_pretrained function to handle both cases. The change is well-justified, as AutoTokenizer provides clear error messages for invalid model identifiers or paths. This is a solid improvement that increases the flexibility and user-friendliness of the benchmark script without introducing any apparent risks.

@ai-jz
Copy link
Contributor Author

ai-jz commented Oct 31, 2025

@DarkLight1337 Could you have a look? It's a very simple change.

@github-actions
Copy link

👋 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 ask your reviewers to trigger select CI tests on top of fastcheck CI.

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.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

🚀

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) October 31, 2025 02:34
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 31, 2025
@DarkLight1337 DarkLight1337 merged commit 2c0c7c3 into vllm-project:main Nov 1, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance-related issues ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants