Inference: config validation and failure handling

[stuitje]

Description

This PR validates the reference PROTEUS config used in an inference run the same way PROTEUS does, so it rejects bad configs immediately without wasting computation time, and improves failure handling in PROTEUS workers. Covers #803. It also adds an optional spectral-file cache so that runs sharing a stellar spectrum reuse one prepared runtime.sf instead of each building their own. It does not yet change the way workers are dispatched (e.g., keeping the Julia instance alive between workers) which is a more drastic change and will be covered in a next PR.

Files changed:

Inference files

• src/proteus/inference/inference.py
  Added validate_reference_config / parameter_bounds: runs the PROTEUS config checks on the reference toml and on the two parameter bounds before inference launch. Moved the CPU-count and ref-config-exists checks ahead of safe_rm of the output dir. Implements abort_on_failure (user-activated, default off) and calls summarise_failures before the results summary.
• src/proteus/inference/failures.py (new since first PR draft)
  Failure handling moved out of inference.py, objective.py and utils.py to keep those files shorter. Holds the ProteusRunFailure dataclass (status, exit code, logfile, console log, swept parameters, failure/excluded), record_failure / read_failure_records (each failed or excluded run is added as a row to failures.csv when it happens), find_run_logfile and summarise_failures (logs failures/excluded by cause at the end of the study).
• src/proteus/inference/objective.py
  Child stdout/stderr now logged to <run>_console.log. A timeout, a non-zero exit or an unreadable helpfile now becomes a failure instead of a RuntimeError. Distinguishes real failures (status 20-28, 0, 1, missing status) from excluded failure_codes (eg status 11). Real failures are logged at warning, excluded ones at info. Added apply_nested_updates (from update_toml, now also used during startup validation), run_output_dir and WORKER_CONFIG_OVERRIDES (plots off, logging WARNING, archive_mod = 0). Points every worker at a shared spectral_cache folder in the study output.
• src/proteus/inference/utils.py
print_results warns on unscored optimisation evaluations, and raises if none of them was scored.
• src/proteus/inference/async_BO.py
  worker split into a thin wrapper plus _worker_loop, so a dying worker logs its traceback and always releases its busy point. Workers reattach to the study logfile when started with spawn (macOS). parallel_process reports workers that exited non-zero and raises when no optimisation step completed.
• src/proteus/inference/BO.py
  Busy points selected by worker key rather than list position (a finished worker is absent from B, so positional selection picked the wrong entry); handles the no-other-busy-worker case.
• src/proteus/inference/plot.py
plot_result_correlation filters the i_* glob to directories.
• src/proteus/inference/gen_D_init.py
  Docstring only. failure_codes reworded.

Source files

• src/proteus/cli.py
ConfigRejectedError refused config is now logged using the fwl logger at ERROR.
• src/proteus/utils/logs.py
attach_worker_logfile, attaches a file handler to the fwl logger in a worker process that has none.
• src/proteus/utils/helper.py
ReadStatus (moved from the inference code) and STATUS_MISSING, so a run that never wrote a status file is reported as such instead of as a generic error.
• src/proteus/atmos_clim/spectral_cache.py (new since first PR draft)
  cache_key / seed_from_cache / store_in_cache. Keeps one copy of the prepared spectral file pair (runtime.sf, runtime.sf_k), keyed by spectral group, bands, the base spectral file and a hash of the stellar spectrum. Mainly for inference runs, but possibly useful outside of this, so it is a normal config option.
• src/proteus/atmos_clim/agni.py
  Seeds runtime.sf from the cache when atmos_clim.spectral_cache is set, and stores it once AGNI has built it successfully. The I/O folder is now decided before the spectral file, since that is where the prepared pair lives.
• src/proteus/config/_atmos_clim.py
  New spectral_cache option (str or none, default none = off).

Input files

• input/inference/example.infer.toml
  Describes failure_codes as exclusions, adds abort_on_failure = false
• input/all_options.toml
  Adds spectral_cache = "none".

docs

• docs/How-to/inference.md
  New 'Failed and excluded simulations' section, failures.csv in the output list, and failures.py in the project structure overview.
• docs/Reference/config/atmosphere.md, docs/Reference/config/config_schema.json
spectral_cache entry.

Other

• .gitignore
  Ignores *.out and my local config folders I use to test the inference.

Validation of changes

I have ran the inference in a new setup with working and old configs (which should raise). The logged failures work well in my opinion.

Test configuration: Linux, Python 3.12.14.

Tests run:

• pytest tests/inference/, all passed
• pytest -m "unit and not skip and not slow and not integration" --ignore=tests/examples, passed

I ran:

bash tools/validate_test_structure.sh
python tools/check_test_quality.py --check

Checklist

