feat: support custom throughput test cases

[cyber-pioneer]

PR Category

Tools

PR Type

Test Case

Description

Add optional cli args port, test-cases, served-model-name for benchmarks/benchmark_throughput_serve.py

Testing

Each test case: [input_len, output_len, concurrency, num_prompts]

python3 benchmarks/benchmark_throughput_serve.py --model /models/Qwen3.6-35B-A3B --port 8002 --test-cases '[[4096,1024,64,256],[16384,1024,64,256]]'

[Copilot]

Pull request overview

Adds support for specifying custom throughput benchmark cases via CLI in benchmark_throughput_serve.py, extending the existing default/all-case selection without changing benchmark execution logic.

Changes:

• Add --test-cases CLI option that accepts a JSON list of custom test cases.
• Validate custom test-case structure/types and make it mutually exclusive with --enable-all.
• Allow parse_args(argv=None) for easier programmatic invocation while preserving default CLI behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

benchmarks/benchmark_throughput_serve.py:174

• --trust-remote-code is currently always enabled for the benchmark client. This changes the script’s security posture and makes it harder to run in environments where trusting remote code is disallowed. Prefer making it opt-in via a CLI flag (and only append it when requested).

        "--dataset-name",
        "random",
        "--ignore-eos",
        "--trust-remote-code",
    ]

benchmarks/benchmark_throughput_serve.py:162

• build_common_args() hard-codes --model and --tokenizer to the same value, which prevents using a separate served model name (request model field) from the tokenizer identifier/path. Using an explicit served_model_name avoids failures when the server exposes a different model name.

def build_common_args(model, port):
    return [
        "vllm",
        "bench",
        "serve",
        "--backend",
        "vllm",
        "--model",
        model,
        "--tokenizer",

benchmarks/benchmark_throughput_serve.py:424

• If you add --served-model-name / --trust-remote-code options (or otherwise decouple served model name from tokenizer), main() will also need to pass the served model name into build_common_args() and append --trust-remote-code conditionally. Otherwise the new CLI args would be ignored and the benchmark would still always use the tokenizer value as the served model name.

    args = parse_args()
    common_args = build_common_args(args.model, args.port)

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

benchmarks/benchmark_throughput_serve.py:125

• The --model CLI option is documented as “used to load the tokenizer”, but the script also passes it to vllm bench serve as --model (in addition to --tokenizer). This can mislead users about what value is expected; at minimum, the help text should reflect the dual use.

    parser.add_argument(
        "--model",
        type=existing_path,
        required=True,
        help="Local model path used to load the tokenizer.",
    )

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

benchmarks/benchmark_throughput_serve.py:103

• --model is typed with existing_path, which rejects non-local model/tokenizer identifiers (e.g., HuggingFace IDs like Qwen/Qwen3-Next-80B-A3B-Instruct). Other benchmark scripts in this repo pass HF tokenizer IDs to vllm bench serve, so this validation makes the CLI unnecessarily restrictive and breaks those workflows.

Consider validating only when the value resolves to an existing local path, and otherwise leaving the value as-is for vllm to resolve/download.

This issue also appears on line 120 of the same file.

def existing_path(value):
    path = os.path.abspath(os.path.expanduser(value))
    if not os.path.exists(path):
        raise argparse.ArgumentTypeError(f"model path does not exist: {path}")
    return path

benchmarks/benchmark_throughput_serve.py:125

• The --model help text says this must be a local path, but vllm bench serve commonly accepts remote model/tokenizer IDs as well (and other benchmark scripts in this repo use HF IDs). If --model is intended to support both local paths and HF IDs, the help string should reflect that to avoid confusing users.

    parser.add_argument(
        "--model",
        type=existing_path,
        required=True,
        help="Local model path used to load the tokenizer.",
    )

benchmarks/benchmark_throughput_serve.py:174

• --trust-remote-code is always enabled in the generated vllm bench serve command. This opts users into executing arbitrary model code by default, which is a security footgun for a benchmark script (especially when users experiment with new/untrusted models). Make this opt-in via a CLI flag, or drop it from the default command and document when it’s needed.

        "--port",
        str(port),
        "--dataset-name",
        "random",
        "--ignore-eos",
        "--trust-remote-code",
    ]

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

benchmarks/benchmark_throughput_serve.py:178

• --trust-remote-code is always enabled in the benchmark command. This allows executing arbitrary code from model repositories and should generally be opt-in (or at least clearly gated behind a CLI flag) rather than the default behavior.

        "--dataset-name",
        "random",
        "--ignore-eos",
        "--trust-remote-code",
    ]

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

benchmarks/benchmark_throughput_serve.py:103

• existing_path() converts the provided model path to an absolute path. Since args.model is also used as the default request model name when --served-model-name is omitted, this normalization can change the model identifier and fail to match what the server is actually serving (e.g., server started with a relative path). Consider avoiding abspath() here so the model string remains what the user supplied.

def existing_path(value):
    path = os.path.abspath(os.path.expanduser(value))
    if not os.path.exists(path):
        raise argparse.ArgumentTypeError(f"model path does not exist: {path}")
    return path

benchmarks/benchmark_throughput_serve.py:19

• The examples below use --served-model-name qwen and --port 8000, but the server startup command doesn’t set a matching --served-model-name (and doesn’t show the port). This can lead to a model-name mismatch when running the benchmark.

#  1. Start the server as follows (adjust model path and args as needed):
# vllm serve /models/Qwen3.6-35B-A3B --tensor-parallel-size 2 --max-model-len 262144 --no-enable-log-requests --no-enable-prefix-caching

benchmarks/benchmark_throughput_serve.py:125

• PR description says it “Add optional cli args port, test-cases, served-model-name”, but this change also introduces a new required --model argument. If this is intended, the PR description/testing instructions should be updated to mention the breaking CLI change; otherwise consider keeping a default model value for backward compatibility.

        "--model",
        type=existing_path,
        required=True,
        help="Local model path used to load the tokenizer.",
    )