-
Notifications
You must be signed in to change notification settings - Fork 26
DRAFT: feat(config): consolidate all durations and deadlines into one Timeouts model #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5b2c348
35f118f
bb8833b
ded413f
d6ac8ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,8 +96,9 @@ Flag names shown as `--full.dotted.path --alias`. Both forms work. | |
| - `--model-params.max-new-tokens --max-output-tokens` - Max output tokens (default: 1024) | ||
| - `--model-params.osl-distribution.min --min-output-tokens` - Min output tokens (default: 1) | ||
| - `--model-params.streaming --streaming` - Streaming mode: auto/on/off (default: auto) | ||
| - `--runtime.min-duration-ms --duration` - Min duration: ms default, or with suffix (600s, 10m) (default: 600000) | ||
| - `--runtime.n-samples-to-issue --num-samples` - Explicit sample count override | ||
| - `--min-duration-ms --duration` - Min perf-phase duration: ms default, or with suffix (600s, 10m); sample count = QPS x duration | ||
| - `--runtime.n-samples-to-issue --num-samples` - Explicit sample count | ||
| - `--duration` and `--num-samples` are mutually exclusive; omit both to issue the dataset once (the default) | ||
| - `--client.num-workers --workers` - HTTP workers (-1=auto, default: -1) | ||
| - `--client.max-connections --max-connections` - Max TCP connections (-1=unlimited) | ||
| - `--endpoint-config.api-key --api-key` - API authentication | ||
|
|
@@ -106,7 +107,7 @@ Flag names shown as `--full.dotted.path --alias`. Both forms work. | |
| Note: applies to CLI-driven `benchmark offline` / `benchmark online`; `benchmark from-config` | ||
| does not expose a CLI override for `report_dir`. Set it in the YAML only if you need to control | ||
| the output location; otherwise a default report directory is used. | ||
| - `--timeout` - Global timeout in seconds | ||
| - `--timeout` - Whole-run watchdog in seconds (off by default). If it fires, the run is aborted and the report is marked INTERRUPTED (exits non-zero). | ||
| - `--enable-cpu-affinity / --no-cpu-affinity` - NUMA-aware CPU pinning (default: true) | ||
|
|
||
| **Online-specific:** | ||
|
|
@@ -284,10 +285,10 @@ datasets: | |
|
|
||
| settings: | ||
| runtime: | ||
| min_duration_ms: 600000 # 10 minutes | ||
| n_samples_to_issue: null # Optional: explicit sample count (null = auto-calculate) | ||
| scheduler_random_seed: 42 # For Poisson/distribution sampling | ||
| dataloader_random_seed: 42 # For dataset shuffling | ||
| timeouts: | ||
| min_duration_ms: 600000 # 10 minutes (or set runtime.n_samples_to_issue instead; omit both = dataset once) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trying to think why min_duration is a timeout? Sounds like it's a lower bound instead of a upper bound |
||
| load_pattern: | ||
| type: "max_throughput" | ||
| target_qps: 10.0 | ||
|
|
@@ -325,8 +326,8 @@ Note: For submission configs, `model_params.name` is optional when `submission_r | |
|
|
||
| **Sample Count Control:** | ||
|
|
||
| - Priority: `--num-samples` > calculated (target_qps × duration) > dataset size | ||
| - Default duration: 600000ms (10 minutes) | ||
| - `--num-samples` and `--duration` are mutually exclusive: explicit count, or calculated (target_qps × duration) | ||
| - Default (neither set): issue the dataset once | ||
|
|
||
| **Mode Requirements:** | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,8 +74,7 @@ Waiting for 5 responses... | |
| uv run inference-endpoint -v benchmark offline \ | ||
| --endpoints http://localhost:8765 \ | ||
| --model Qwen/Qwen3-8B \ | ||
| --dataset tests/assets/datasets/dummy_1k.jsonl \ | ||
| --duration 0 | ||
| --dataset tests/assets/datasets/dummy_1k.jsonl | ||
|
|
||
| # Production test with custom params and report generation | ||
| uv run inference-endpoint -v benchmark offline \ | ||
|
|
@@ -115,7 +114,6 @@ uv run inference-endpoint -v benchmark online \ | |
| --endpoints http://localhost:8765 \ | ||
| --model Qwen/Qwen3-8B \ | ||
| --dataset tests/assets/datasets/dummy_1k.jsonl \ | ||
| --duration 0 \ | ||
| --load-pattern poisson \ | ||
| --target-qps 100 \ | ||
| --report-dir online_benchmark_report | ||
|
|
@@ -311,9 +309,8 @@ uv run inference-endpoint benchmark online \ | |
|
|
||
| **Sample Count Control:** | ||
|
|
||
| - Use `--duration 0` when you want a local test to stop after exhausting the dataset instead of running for the default timed duration | ||
| - Sample priority: `--num-samples` > dataset size (when `--duration 0`) > calculated (target_qps × duration) | ||
| - Default duration: 600000ms (10 minutes) | ||
| - By default (no `--num-samples`, no `--duration`) a run stops after issuing the dataset once | ||
| - `--num-samples` (explicit count) and `--duration` (count = target_qps × duration) are mutually exclusive | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does duration work when running without target_qps (fixed concurrency or offline)? |
||
|
|
||
| **Testing & Debugging:** | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,11 +19,13 @@ datasets: | |
|
|
||
| settings: | ||
| runtime: | ||
| min_duration_ms: 6000 # 6 seconds | ||
| max_duration_ms: 60000 # 1 minute | ||
| scheduler_random_seed: 137 # For Poisson/distribution sampling | ||
| dataloader_random_seed: 111 # For dataset shuffling | ||
|
|
||
| timeouts: | ||
| min_duration_ms: 6000 # 6 seconds | ||
| max_duration_ms: 60000 # 1 minute | ||
|
Comment on lines
+26
to
+27
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit weird to put runtime lower/upper bound in the timeouts? |
||
|
|
||
| load_pattern: | ||
| type: "max_throughput" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean if both are set, both will be omitted?
What is the current loadgen behavior? I am thinking whether taking the max of the 2 will make more sense or we should error out