• I have followed the contributing guidelines
• My code follows the style guidelines of this project
• I have performed a self-review of my code
• My changes generate no new warnings or errors
• I have checked that the tests still pass on my computer
• I have updated the docs, as appropriate
• I have added tests for these changes, as appropriate
• I have checked that all dependencies have been updated, as required

[codecov]

Codecov Report

❌ Patch coverage is 94.32432% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.23%. Comparing base (ca252b3) to head (2bb0f2a).

Files with missing lines	Patch %	Lines
src/proteus/atmos_clim/spectral_cache.py	78.46%	14 Missing ⚠️
src/proteus/inference/async_BO.py	87.09%	3 Missing and 1 partial ⚠️
src/proteus/inference/inference.py	95.23%	2 Missing ⚠️
src/proteus/atmos_clim/agni.py	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #876      +/-   ##
==========================================
+ Coverage   94.15%   94.23%   +0.07%     
==========================================
  Files         118      120       +2     
  Lines       18641    18967     +326     
  Branches     3187     3227      +40     
==========================================
+ Hits        17552    17874     +322     
+ Misses       1089     1085       -4     
- Partials        0        8       +8     

Flag	Coverage Δ
unit-tests	88.90% <94.32%> (+0.07%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

Multiple moderate issues remain in failure propagation, validation, logging, reporting, and plotting.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds preflight PROTEUS config validation and improved inference failure handling, including failure records, summaries, and worker lifecycle updates.

Changes:

• Validates configs and parameter bounds before execution.
• Captures and summarizes failed or excluded simulations.
• Improves worker bookkeeping, plotting, logging, documentation, and tests.

File summaries

File	Description
tests/test_cli.py	CLI logging tests
tests/inference/test_utils_branches.py	Failure summary tests
tests/inference/test_transforms.py	Timeout behavior tests
tests/inference/test_plot.py	Plot filtering tests
tests/inference/test_objective.py	Failure-handling tests
tests/inference/test_inference.py	Validation tests
tests/inference/test_bo.py	Busy-point selection tests
tests/inference/test_async_bo.py	Worker lifecycle tests
src/proteus/inference/utils.py	Failure aggregation and result reporting
src/proteus/inference/plot.py	Case-directory filtering
src/proteus/inference/objective.py	Run failure handling and records
src/proteus/inference/inference.py	Startup validation and failure summaries
src/proteus/inference/gen_D_init.py	Failure-code documentation
src/proteus/inference/BO.py	Busy-point ownership handling
src/proteus/inference/async_BO.py	Worker lifecycle and exit reporting
src/proteus/cli.py	Configuration refusal logging
input/inference/example.infer.toml	Failure-handling options and descriptions
docs/How-to/inference.md	Failure and exclusion guidance
.gitignore	Local inference artifacts

Review details

Suppressed comments (8)

src/proteus/inference/async_BO.py:375

• When abort_on_failure is enabled, J re-raises a ProteusRunFailure, so this worker exits non-zero. This code only logs the dead worker and continues whenever another worker has added a point, meaning the remaining workers finish and the study can still produce a summary instead of stopping at the first failure as documented. Propagate the failure to the parent, or make this branch terminate the study when abort mode is active.

    died = [wid for wid, p in enumerate(procs) if p.exitcode != 0]
    if died:
        names = ', '.join(str(wid) for wid in died)
        log.error(
            f'{len(died)} of {n_workers} workers stopped before the evaluation budget '

src/proteus/inference/inference.py:281

• len(D_final['X']) counts evaluations that appended a result, not all evaluations attempted. A worker can die inside _worker_loop before the append while other workers have already written failure records, so this smaller denominator makes the failure fraction and the reported number of real evaluations inaccurate and can trigger the >50% warning spuriously. Track the requested/started count in the coordinator or report this as completed evaluations instead of attempts.

    summarise_failures(dirs['output'], len(D_final['X']))

src/proteus/inference/inference.py:111

• The schema permits zero for planet.elements.H_budget, but this key is log-scaled by variable_is_logarithmic and _log10_bounds rejects any non-positive bound. A sweep such as [0, 1] therefore passes this startup validation and only fails later in create_init, after the output directory has been removed. Apply the same positivity check used by the transform to logarithmic parameter bounds here so invalid sweeps are rejected with the other preflight errors.

        low, high = float(value[0]), float(value[1])
        # TOML admits `inf` and `nan`. An infinite bound passes the schema's
        # own range checks and then makes every unnormalised sample infinite.
        if not (math.isfinite(low) and math.isfinite(high)):
            raise ValueError(
                f"Bounds for inference parameter '{key}' must be finite, got {value!r}"
            )
        if low >= high:

src/proteus/inference/objective.py:107

• Although the console output is kept on disk during the run, _tail_file reads the entire file into memory and only then keeps the last 40 lines. A chatty failed child can therefore consume memory proportional to its full log and defeat the stated bounded-tail behavior. Stream only the last lines entries (for example with a bounded deque).

    try:
        with open(path, 'r', errors='replace') as f:
            return _tail(f.read(), lines)

src/proteus/inference/objective.py:549

• The helpfile read path still lets some unreadable files escape as worker exceptions. For example, invalid UTF-8 raises UnicodeDecodeError, which is not in this tuple, so it kills the worker instead of becoming the documented per-run ProteusRunFailure and failure record. Catch the remaining read/decode error class here (without swallowing setup errors outside this read).

    except (
        FileNotFoundError,
        OSError,
        pd.errors.EmptyDataError,
        pd.errors.ParserError,

src/proteus/inference/objective.py:697

• This raise only terminates the current BO worker. parallel_process launches independent Process objects and only inspects nonzero exit codes after joining them; it does not signal or terminate the siblings, so abort_on_failure = true still lets the other workers continue to consume their budgets and can return partial results. Propagate a shared abort event or terminate the remaining workers when one fails.

        if abort_on_failure():
            raise

src/proteus/inference/objective.py:737

• For cleanly exiting runs with a failed or excluded status, raw has already been mutated by run_proteus to include params.out.path and the worker overrides. Recording it directly therefore puts fixed implementation values into the swept-parameter columns, unlike the subprocess-failure path and the documented failure table. Filter _FIXED_PARAMETER_KEYS here before constructing the record so the report identifies only the sampled parameter values.

                parameters=raw,

src/proteus/inference/plot.py:568

• Filtering to directories still admits failed evaluation folders. run_proteus creates out_abs before launching the child, so a startup rejection or timeout can leave an i_* directory without init_coupler.toml; the loop below then calls toml.load(c / 'init_coupler.toml') and crashes during plotting instead of completing with the failure summary. Skip cases without the generated config before loading it.

    cases = sorted(p for p in (Path(directory) / 'workers').glob('w_*/i_*') if p.is_dir())

• Files reviewed: 18/19 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[nichollsh]

Thanks for this @stuitje. I've made some in-line comments below regarding the documentation and maintainability of the new code. Hope these are useful.

I ran the example inference config and it still seems to work well.

Another suggestion: it would be useful to include here an added option for sharing a spectral file between workers/runs. Maybe worth a separate PR - your call - but it would fit within the work here.

[nichollsh]

Why does behaviour differ on MacOS?

[stuitje]

I added this fix after a comment from Copilot in this thread. In multiprocessing in python, MacOS uses the 'spawn' start method by default, whereas Linux uses 'fork'. In the case of 'spawn', the child is fresh and doesn't inherit python objects such as handlers for the fwl logger.

I would force fork everywhere for simplicity but apparently fork is considered unsafe on Mac since python 3.8: https://bugs.python.org/issue33725

I could move everything to spawn since the logging fix is already there, or leave it like this. However, since I was planning on taking a new look at the whole multiprocessing part of the BO, I could also leave it for that PR. What do you think?

[nichollsh]

I think status=0 would be more appropriate here.

[stuitje]

I am not sure, as status= 0 already exists, meaning "Proteus started". STATUS_MISSING should, I would say, really catch a missing status, set to -1, since proteus never writes status -1.

[stuitje]

@nichollsh Thank you for the thorough review. I agree with most points. This was the first time I used Claude Code in my work and I agree with you that it sometimes produces spammy or opaque logs, and that it can add unnecessary lines of code. I will fix this with more rigour next time.

I have implemented most of your comments. One vital change from last version: everything to do with failure handling (except abort_on_failure) has been moved to failures.py and out of inference.py, utils.py and objective.py. The latter was getting long and I think this makes the code much clearer. I am aware this might complicate your review, and I apologise. However I believe it is the best option in the long run.

The few comments that I didn't implement immediately I replied to, perhaps you can take a quick look :)

Before I ask for a re-review, I am looking into the spectral files now, updating the docs, and I want to test the current code again by doing a few inference runs. Thanks again!

[nichollsh]

@stuitje please let me know when this is ready for another review. You'll need to update with changes from the main branch too.

[stuitje]

@stuitje please let me know when this is ready for another review. You'll need to update with changes from the main branch too.

Hi Harrison, I am aware main needs to be merged into this branch, I will do so when I am done. I could not finish the spectral files parts of the code yesterday.

When I do finish them I will of course let you know. This will probably be on Monday morning, after the weekend.

[stuitje]

@nichollsh, the tests are passing again, so this is ready for another review. I added the spectral file cache and exposed it as a config option: please let me know if you disagree; I can change the way it is handled.