Skip to content

Fix the guidellm benchmark --sample-requests command line option#591

Merged
sjmonson merged 2 commits intovllm-project:mainfrom
natoscott:fix-sample_requests
Feb 11, 2026
Merged

Fix the guidellm benchmark --sample-requests command line option#591
sjmonson merged 2 commits intovllm-project:mainfrom
natoscott:fix-sample_requests

Conversation

@natoscott
Copy link
Contributor

Summary

Fix the guidellm benchmark --sample-requests command line option

Details

The default behavior of saving requests in the benchmarks.json file can result in very large JSON output files. These can be quite unwieldy if all one needs is benchmark results.

Such verbose logging could fill the root or other local filesystem, and accidentally cause benchmark or machine failure.

I had previously used --sample-requests=0 to avoid this problem, but it regressed at some point late last year (v0.4?). I am having good success with this patch to address the issue though.


Use of AI

🤖 Generated with Claude Code

  • "I certify that all code in this PR is my own, except as noted below."

(none of this code is my own, it is 100% generated by AI)

Use of AI

  • Includes AI-assisted code completion
  • Includes code generated by an AI application
  • Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes ## WRITTEN BY AI ##)

Copy link
Collaborator

@sjmonson sjmonson left a comment

Choose a reason for hiding this comment

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

Not a huge fan of this because its papering over some bad design, but it will do for now.

The request_samples defaults are all over the place; by default we should not sample so needs this patch as well:

diff --git a/src/guidellm/benchmark/benchmarker.py b/src/guidellm/benchmark/benchmarker.py
index 56cdb9a7..c0caba40 100644
--- a/src/guidellm/benchmark/benchmarker.py
+++ b/src/guidellm/benchmark/benchmarker.py
@@ -64,7 +64,7 @@ class Benchmarker(
         environment: Environment,
         warmup: TransientPhaseConfig,
         cooldown: TransientPhaseConfig,
-        sample_requests: int | None = 20,
+        sample_requests: int | None = None,
         prefer_response_metrics: bool = True,
         progress: (
             BenchmarkerProgress[BenchmarkAccumulatorT, BenchmarkT] | None
diff --git a/src/guidellm/benchmark/schemas/base.py b/src/guidellm/benchmark/schemas/base.py
index 9a41171f..9370c215 100644
--- a/src/guidellm/benchmark/schemas/base.py
+++ b/src/guidellm/benchmark/schemas/base.py
@@ -273,7 +273,7 @@ class BenchmarkConfig(StandardBaseDict):
         description="Constraint definitions applied to scheduler strategy execution",
     )
     sample_requests: int | None = Field(
-        default=20,
+        default=None,
         description="Request count for statistical sampling in final metrics",
     )
     warmup: TransientPhaseConfig = Field(
diff --git a/src/guidellm/benchmark/schemas/generative/entrypoints.py b/src/guidellm/benchmark/schemas/generative/entrypoints.py
index 45d9a4b2..e85a5ba5 100644
--- a/src/guidellm/benchmark/schemas/generative/entrypoints.py
+++ b/src/guidellm/benchmark/schemas/generative/entrypoints.py
@@ -252,7 +252,7 @@ class BenchmarkGenerativeTextArgs(StandardBaseModel):
     )
     # Benchmarker configuration
     sample_requests: int | None = Field(
-        default=10,
+        default=None,
         description="Number of requests to sample for detailed metrics (None for all)",
     )
     warmup: int | float | dict | TransientPhaseConfig | None = Field(

@natoscott
Copy link
Contributor Author

@sjmonson thanks for reviewing - would you prefer those as a follow-up commit or rolled into the original?

Re "by default we should not sample" - that'd be my preference too, I think it'd be better to opt-in.

@sjmonson
Copy link
Collaborator

would you prefer those as a follow-up commit or rolled into the original?

No preference, either is fine.

natoscott and others added 2 commits February 11, 2026 11:41
The default behaviour of saving requests in the benchmarks.json file
can result in extrememly large JSON output there.  This can be quite
unwieldy if all one needs is benchmark results.

Such verbose logging could fill the root or other local filesystem,
and accidentally cause benchmark or machine failure.

I had previously used --sample-requests=0 to avoid this problem, but
it regressed at some point late last year (v0.4?).  I am having good
success with this patch to address the issue though.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Nathan Scott <nathans@redhat.com>
Reviewed-by: Samuel Monson <smonson@redhat.com>
Signed-off-by: Samuel Monson <smonson@redhat.com>
Reviewed-by: Nathan Scott <nathans@redhat.com>
@sjmonson sjmonson merged commit a5122b6 into vllm-project:main Feb 11, 2026
16 checks passed
sjmonson added a commit that referenced this pull request Mar 12, 2026
## Summary

Fix the guidellm benchmark --sample-requests command line option

## Details

The default behavior of saving requests in the benchmarks.json file can
result in very large JSON output files. These can be quite unwieldy if
all one needs is benchmark results.

Such verbose logging could fill the root or other local filesystem, and
accidentally cause benchmark or machine failure.

I had previously used --sample-requests=0 to avoid this problem, but it
regressed at some point late last year (v0.4?). I am having good success
with this patch to address the issue though.

---

## Use of AI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

- [ ] "I certify that all code in this PR is my own, except as noted
below."

(none of this code is my own, it is 100% generated by AI)

## Use of AI

- [ ] Includes AI-assisted code completion
- [x] Includes code generated by an AI application
- [ ] Includes AI-generated tests (NOTE: AI written tests should have a
docstring that includes `## WRITTEN BY AI ##`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants