Skip to content

fix(solve): wait and collect results when --timeout 0#735

Merged
lewisjared merged 3 commits into
mainfrom
fix/solve-timeout-zero-skips-join
Jun 17, 2026
Merged

fix(solve): wait and collect results when --timeout 0#735
lewisjared merged 3 commits into
mainfrom
fix/solve-timeout-zero-skips-join

Conversation

@lewisjared

Copy link
Copy Markdown
Contributor

Description

ref solve --timeout 0 ran every diagnostic successfully into the scratch directory but copied nothing to the results directory and ingested nothing into the database.

Root cause

The default LocalExecutor only submits work in run(); all result handling (copying scratch → results, DB ingestion, mark_successful) happens inside join(). The solver only called join() when timeout > 0:

if timeout > 0:
    executor.join(timeout=timeout)

The CLI overloaded timeout as the "don't wait" sentinel (timeout=timeout if wait else 0), so --no-wait and --timeout 0 were the same thing internally. Passing --timeout 0 — which intuitively reads as "no time limit" — therefore skipped join() entirely, orphaning the completed work in scratch and leaving every Execution row stuck at successful=None.

Fix

  • Decouple whether to wait from how long: solve_required_executions now takes an explicit wait and joins whenever wait is True, regardless of the timeout value.
  • A non-positive timeout (<= 0) now means wait with no time limit in LocalExecutor.join and CeleryExecutor.join (HPC already did this via walltime).
  • The CLI passes wait and timeout through separately instead of collapsing --no-wait onto timeout=0.
  • Warn when --no-wait is used with an executor that only persists results during join (local, HPC), since skipping the join silently orphans completed work. This is driven by a new collects_results_on_join marker on each executor (True for local/HPC; False for synchronous, which collects in run(), and Celery, which collects via the handle_result worker callback).

Recovery for affected users

Existing scratch outputs from a previous --timeout 0 run can be recovered without re-computing via ref executions reingest --include-failed.

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

`ref solve --timeout 0` queued executions to the local process pool and then
exited without ever joining,
so completed outputs were left in the scratch directory
and never copied to the results directory or ingested into the database.

The timeout value was overloaded as the "don't wait" sentinel
(`timeout if wait else 0`),
so a `0` timeout meant fire-and-forget rather than the intuitive
"wait with no time limit".

Decouple waiting from the timeout:
`solve_required_executions` now takes an explicit `wait` and joins whenever it
is True, regardless of the timeout value.
A non-positive timeout (`<= 0`) means "wait with no time limit".

Warn when `--no-wait` is used with an executor that only persists results
during `join` (local and HPC),
since skipping the join would silently orphan completed work in scratch.
This is captured by a new `collects_results_on_join` marker on each executor.
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
core 93.31% <100.00%> (+0.02%) ⬆️
providers 91.79% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...mate-ref-celery/src/climate_ref_celery/executor.py 97.61% <100.00%> (+0.05%) ⬆️
.../climate-ref-core/src/climate_ref_core/executor.py 86.27% <100.00%> (+0.56%) ⬆️
packages/climate-ref/src/climate_ref/cli/solve.py 100.00% <ø> (ø)
...ckages/climate-ref/src/climate_ref/executor/hpc.py 64.54% <100.00%> (+0.14%) ⬆️
...ages/climate-ref/src/climate_ref/executor/local.py 96.55% <100.00%> (+0.04%) ⬆️
...limate-ref/src/climate_ref/executor/synchronous.py 100.00% <100.00%> (ø)
packages/climate-ref/src/climate_ref/solver.py 98.90% <100.00%> (+0.37%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lewisjared lewisjared merged commit e0c3129 into main Jun 17, 2026
24 checks passed
@lewisjared lewisjared deleted the fix/solve-timeout-zero-skips-join branch June 17, 2026 15:01
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.

1 participant