Skip to content

fix(test): reliable nightly GPU runs — phased execution as default, script-owned timings - #1643

Open
planetf1 wants to merge 8 commits into
mainfrom
fix/nightly-script-reliability
Open

fix(test): reliable nightly GPU runs — phased execution as default, script-owned timings#1643
planetf1 wants to merge 8 commits into
mainfrom
fix/nightly-script-reliability

Conversation

@planetf1

@planetf1 planetf1 commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Pull Request

Issue

Fixes #1642

Description

Summary. The nightly GPU run was unreliable on the shared H100 pool — five script bugs, and a single pytest process keeping Ollama + vLLM up at the same time (~72 GiB peak, at the mercy of co-tenants: 15 OOM failures observed from one neighbour). This PR fixes the bugs and makes phased execution the default: one pytest process per backend group, one CUDA context at a time, ~33 GiB peak, and the hf/vllm/base phases can run on an exclusive GPU. Every run now records per-test durations and JSON reports without callers passing any flags. Both modes verified green: 4309/0.

What changes for nightly callers

  • Default run is now phased (SERIAL_PHASES=1). Legacy single-process run: SERIAL_PHASES=0 --group-by-backend.
  • Report layout: a phased nightly writes one JSON report per phase (pytest_report_<name>.json) + phase_<name>.log + consolidated pytest_full.log. Consumers of the old single pytest_report.json must be updated (single-run mode still writes one).
  • Callers pass selection/verbosity only — durations + JSON report are baked in (override with your own --durations*/--json-report*).
  • Caller -m falls back to single-run with a loud warning (per-phase selection would otherwise apply the same -m four times).
  • Recommended LSF split (in the script header + test/README.md): PHASES=hf,vllm,base on -gpu "num=1" (exclusive) + PHASES=ollama on mode=shared:gmem=20G (Ollama's per-model workers can't run exclusive). Concurrent jobs need distinct MELLEA_LOGDIR and COVERAGE_FILE. A single mode=shared job still works.
  • New env vars: SERIAL_PHASES, PHASES, VLLM_READY_TIMEOUT (default 300 s), GPU_FREE_TIMEOUT, GPU_FREE_THRESHOLD_MB.

Observed issues (H100, 2026-09-09, pre-fix)

  1. Ollama version probe always failed → 1.9 GB tarball re-downloaded every run; parallel jobs raced (tar: File exists killed one of two).
  2. vLLM startup measured 54–181 s; the hardcoded 120 s readiness loop killed healthy runs.
  3. ollama serve shutdown left llama-server workers holding 5.4 GiB 120 s later.
  4. Warmup list pulled two examples-only models (~6.7 GiB) and missed llama3.2:1b (SOFAI e2e) → on-demand mid-test download.
  5. Single-process peak ~72 GiB; a 46.95 GiB co-tenant OOM-killed the vLLM phase (15 failures) while the same suite passed with no co-tenant pressure.
  6. No timings recorded unless the caller remembered the flags.

Changes made (one commit each)

  1. Probe extracts the first dotted version from --version output.
  2. VLLM_READY_TIMEOUT (default 300 s) + fast-fail when the server process dies during startup, pointing at vllm.log.
  3. Orphan-worker pkill on shutdown, scoped to this install's runtime dir.
  4. Model list aligned to what the tests use; sync obligation documented.
  5. Phased execution (default): per-phase processes/lifecycles, wait_gpu_free() release gate, PHASES=, baked-in reporting, -m guard, docs.
  6. Review hardening: the vLLM venv is only removed by the job that created it (concurrent split jobs share the path); the Ollama install runs only when this run uses Ollama; PHASES typos and zero-phase runs now fail loudly; pytest exit 5 (nothing selected in a phase) no longer fails the run; phased runs pass --group-by-backend so the conftest group warm-up fires; a vLLM start failure fails only the vllm phase, not the base phase; signal kills route through the shutdown trap.

Test timing (H100-80, full suite, --durations=0)

14 m 29 s wall (4308 passed). Groups: huggingface 412.5 s (47%) · ollama 247.9 s (29%) · base 136 s (16%) · openai_vllm 4.1 s · server start/stop ~40 s. Slowest: test_think_big 28.3 s; test_intrinsics_formatters.py 151.6 s (15 fresh model loads); test_groundedness_* 79.8 s; test_component_typing::test_generating 23.5 s clean, 179 s under co-tenant pressure. Phased mode: +17–23% wall for the 4-way split. DGX Spark (128 GB unified): ~1.5–2.5 h expected; use VLLM_GPU_MEM=0.2–0.25 (0.4 would reserve 51 GiB) — fuller discussion to follow.

