From 0f5a602f838ef5624d37a3ef9540bbed26d12cbb Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Tue, 18 Aug 2026 06:11:23 +0200 Subject: [PATCH 1/2] Name the actual solver in retry-ladder exhaustion messages The CVODE wrapper can fail to import on a build or ABI mismatch, in which case aragog falls back to a scipy integrator without saying so. The retry-ladder exhaustion message hardcoded "CVODE status=..." regardless, so a run that silently fell back to scipy still reported CVODE failures, pointing anyone reading the log at the wrong solver. Add _active_solver_name(), which checks aragog's own _CVODE_AVAILABLE import-time flag instead of trusting the configured solver_method, and use it to build the exhaustion reason. It mirrors aragog's own Radau/BDF choice rather than collapsing both into a generic "scipy" label, and falls back to a safe label instead of crashing the coupled run if that private flag is ever renamed or removed upstream. Also generalizes the comment above the status==0 branch, which made the same CVODE-only assumption. --- src/proteus/interior_energetics/aragog.py | 38 ++++++++-- tests/interior_energetics/test_aragog.py | 86 +++++++++++++++++++++++ 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/proteus/interior_energetics/aragog.py b/src/proteus/interior_energetics/aragog.py index cbd00702a..44d47f541 100644 --- a/src/proteus/interior_energetics/aragog.py +++ b/src/proteus/interior_energetics/aragog.py @@ -1966,6 +1966,33 @@ def run_solver(self, hf_row, interior_o, dirs, write_data: bool = True): return sim_time, output + def _active_solver_name(self) -> str: + """Name the integrator that is actually running, not the one configured. + + ``solver_method`` can ask for CVODE and still run scipy: the wrapper + is compiled against SUNDIALS and falls back silently on a build or + ABI mismatch, so trusting the config name mislabels every failure + the fallback produces. Mirrors aragog's own Radau/BDF choice + (entropy_solver.py) so a scipy fallback names the integrator that + ran instead of a generic 'scipy'. + + Returns + ------- + str + 'CVODE' when the configured and available integrator is CVODE, + 'BDF' when ``solver_method='bdf'``, 'Radau' otherwise (including + a CVODE fallback, which aragog also resolves to Radau). + """ + try: + from aragog.solver.entropy_solver import _CVODE_AVAILABLE + except ImportError: + _CVODE_AVAILABLE = False + + method = str(self._config.interior_energetics.aragog.solver_method or '') + if method == 'cvode' and _CVODE_AVAILABLE: + return 'CVODE' + return 'BDF' if method == 'bdf' else 'Radau' + def _solve_with_retry(self, hf_row, interior_o) -> SolverOutput: """Run aragog_solver.solve() with a dt-halving retry ladder. @@ -2064,7 +2091,7 @@ def _solve_with_retry(self, hf_row, interior_o) -> SolverOutput: float(hf_row.get('Time', 0.0)), ) - # Status check: did CVODE accept the step? + # Status check: did the solver accept the step? if out.status == 0: # Sanity check: reject suspiciously large T_core jumps # that indicate the solver "succeeded" with garbage. @@ -2105,16 +2132,17 @@ def _solve_with_retry(self, hf_row, interior_o) -> SolverOutput: return out if attempt >= max_attempts: - # status==0 here means CVODE accepted every step but each - # result was rejected for an over-threshold T_core jump, so - # report that reason rather than the misleading status=0. + # status==0 here means the solver accepted every step but + # each result was rejected for an over-threshold T_core + # jump, so report that reason rather than the misleading + # status=0. if out.status == 0: reason = ( 'status=0 but the T_core jump exceeded the ' f'{sanity_dT_core:.0f} K sanity threshold on every attempt' ) else: - reason = f'CVODE status={out.status}' + reason = f'{self._active_solver_name()} status={out.status}' log.error( 'Aragog solver failed after %d attempts (%s). ' 'Raising RuntimeError so wrapper can apply skip-step fallback.', diff --git a/tests/interior_energetics/test_aragog.py b/tests/interior_energetics/test_aragog.py index 01870698e..ea43a5f73 100644 --- a/tests/interior_energetics/test_aragog.py +++ b/tests/interior_energetics/test_aragog.py @@ -760,3 +760,89 @@ def test_setup_or_update_solver_tracks_stale_structure_steps(): interior_o.structure_stale = True AragogRunner.setup_or_update_solver(config, hf_row, interior_o, 1.0, dirs) assert interior_o._stale_struct_steps == 1 + + +@pytest.mark.unit +def test_solve_with_retry_ladder_exhaustion_names_the_solver_that_actually_ran( + monkeypatch, +): + """A retry-ladder exhaustion names the integrator that actually ran. + + ``solver_method`` can ask for CVODE and still run scipy: the wrapper is + compiled against SUNDIALS and falls back silently on a build or ABI + mismatch, so trusting the config name mislabels every scipy-fallback + failure as a CVODE one. Covers CVODE available, CVODE unavailable + (silent fallback to Radau), and an explicitly configured 'bdf', so a + mutant that drops the solver_method check or collapses Radau/BDF into + one label fails at least one branch. + """ + from proteus.interior_energetics.aragog import AragogRunner + + module_path = 'aragog.solver.entropy_solver' + + def _build_runner(status, solver_method='cvode'): + runner = AragogRunner.__new__(AragogRunner) + runner._config = MagicMock() + runner._config.interior_energetics.aragog.solver_method = solver_method + runner._config.planet.mass_tot = 1.0 + + out = MagicMock() + out.status = status + out.T_core = 0.0 + + solver = MagicMock() + solver.parameters.solver.start_time = 0.0 + solver.parameters.solver.end_time = 1.0 + solver.get_current_dSdr_cmb.return_value = None + solver._dSdr_cmb_init = None + solver.get_state.return_value = out + runner.aragog_solver = solver + + interior_o = MagicMock() + interior_o._last_entropy = None + + hf_row = {'Time': 2.15e5, 'T_cmb': 0.0} + return runner, interior_o, hf_row + + monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', True) + cvode_runner, cvode_interior_o, cvode_hf_row = _build_runner(status=-1) + with pytest.raises(RuntimeError, match='CVODE status=-1') as cvode_info: + cvode_runner._solve_with_retry(cvode_hf_row, cvode_interior_o) + assert 'Radau status=' not in str(cvode_info.value) + assert 'BDF status=' not in str(cvode_info.value) + + monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', False) + fallback_runner, fallback_interior_o, fallback_hf_row = _build_runner(status=-1) + with pytest.raises(RuntimeError, match='Radau status=-1') as fallback_info: + fallback_runner._solve_with_retry(fallback_hf_row, fallback_interior_o) + assert 'CVODE status=' not in str(fallback_info.value) + assert 'BDF status=' not in str(fallback_info.value) + + monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', True) + bdf_runner, bdf_interior_o, bdf_hf_row = _build_runner(status=-1, solver_method='bdf') + with pytest.raises(RuntimeError, match='BDF status=-1') as bdf_info: + bdf_runner._solve_with_retry(bdf_hf_row, bdf_interior_o) + assert 'CVODE status=' not in str(bdf_info.value) + assert 'Radau status=' not in str(bdf_info.value) + + +@pytest.mark.unit +def test_active_solver_name_falls_back_when_cvode_flag_is_unimportable(monkeypatch): + """A missing/renamed aragog CVODE flag degrades to a label, not a crash. + + ``_active_solver_name()`` reaches into aragog's private + ``_CVODE_AVAILABLE`` flag. aragog is a separate, actively developed + package that owes that private name no stability guarantee; if it is + ever renamed or removed, the retry ladder's exhaustion path must still + raise the intended ``RuntimeError`` (which the wrapper catches to apply + a skip-step fallback) rather than an uncaught ``ImportError``. + """ + from proteus.interior_energetics.aragog import AragogRunner + + monkeypatch.delattr('aragog.solver.entropy_solver._CVODE_AVAILABLE') + + runner = AragogRunner.__new__(AragogRunner) + runner._config = MagicMock() + runner._config.interior_energetics.aragog.solver_method = 'cvode' + + assert runner._active_solver_name() == 'Radau' From d1c82589cb1d54875242621c38781b2515772e5d Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Sun, 23 Aug 2026 09:34:40 +0200 Subject: [PATCH 2/2] Strengthen retry-ladder solver-name tests Add an explicit 'radau' case alongside cvode/fallback/bdf, and assert solve() ran once per retry attempt in each case, so a mutant that breaks the retry loop itself fails alongside the solver label. Add a second assertion to the missing-flag fallback test covering the 'bdf' branch, which never consults the flag. --- tests/interior_energetics/test_aragog.py | 29 ++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/interior_energetics/test_aragog.py b/tests/interior_energetics/test_aragog.py index ea43a5f73..52a062a7c 100644 --- a/tests/interior_energetics/test_aragog.py +++ b/tests/interior_energetics/test_aragog.py @@ -772,9 +772,11 @@ def test_solve_with_retry_ladder_exhaustion_names_the_solver_that_actually_ran( compiled against SUNDIALS and falls back silently on a build or ABI mismatch, so trusting the config name mislabels every scipy-fallback failure as a CVODE one. Covers CVODE available, CVODE unavailable - (silent fallback to Radau), and an explicitly configured 'bdf', so a - mutant that drops the solver_method check or collapses Radau/BDF into - one label fails at least one branch. + (silent fallback to Radau), an explicit 'radau', and an explicit 'bdf', + so a mutant that drops the solver_method check or collapses Radau/BDF + into one label fails at least one branch. Each case also asserts + ``solve()`` ran once per attempt, so a mutant that breaks the retry loop + itself (wrong attempt count, early exit) fails alongside the label. """ from proteus.interior_energetics.aragog import AragogRunner @@ -804,12 +806,15 @@ def _build_runner(status, solver_method='cvode'): hf_row = {'Time': 2.15e5, 'T_cmb': 0.0} return runner, interior_o, hf_row + max_attempts = 6 + monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', True) cvode_runner, cvode_interior_o, cvode_hf_row = _build_runner(status=-1) with pytest.raises(RuntimeError, match='CVODE status=-1') as cvode_info: cvode_runner._solve_with_retry(cvode_hf_row, cvode_interior_o) assert 'Radau status=' not in str(cvode_info.value) assert 'BDF status=' not in str(cvode_info.value) + assert cvode_runner.aragog_solver.solve.call_count == max_attempts monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', False) fallback_runner, fallback_interior_o, fallback_hf_row = _build_runner(status=-1) @@ -817,6 +822,17 @@ def _build_runner(status, solver_method='cvode'): fallback_runner._solve_with_retry(fallback_hf_row, fallback_interior_o) assert 'CVODE status=' not in str(fallback_info.value) assert 'BDF status=' not in str(fallback_info.value) + assert fallback_runner.aragog_solver.solve.call_count == max_attempts + + monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', True) + radau_runner, radau_interior_o, radau_hf_row = _build_runner( + status=-1, solver_method='radau' + ) + with pytest.raises(RuntimeError, match='Radau status=-1') as radau_info: + radau_runner._solve_with_retry(radau_hf_row, radau_interior_o) + assert 'CVODE status=' not in str(radau_info.value) + assert 'BDF status=' not in str(radau_info.value) + assert radau_runner.aragog_solver.solve.call_count == max_attempts monkeypatch.setattr(f'{module_path}._CVODE_AVAILABLE', True) bdf_runner, bdf_interior_o, bdf_hf_row = _build_runner(status=-1, solver_method='bdf') @@ -824,6 +840,7 @@ def _build_runner(status, solver_method='cvode'): bdf_runner._solve_with_retry(bdf_hf_row, bdf_interior_o) assert 'CVODE status=' not in str(bdf_info.value) assert 'Radau status=' not in str(bdf_info.value) + assert bdf_runner.aragog_solver.solve.call_count == max_attempts @pytest.mark.unit @@ -844,5 +861,9 @@ def test_active_solver_name_falls_back_when_cvode_flag_is_unimportable(monkeypat runner = AragogRunner.__new__(AragogRunner) runner._config = MagicMock() runner._config.interior_energetics.aragog.solver_method = 'cvode' - assert runner._active_solver_name() == 'Radau' + + # The missing flag must not leak into or corrupt the 'bdf' branch, + # which never consults _CVODE_AVAILABLE in the first place. + runner._config.interior_energetics.aragog.solver_method = 'bdf' + assert runner._active_solver_name() == 'BDF'