[thomasdhc]

Description

• Move the benchmark CI job timeout knobs into nightly-benchmark.yaml as the single source of truth: default_timeout_s (now also consumed by generate_ci_tests.py, matching the runner), plus new cleanup_timeout_s (60s) and min_timeout_s (600s). Generated job time is now max(entry.timeout_s or default_timeout_s + cleanup_timeout_s, min_timeout_s), replacing the hardcoded MIN_TIME_S constant.

• This fixes two issues with the old max(timeout_s, 600) logic: entries omitting timeout_s now default consistently with the runner (7200s instead of 600s), and every job gets a cleanup buffer so long-running benchmarks (e.g. 4h) no longer get a wall-clock equal to their own timeout with no room to terminate and clean up. The 600s floor is retained for short entries to cover container setup.

Usage

# Add snippet demonstrating usage

Checklist

• I am familiar with the Contributing Guide.
• New or Existing tests cover these changes.
• The documentation is up to date with these changes.

[rlratzel]

LGTM, thanks!

[greptile-apps]

Greptile Summary

This PR moves CI job wall-clock timeout configuration out of hardcoded constants in generate_ci_tests.py and into nightly-benchmark.yaml as the single source of truth. The timeout computation formula also changes to always add a cleanup_timeout_s buffer on top of each entry's effective timeout, floored at min_timeout_s.

• Formula change: old max(entry.timeout_s or 600, 600) → new max((entry.timeout_s or default_timeout_s) + cleanup_timeout_s, min_timeout_s). Two concrete fixes: entries without timeout_s (e.g., audio_fleurs_*) previously got a 600 s CI job instead of the runner's 7200 s default, and long-running entries (e.g., 4 h audio_readspeech_*) previously got a wall-clock equal to their own timeout with no room for post-run cleanup.
• Config addition: cleanup_timeout_s: 60 and min_timeout_s: 600 are added to nightly-benchmark.yaml alongside the pre-existing default_timeout_s: 7200, with a comment explaining how generate_ci_tests.py uses all three knobs.

Confidence Score: 4/5

Safe to merge. The formula change is correct and represents a genuine fix for two distinct CI timing bugs.

The new wall-clock formula (entry timeout + cleanup buffer, floored at min) correctly handles all three cases: entries without an explicit timeout now get 7260 s instead of 600 s (matching the runner), long-running entries like the 4 h audio_readspeech benchmarks now get 60 s post-run cleanup headroom, and short entries still hit the 600 s container-setup floor.

benchmarking/tools/generate_ci_tests.py — the generate_job call at line 114 passes all three new knobs positionally; a follow-up could use keyword arguments for readability, but there is no functional concern.

Important Files Changed

Filename	Overview
benchmarking/tools/generate_ci_tests.py	Replaces hardcoded MIN_TIME_S with three config-driven fallback constants and passes them through generate_pipeline to generate_job; the new formula max(timeout_s + cleanup_timeout_s, min_timeout_s) is logically correct for all current entries.
benchmarking/nightly-benchmark.yaml	Adds cleanup_timeout_s: 60 and min_timeout_s: 600 alongside the pre-existing default_timeout_s: 7200; placement and comment are clear and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Entry in nightly-benchmark.yaml] --> B{Has timeout_s?}
    B -- Yes --> C[use entry.timeout_s]
    B -- No --> D[use default_timeout_s from YAML or 7200s fallback]
    C --> E[effective_timeout + cleanup_timeout_s]
    D --> E
    E --> F{effective_timeout + cleanup > min_timeout_s?}
    F -- Yes --> G[CI wall-clock = timeout_s + cleanup_timeout_s]
    F -- No --> H[CI wall-clock = min_timeout_s 600s floor]
    G --> I[seconds_to_time HH:MM:SS for SLURM TIME variable]
    H --> I

Loading

_{Reviews (1): Last reviewed commit: "feat(benchmarking): config-driven CI tim..." | Re-trigger Greptile}