Verification (final runs, LSF H100-80)

  • Phased two-job split (recommended pattern, final code): 4309 passed / 0 failed (4186 + 123)
  • Single-run + all fixes: 4309 passed / 0 failed (16:31)
  • Targeted: PHASES typo → loud failure; empty phase selection (exit 5) → treated as pass; a WITH_VLLM=1 job without the vllm phase does not delete the shared venv; release gate non-fatal with co-tenant resident

Follow-ups (out of scope)

  • Conftest's ollama group warmup hardcodes 3 models; should track the script's list.
  • Optional wait-for-free-GPU gate before vLLM start in single-run mode.
  • Ollama download URL is amd64-only — an arm64 box (e.g. DGX Spark) needs a preinstalled Ollama ≥ 0.32.2.
  • The installer's curl has no --max-time (pre-existing: hangs on nodes without egress).
  • WITH_EXAMPLES=1 runs pull examples-only models on demand (not in the warmup list by design).
  • WITH_VLLM auto-detect fires on GPU-less nodes (nvidia-smi -L exits 0 without devices); now degrades gracefully, detection could be tightened.
  • DGX Spark configuration guidance.

Testing

  • Tests added to the respective file if code was changed (shell script; verified end-to-end on LSF — see Verification above)
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

  • AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

  • Component
  • Requirement
  • Sampling Strategy
  • Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

@planetf1
fix(test): make Ollama version probe parse multi-line --version output
3bac9e2 
ollama --version prints a multi-line warning block to stdout (e.g.
"Warning: client version is 0.32.2"). The probe's awk '{print $NF}'
therefore produced the two-line string "instance\n0.32.2", which never
passed the sort -V -C comparison, so every run re-downloaded and
re-extracted the 1.9 GB Ollama tarball even when the right version was
installed — and concurrent runs raced the re-extraction (observed:
"tar: File exists" killing one of two parallel nightly jobs).

Extract the first dotted version number instead.

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
fix(test): make the vLLM readiness timeout configurable
bad55bb 
vLLM 0.29 startup (package import, weight load, torch.compile, CUDA graph
capture, API server startup) measured 54-181 s on H100s depending on
compile-cache warmth and whether instances start concurrently — the script's
hardcoded 120 s readiness loop killed healthy runs that needed 143-181 s.

Add VLLM_READY_TIMEOUT (default 120 s) and use it in the readiness loop and
the failure message.

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
fix(test): kill orphaned Ollama llama-server workers on shutdown
a9ab091 
Stopping the 'ollama serve' parent leaves its llama-server worker children
alive; they keep holding GPU memory (observed 5.4 GiB still resident 120 s
after shutdown in a phased nightly run, caught by the post-phase GPU
release check).

pkill the workers scoped to this install's runtime directory — other
jobs' workers carry their own runtime path in their command line and are
not affected.

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
fix(test): align the Ollama model list with models the tests use
d9148d3 
The warmup list included two models no test in test/ requests (examples
only, ~6.7 GiB of warmup residency per nightly) and omitted llama3.2:1b,
which the SOFAI e2e tests do request — the server downloaded it on demand
mid-test, adding a hidden network dependency.

Replace the two examples-only entries with llama3.2:1b and document the
sync obligation.

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
feat(test): phased nightly execution, now the default
4867436 
SERIAL_PHASES (default 1) runs each backend group as its own pytest
process with per-phase server lifecycles: only one CUDA context is alive
at a time, peak GPU memory drops from ~72 GiB to ~33 GiB, in-process HF
models are released between phases, and the single-context phases
(hf/vllm/base) can run on an exclusive LSF GPU (-gpu "num=1").

- PHASES=hf,ollama,vllm,base selects which phases run, so one script can
  drive the recommended two-job split (exclusive for single-context
  phases; mode=shared for the Ollama phase, whose per-model llama-server
  workers need multi-process GPU access).
- wait_gpu_free() release gate between phases and after each server
  stop (max_wait 120 s, poll 10 s, threshold 2 GiB).
- The script owns the diagnostic report contract: full per-test
  durations and JSON reports under the log directory in both modes
  (pytest_full.log + pytest_report.json single-run; phase_<name>.log +
  pytest_report_<name>.json per phase). Callers pass selection/verbosity
  args only; a caller -m falls back to single-run mode with a warning.
- SERIAL_PHASES=0 keeps the legacy single-process --group-by-backend run.

