Skip to content

Conversation

@LucasWilkinson
Copy link
Collaborator

@LucasWilkinson LucasWilkinson commented Jan 23, 2026

add a script that runs one batch and can optionally profile it; e.g.

python examples/offline_inference/run_one_batch.py --model deepseek-ai/DeepSeek-V2-Lite --tensor-parallel-size 1 --max-model-len 1024 --gpu-memory-utilization 0.9 --kv-cache-dtype fp8 --trust-remote-code --profile both --profile-dir ./profiles/main

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@mergify
Copy link

mergify bot commented Jan 23, 2026

Documentation preview: https://vllm--32968.org.readthedocs.build/en/32968/

@mergify mergify bot added the documentation Improvements or additions to documentation label Jan 23, 2026
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 introduces a new example script for running a single batch inference with optional profiling capabilities. The script is well-structured and serves its purpose as a clear example. I've identified one high-severity issue related to input validation for the profiling directory path, which could lead to a crash. My suggestion addresses this by adding a check to ensure the path is a valid directory.

Comment on lines +55 to +56
if not profile_dir:
raise ValueError("--profile-dir must be set when profiling is enabled.")
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The script crashes if --profile-dir points to an existing file instead of a directory. This is because the underlying PyTorch profiler expects a directory path and will fail when trying to create it if a file with the same name exists. Adding a check to validate the path will make the script more robust and prevent unexpected crashes.

Suggested change
if not profile_dir:
raise ValueError("--profile-dir must be set when profiling is enabled.")
if not profile_dir:
raise ValueError("--profile-dir must be set when profiling is enabled.")
import os
if os.path.exists(profile_dir) and not os.path.isdir(profile_dir):
raise ValueError(
f"--profile-dir '{profile_dir}' exists and is not a directory.")

@mergify
Copy link

mergify bot commented Jan 23, 2026

Hi @LucasWilkinson, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy or markdownlint failing?
mypy and markdownlint are run differently in CI. If the failure is related to either of these checks, please use the following commands to run them locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10
# For markdownlint
pre-commit run --hook-stage manual markdownlint

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant