BayesOpt scheme, AGNI grey gas, improved test coverage

[nichollsh]

BayesOpt changes

• BO scheme now uses logging to print information
• Improving the BO scheme plots and updates its config parser
• BO scheme now writes CSV rather than Pickle files, which makes it easier to debug and improves security (I believe that pickle can be insecure).
• New script in tools/ for extracting truth values from baseline PROTEUS runs, for BO retrieval
• Added tests for BO scheme, which increases its coverage to ~90%
• Removed notebook demo from Ben, and removed unused config files

Timestepping changes

• Updated time-stepping to ensure simulations do not overshoot final time - closes PROTEUS overshoots max-time stopping criterion #676.
• Corrected methods which parse the output of Robb's boundary-layer model, so that they respect the prevent_warming config flag. This improves model stability and performance substantially, by damping oscillations.

AGNI grey gas

• Support for AGNI's grey gas RT scheme - closes Option for grey-gas radiative transfer #674
• Added more tests for AGNI wrapper.
• Emission and visualisation plot routines updated to avoid crash when nband=1.
• Added tests for these plot functions

Moved hard-coded config parameters in PROTEUS

• Moved some time-stepping parameters into config,
• Also, moved the CALLIOPE and Interior-structure solver tolerances into the config. I changed these in this PR because the structure solver using the BL interior module factors into the BO paper I am preparing.

Validation of changes

• Added new tests, which pass on my computer (Fedora 44, Python 3.12) and on GitHub
• Checked that standard PROTEUS configs still run as expected
• BayesOpt scheme successfully retrieves ground-truth parameters (for purposes of paper)

A nice bonus from this PR is that the total PROTEUS coverage is increased by 9 percentage points, bringing it to 78.50%. We are almost at the 80% target.

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

Relevant people

@timlichtenberg

[nichollsh]

@timlichtenberg this branch has developed substantially enough that it should be merged (ideally) next week. I will continue development iteratively after that.

However, I need to tidy it up a bit before it's ready for human-review.

I will let you know when this is, but hopefully before the PROTEUS meeting tomorrow (4 May).

[Copilot]

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 4 comments.

[nichollsh]

@timlichtenberg I believe that this is now ready for its first human-review.

[timlichtenberg]

Thanks for bundling this up, Harrison. The BO refactor and AGNI grey-gas plumbing read cleanly. Two issues I'd like to resolve before approving.

---

1. prevent_warming semantics change in F_int handling

src/proteus/interior/spider.py removes the old floor:

# REMOVED from ReadSPIDER:
if config.atmos_clim.prevent_warming:
    output['F_int'] = max(1.0e-8, output['F_int'])

src/proteus/interior/wrapper.py:402 replaces it with a ratchet that also covers T_surf and F_int:

if config.atmos_clim.prevent_warming and (interior_o.ic == 2):
    hf_row['Phi_global'] = min(hf_row['Phi_global'], Phi_global_prev)
    hf_row['T_magma']    = min(hf_row['T_magma'],   T_magma_prev)
    hf_row['T_surf']     = min(hf_row['T_surf'],    T_surf_prev)
    hf_row['F_int']      = min(hf_row['F_int'],     hf_all.iloc[-1]['F_int'])

These are not equivalent on F_int.

• Old: F_int floored at +1e-8 under prevent_warming (no negatives, no monotonicity requirement).
• New: F_int monotonically non-increasing across iterations, including into negatives. T_surf and Phi also monotonically non-increasing.

F_int can now go negative under prevent_warming. A single noisy iteration where the interior is briefly cooler than the atmosphere produces F_int < 0, after which the ratchet pins F_int at that negative value indefinitely (since min(later, negative) = negative). The old floor explicitly prevented this. The PR description says the BL parser was "corrected to respect the prevent_warming flag" — but the new ratchet semantics are stronger than what the old SPIDER floor enforced. Was the floor's removal intentional, or an incidental consequence of moving the logic from ReadSPIDER into run_interior?

If the intent is to keep both behaviours, the simplest fix is to combine them, e.g.:

hf_row['F_int'] = max(1.0e-8, min(hf_row['F_int'], hf_all.iloc[-1]['F_int']))

There is also no unit test exercising the new T_surf or F_int clamp on either interior module (tests/interior/test_timestep.py covers the timestep machinery only). At least one regression test pinning expected behaviour at the clamp boundary would help future reviewers.

2. Element-mass-ratio derivation returns 0.0 for both numerator-zero and denominator-zero

src/proteus/outgas/wrapper.py:94:

if min(hf_row[f'{e1}_kg_atm'], hf_row[f'{e2}_kg_atm']) < 1e-30:
    hf_row[key] = 0.0
else:
    hf_row[key] = hf_row[f'{e2}_kg_atm'] / hf_row[f'{e1}_kg_atm']

Numerator-zero → 0.0 is correct (no e2, ratio is zero). Denominator-zero → 0.0 is wrong: the ratio is undefined / +inf, and a silent zero will mislead any downstream plot or BO objective that ingests these columns (the BO objective treats _atm ratios as observables). Suggest np.nan for the denominator-zero branch and 0.0 for the numerator-zero branch; the ratio symmetry is already handled by GetHelpfileKeys(). Also consider promoting the 1e-30 floor to a named constant alongside the other mass thresholds.

[nichollsh]

Thanks for this @timlichtenberg.

Regarding point 1... I have corrected the F_int limiting routine which is enabled when prevent_warming=True, by following your suggestion above. You can see this in the most recent commit. I have also added more unit/smoke tests using Copilot, which test for this limiter behaviour. I have checked that these tests make sense, and they pass on my computer.

Regarding point 2... The behaviour you've highlighted is what I had intended. It isn't a mistake, because returning X/Y element ratios=0 is probably safer than returning NaN values. Dividing by zero isn't a defined operation, and can reasonably be evaluated to =0 in this case. It works appropriately for the BO scheme.