Document the LSF submission patterns in the script header and update
test/README.md (CI tier table, nightly description, local-run example,
group warm-up note).

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@github-actions github-actions Bot added the bug Something isn't working label Sep 10, 2026
@planetf1
fix(test): harden the phased nightly script after code review
a741706 
- stop_vllm only removes a vLLM venv this invocation created
  (VLLM_VENV_OWNED); the recommended two-job split no longer lets the
  ollama-only job delete the venv its sibling runs vLLM from, and
  VLLM_EXTERNAL runs no longer lose a venv the script never built.
- The Ollama install block runs only when this run will use Ollama
  (single-run, or phased with the ollama phase enabled), so the
  exclusive split's other job skips a 1.9 GB download and the two
  jobs no longer race the install path on a fresh node.
- PHASES tokens are validated (typo now dies loudly instead of
  silently running zero phases with exit 0); phased runs also fail
  if no phase actually executed.
- run_phase treats pytest exit 5 (nothing selected in this phase)
  as a pass, so documented -k selections no longer fail the run.
- wait_gpu_free survives a transient nvidia-smi failure (its own
  contract is diagnostic, not fatal) and its timeout wording no
  longer calls co-tenant memory a leak.
- Phased pytest runs now pass --group-by-backend, so the conftest
  group warm-up/eviction (keep_alive=-1 pinning) actually fires in
  the default mode; each run also starts from a clean coverage file.
- A vLLM start failure now records the phase as failed and continues
  with the base phase instead of aborting the whole run.
- vLLM readiness default raised 120s -> 300s (observed cold starts
  hit 181s; the loop breaks on readiness so a high default is free).
- The -m guard also catches the attached -mVALUE form; caller
  --durations-min no longer suppresses the built-in durations list;
  removed the --no-json-report arm (not a real pytest option).
- Signal kills (bkill, wall-time) now route through the EXIT trap so
  servers are torn down; trap/INT/TERM added.
- Fixed the stale Ollama readiness message (120s, not 30s) and the
  orphan-pkill scoping comment (per OLLAMA_BIN, not per job).
- Header docs: per-job MELLEA_LOGDIR/COVERAGE_FILE in the two-job
  example, gpu_release_gate.log in the report inventory, node-id
  wording removed; README warm-up note corrected.

Assisted-by: opencode
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
planetf1 marked this pull request as ready for review September 10, 2026 11:12
@planetf1
planetf1 requested a review from a team as a code owner September 10, 2026 11:12
@planetf1
fix(test): merge per-phase JSON reports for callers with --json-repor…
5797a94 
…t-file

In phased mode, a caller-supplied --json-report-file was forwarded verbatim
into every phase's pytest invocation, so each phase overwrote it and only
the last phase's results survived. Strip the caller's --json-report-file
out of per-phase args and merge the phases' own pytest_report_<name>.json
files into the caller's requested path once all phases finish.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
ajbozarth
ajbozarth requested changes Sep 10, 2026
View reviewed changes

@ajbozarth ajbozarth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some feedback from Claude. Solid PR — verified the phase marker partition holds (clean split, no double-runs, addopts only carries -m "not slow" so re-stating it per phase is correct, and the server-dependent tests land in the right phase). Three findings below; only the first is worth acting on.

Comment thread test/scripts/run_tests_with_ollama_and_vllm.sh
Comment thread test/scripts/run_tests_with_ollama_and_vllm.sh Outdated
Comment thread test/scripts/run_tests_with_ollama_and_vllm.sh
@planetf1
fix(test): address code review on the JSON report merge step
5086611 
- Make the per-phase JSON merge non-fatal: wrap the uv run call in
  set +e/set -e so a truncated or malformed pytest_report_p*.json (e.g. a
  phase OOM-killed mid-write) can't abort the script under set -e before
  EXIT_CODE is recorded, which would surface a merge traceback instead of
  the real per-phase pass/fail.
- Skip an individual unreadable report with a warning instead of failing
  the whole merge, so the other phases' results still make it through.
- Truncate pytest_full.log once before phase 1 so a reused MELLEA_LOGDIR
  doesn't mix this run's phased output with a previous run's.
- Name WITH_VLLM in the "no phases executed" die message, since a
  WITH_VLLM=0 vllm-only PHASES selection is the likely cause.

Addresses review from ajbozarth on PR #1643.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ajbozarth ajbozarth ajbozarth approved these changes
@avinash2692 avinash2692 Awaiting requested review from avinash2692
@markstur markstur Awaiting requested review from markstur markstur is a code owner automatically assigned from generative-computing/mellea-maintainers

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nightly GPU run: reliability failures on shared H100s and no per-test timing data

2 participants

@planetf1 @ajbozarth