From cfa967ec428114e08231b81206317d7b7c149699 Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 17:31:08 +0000 Subject: [PATCH 1/4] Restore the accepted state when a native Adams source fails A native Adams trial swapped its prediction into the state around its source evaluation (elementwise and fused trials) or wrote it there and copied the accepted state back afterwards (coupled trial and startup). A source that threw skipped the restore, so the failed run's terminal checkpoint and march snapshot held the prediction. The swap is now a scope guard, and adams_evaluate restores the state before rethrowing. A test hook, adams_march_fault("trial_source", ...), raises inside the source evaluation; the regression test compares its checkpoint and snapshot with a fault before the prediction on all three paths. Closes #129 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- .../native-adams-source-failure.fixed.md | 3 + native/include/solprop_native/core.hpp | 4 ++ native/include/solprop_native/engines.hpp | 5 +- native/src/bindings.cpp | 12 +++- native/src/engines.cpp | 19 ++++-- native/src/general/kernels.hpp | 19 ++++++ native/src/general/state.cpp | 4 ++ native/src/general/trial.cpp | 30 ++++++---- native/src/general/workspace.hpp | 6 ++ tests/driver/test_native_march_lifecycle.py | 59 ++++++++++++++++++- 10 files changed, 141 insertions(+), 20 deletions(-) create mode 100644 changelog.d/native-adams-source-failure.fixed.md diff --git a/changelog.d/native-adams-source-failure.fixed.md b/changelog.d/native-adams-source-failure.fixed.md new file mode 100644 index 000000000..2c6a7a51d --- /dev/null +++ b/changelog.d/native-adams-source-failure.fixed.md @@ -0,0 +1,3 @@ +Fixed a native Adams trial whose source evaluation failed leaving its prediction in the +state, so that the failed run's terminal checkpoint and march snapshot held the +prediction instead of the last accepted step (#129). diff --git a/native/include/solprop_native/core.hpp b/native/include/solprop_native/core.hpp index 35b40eef4..54a38ac62 100644 --- a/native/include/solprop_native/core.hpp +++ b/native/include/solprop_native/core.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1174,6 +1175,9 @@ class GeneralState { [[nodiscard]] HostSource* host_source() const noexcept; /** The next host evaluation's position, m, and purpose, which the engine sets. */ void host_evaluation(double z, HostPurpose purpose) noexcept; + /** A test hook (solprop issue #129): `fault` runs, once, as the next Adams source + evaluation starts, a trial's with its prediction in the state; empty disarms. */ + void arm_source_fault(std::function fault) noexcept; private: /** `function(workspace, arguments...)` inside the workspace's CPU team scope diff --git a/native/include/solprop_native/engines.hpp b/native/include/solprop_native/engines.hpp index 9be46f68c..6e626e3ae 100644 --- a/native/include/solprop_native/engines.hpp +++ b/native/include/solprop_native/engines.hpp @@ -34,12 +34,15 @@ namespace solprop_native::engines { /** A failure injected at one of the resident engine's stages, after that stage has run `after` more times: solprop's lifecycle tests fail the native march with it. With `signal`, that signal is raised in the native call instead and the stage goes on, - as a signal arriving while native code holds no interpreter lock does. */ + as a signal arriving while native code holds no interpreter lock does. With + `in_source`, the failure is raised inside the stage's source evaluation instead, + a trial's with its prediction in the state (solprop issue #129). */ struct MarchFault { march::Stage stage = march::Stage::none; std::size_t after = 0; march::Category category = march::Category::backend; int signal = 0; + bool in_source = false; }; /** A scalar carrier frame's shift and rotations (solprop SEP 35), packed by solprop's diff --git a/native/src/bindings.cpp b/native/src/bindings.cpp index 07c52b72b..ff95fde9f 100644 --- a/native/src/bindings.cpp +++ b/native/src/bindings.cpp @@ -2073,8 +2073,13 @@ class BoundState { fault.stage = march::Stage::trial; else if (stage == "accept") fault.stage = march::Stage::accept; - else - throw std::invalid_argument("a march fault's stage is startup, trial or accept"); + else if (stage == "trial_source") { + // Inside the trial's source evaluation, its prediction in the state. + fault.stage = march::Stage::trial; + fault.in_source = true; + } else + throw std::invalid_argument( + "a march fault's stage is startup, trial, trial_source or accept"); if (category == "convergence") fault.category = march::Category::convergence; else if (category == "value") @@ -3850,7 +3855,8 @@ NB_MODULE(_core, module_) { "partials"_a.noconvert(), "rtol"_a, "atol"_a) .def("flow_guard_configure", &BoundState::flow_guard_configure, "row_sum"_a, "condition_bound"_a, "taylor_scale"_a, "max_squarings"_a, "allowance"_a) - // A test hook: fail the resident engine's `stage` after `after` more calls. + // A test hook: fail the resident engine's `stage` after `after` more calls; + // `trial_source` fails inside the trial's source evaluation. .def("adams_march_fault", &BoundState::march_fault, "stage"_a, "after"_a, "category"_a) .def( diff --git a/native/src/engines.cpp b/native/src/engines.cpp index 788e7e5f6..74676d510 100644 --- a/native/src/engines.cpp +++ b/native/src/engines.cpp @@ -160,17 +160,26 @@ void ResidentEngine::inject(const march::Stage stage) { } const auto category = fault_->category; const int signal = fault_->signal; + const bool in_source = fault_->in_source; fault_.reset(); if (signal != 0) { // Python's handler only notes it: it runs once the call returns to Python. static_cast(std::raise(signal)); return; } - if (category == march::Category::convergence) - throw EnvelopeConvergenceError("an injected native failure"); - if (category == march::Category::value) - throw std::invalid_argument("an injected native failure"); - throw std::runtime_error("an injected native failure"); + const auto raise = [category] { + if (category == march::Category::convergence) + throw EnvelopeConvergenceError("an injected native failure"); + if (category == march::Category::value) + throw std::invalid_argument("an injected native failure"); + throw std::runtime_error("an injected native failure"); + }; + if (in_source) { + // Raised by the stage's own source evaluation, inside its swap. + state_.arm_source_fault(raise); + return; + } + raise(); } ResidentRungeKutta::ResidentRungeKutta(GeneralState& state, Trial trial, diff --git a/native/src/general/kernels.hpp b/native/src/general/kernels.hpp index 28d35701b..7e46e5ea7 100644 --- a/native/src/general/kernels.hpp +++ b/native/src/general/kernels.hpp @@ -132,6 +132,25 @@ class FlowAside { std::unique_ptr held_; }; +// Swaps two views for a scope and back however it is left: an Adams trial's +// prediction stands in the state for its source evaluation alone, so that a failure +// there leaves the accepted state in place (solprop issue #129). +template class SwappedAside { +public: + SwappedAside(View& first, View& second) : first_(first), second_(second) { + std::swap(first_, second_); + } + ~SwappedAside() { std::swap(first_, second_); } + SwappedAside(const SwappedAside&) = delete; + SwappedAside& operator=(const SwappedAside&) = delete; + SwappedAside(SwappedAside&&) = delete; + SwappedAside& operator=(SwappedAside&&) = delete; + +private: + View& first_; + View& second_; +}; + // One frozen-residual weight row from the host into a View's row. template void upload_weights(const ExecutionSpace& space, const Row& out, diff --git a/native/src/general/state.cpp b/native/src/general/state.cpp index 2fc61bfcc..424aa8e02 100644 --- a/native/src/general/state.cpp +++ b/native/src/general/state.cpp @@ -464,6 +464,10 @@ void GeneralState::bind_host_source(std::shared_ptr source) { HostSource* GeneralState::host_source() const noexcept { return workspace_ ? workspace_->host_source() : nullptr; } +void GeneralState::arm_source_fault(std::function fault) noexcept { + if (workspace_) + workspace_->arm_source_fault(std::move(fault)); +} void GeneralState::host_evaluation(const double z, const HostPurpose purpose) noexcept { if (workspace_) workspace_->host_evaluation(z, purpose); diff --git a/native/src/general/trial.cpp b/native/src/general/trial.cpp index d2e0cfd1e..d5f215b4c 100644 --- a/native/src/general/trial.cpp +++ b/native/src/general/trial.cpp @@ -1685,6 +1685,8 @@ void GeneralWorkspace::adams_phase() { // The stage-zero source at an anchor-frame field held in current_, as adams_source // evaluates it, into stages_(0). void GeneralWorkspace::adams_evaluate_source(const TrialScalars& scalars) { + if (source_fault_) + std::exchange(source_fault_, {})(); if (general_) general_->set_weights(scalars.transform_weights); if (problem_.raman.enabled) @@ -1697,9 +1699,15 @@ void GeneralWorkspace::adams_evaluate_source(const TrialScalars& scalars) { has_recycled_ = false; } -// The same, with current_ restored from adams_saved_ afterwards. +// The same, with current_ restored from adams_saved_ afterwards, a failure's too: the +// accepted state stands where the source failed (solprop issue #129). void GeneralWorkspace::adams_evaluate(const TrialScalars& scalars) { - adams_evaluate_source(scalars); + try { + adams_evaluate_source(scalars); + } catch (...) { + graph::deep_copy(execution_space_, current_, adams_saved_); + throw; + } graph::deep_copy(execution_space_, current_, adams_saved_); } @@ -1891,9 +1899,10 @@ GeneralWorkspace::adams_fused_trial(const double dz, const AdamsWeights& weights total += weights.predictor[t] * store(weights.slot[t], column, index); predicted(column, index) = current(column, index) + dz * total; }); - std::swap(current_, adams_saved_); - adams_evaluate_source(scalars); - std::swap(current_, adams_saved_); + { + const SwappedAside aside(current_, adams_saved_); + adams_evaluate_source(scalars); + } const auto stages = stages_; const auto source = adams_source_; const auto propagator = adams_propagator_; @@ -3715,11 +3724,12 @@ std::vector GeneralWorkspace::adams_elementwise_trial( launch::parallel_reduce("solprop_native.adams.exponential_predict", tasks, predict, unused); } - std::swap(current_, adams_saved_); - use_uploaded_sample(sample); - // On a device the source's launches replay as one graph while they repeat. - adams_source_graph_.run(execution_space_, [&] { adams_evaluate_source(sample); }); - std::swap(current_, adams_saved_); + { + const SwappedAside aside(current_, adams_saved_); + use_uploaded_sample(sample); + // On a device the source's launches replay as one graph while they repeat. + adams_source_graph_.run(execution_space_, [&] { adams_evaluate_source(sample); }); + } const ExponentialCorrection reducer{.value_count = weights.n_rows, .store = store, .moments = moments, diff --git a/native/src/general/workspace.hpp b/native/src/general/workspace.hpp index 274f04668..5c9b38985 100644 --- a/native/src/general/workspace.hpp +++ b/native/src/general/workspace.hpp @@ -190,6 +190,9 @@ class GeneralWorkspace { host_z_ = z; host_purpose_ = purpose; } + void arm_source_fault(std::function fault) noexcept { + source_fault_ = std::move(fault); + } /** The team a compact problem's FFTW loops take (`CpuTeamScope`): its pools' widest instance, which every threaded kernel and transform of it uses. Zero for any other problem: the runtime's team. */ @@ -750,6 +753,9 @@ class GeneralWorkspace { ComplexView2 host_input_, host_spectrum_; double host_z_ = 0.0; HostPurpose host_purpose_ = HostPurpose::startup; + // A test hook (solprop issue #129): run, once, as the next Adams source evaluation + // starts. + std::function source_fault_; ComplexView2 current_, advanced_, solution_, error_, residual_, recycled_, noise_; // solprop SEP 24 phase 3: the low-storage march's registers B and C (the state is // A) and its flows. diff --git a/tests/driver/test_native_march_lifecycle.py b/tests/driver/test_native_march_lifecycle.py index e6c404112..a7a320b36 100644 --- a/tests/driver/test_native_march_lifecycle.py +++ b/tests/driver/test_native_march_lifecycle.py @@ -24,11 +24,18 @@ import threading from typing import Any, cast +import numpy as np import pytest from driver.test_interrupts import AfterStep, needs_default_sigterm, planes_of from driver.test_resume import CrashError -from driver.test_resume_history import RTOL, assert_accurate_run, soliton, tight_run +from driver.test_resume_history import ( + RTOL, + assert_accurate_run, + coupled, + soliton, + tight_run, +) from march_bounds import TIGHTER from solprop.backends import available, sessions from solprop.backends.session import DEFAULT_WORK_LIMIT @@ -120,6 +127,56 @@ def test_a_native_failure_raises_its_class_at_the_last_accepted_step( assert state.march.z == state.outcome.z # pyright: ignore[reportOptionalMemberAccess] +SOURCE_PATHS = { + "elementwise": (lambda build_simulation: soliton(build_simulation, "native"), 30), + "fused": ( + lambda build_simulation: soliton( + build_simulation, "native", integrator=Multistep(formulation="interaction") + ), + 30, + ), + "coupled": (lambda _: coupled("native"), 2), +} +"""The native trial's three source evaluations -- the exponential march's elementwise +trial, the interaction formulation's fused trial and a coherent wall's coupled flow -- +each with the trials before its failure, within its run.""" + + +# The coherent wall's warnings, as the resume tests that run it take them. +@pytest.mark.filterwarnings("ignore:the temporal absorber attenuates a packet") +@pytest.mark.filterwarnings("ignore::solprop.exceptions.GrazingIncidenceWarning") +@pytest.mark.filterwarnings("ignore::solprop.exceptions.MaterialRangeWarning") +@pytest.mark.parametrize("path", SOURCE_PATHS) +def test_a_failure_inside_a_trial_s_source_leaves_the_accepted_state( + build_simulation, tmp_path, monkeypatch, path +): + # Issue #129: a trial evaluates its source with its prediction standing in the + # state. A failure there must leave the accepted state, checkpointed and + # snapshotted, as the same failure before the prediction does: the two runs make + # the same attempts, and a prediction left behind differs by a step's change. + make, after = SOURCE_PATHS[path] + build = make(build_simulation) + stops = {} + for stage in ("trial", "trial_source"): + part = tmp_path / f"{stage}.h5" + with monkeypatch.context() as patch: + with_fault(patch, stage, after, "backend") + with pytest.raises(RuntimeError, match="an injected native failure"): + build().run(part, progress="none") + assert_consistent_stop(part) + stops[stage] = resume(part) + before, inside = stops["trial"], stops["trial_source"] + assert before.checkpoint is not None and inside.checkpoint is not None + assert before.march is not None and inside.march is not None + assert inside.checkpoint.plane.z == before.checkpoint.plane.z > 0.0 + for found, accepted in ( + (inside.checkpoint.plane.field, before.checkpoint.plane.field), + (inside.march.state, before.march.state), + ): + scale = np.linalg.norm(accepted) + assert np.linalg.norm(np.asarray(found) - accepted) <= 1e-12 * scale + + @pytest.mark.medium def test_a_run_stopped_by_a_native_failure_continues_from_its_checkpoint( build_simulation, tmp_path, monkeypatch, caplog From 8281643f47d5bb4c13c5fd4907cfafcf4d4df172 Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 17:31:08 +0000 Subject: [PATCH 2/4] Record a rejected Adams attempt before raising its floor When a rejection's shrunk proposal fell below dz_min, both the NumPy reference and the native march raised the floor before recording the completed trial, so it had no attempt row. The floor is now deferred, as an accepted step's is, and raised after the row is recorded; the proposal, accepted state, ledger and message are unchanged. A replay still raises it at once. On native, a failed release of that attempt stays a note on the floor's error, as on the reference. The pre-refactor trace corpus is not regenerated: floor_after_rejections joins the CHANGED scenarios with an explicit test of the one added row. Closes #651 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- .../adams-rejection-floor-row.fixed.md | 4 ++ docs/dev/kernel-contract.md | 5 ++- native/src/adams_march.cpp | 31 +++++++++++---- src/solprop/backends/host/adams.py | 26 +++++++++---- tests/backends/test_adams_traces.py | 30 +++++++++++++++ tests/backends/test_native_march_traces.py | 38 ++++++++++++++++++- tests/backends/test_tdse_native.py | 5 ++- 7 files changed, 119 insertions(+), 20 deletions(-) create mode 100644 changelog.d/adams-rejection-floor-row.fixed.md diff --git a/changelog.d/adams-rejection-floor-row.fixed.md b/changelog.d/adams-rejection-floor-row.fixed.md new file mode 100644 index 000000000..92d567e9d --- /dev/null +++ b/changelog.d/adams-rejection-floor-row.fixed.md @@ -0,0 +1,4 @@ +Fixed the Adams march dropping the attempt row of a rejected trial whose reduced +proposal fell below `dz_min`: the completed trial is now recorded, on both the NumPy +reference and the native backend, before the floor's `ConvergenceError` is raised; the +proposal, accepted state and call ledger are unchanged (#651). diff --git a/docs/dev/kernel-contract.md b/docs/dev/kernel-contract.md index c9c464d05..34b014ac4 100644 --- a/docs/dev/kernel-contract.md +++ b/docs/dev/kernel-contract.md @@ -1290,7 +1290,10 @@ ledger. A resume carries all of it exactly (`MarchSnapshot`). An acceptance commits the engine's state, the history, the ledger, the stored order, the position and a kink restart together. Only then is the next proposal tested against the -floor. A floor so found is raised after the attempt is recorded. +floor. A floor so found is raised after the attempt is recorded. A rejection's shrunk +proposal is tested in the same way, and a floor it falls below leaves the proposal as +it was and is raised after the rejected attempt is recorded (issue #651); a replay +records no rejected attempt and raises it at once. **What a native call takes**: the policy, once per run and again when the monitor's interval changes (`MarchPolicy`, every field of the record), with a replay's diff --git a/native/src/adams_march.cpp b/native/src/adams_march.cpp index 1cb5ea808..104947a07 100644 --- a/native/src/adams_march.cpp +++ b/native/src/adams_march.cpp @@ -573,16 +573,16 @@ Result run(const Policy& policy, State& state, Engine& engine, const Bounds& bou ++state.ledger.failure_restarts; } const double next = proposal(policy, step.dz, outcome.factor); - if (next < policy.dz_min) { - // `limit_step` raises inside the rejection, before the attempt is recorded. - result.reason = Reason::floor; - result.floor_proposal = next; - return result; - } - state.proposal = next; + const bool floor = next < policy.dz_min; if (bounds.replay) { // A replay cannot reject: the field has diverged (`replay_step`), and the - // attempt is not recorded. + // attempt is not recorded; a floor is raised first, inside the rejection. + if (floor) { + result.reason = Reason::floor; + result.floor_proposal = next; + return result; + } + state.proposal = next; result.reason = Reason::diverged; result.replay_dz = step.dz; result.replay_end = step.end; @@ -590,6 +590,21 @@ Result run(const Policy& policy, State& state, Engine& engine, const Bounds& bou return result; } record(step.end, step.dz, norm, false, trial.order); + if (floor) { + // A floor leaves the proposal and is the reason the march stops once the + // completed attempt is recorded (`HostAdamsMarch._reject`, solprop issue + // #651). The reference raises it inside its trial's scope, so a failure of + // the attempt's release is the host's to keep beside the floor, as an + // abandoned attempt's is, never in its place. + try { + attempt.end(HostOutcome::rejected); + } catch (...) { // NOLINT(bugprone-empty-catch) - kept by the host, see above + } + result.reason = Reason::floor; + result.floor_proposal = next; + return result; + } + state.proposal = next; try { attempt.end(HostOutcome::rejected); } catch (const std::exception& error) { diff --git a/src/solprop/backends/host/adams.py b/src/solprop/backends/host/adams.py index 876fda5ce..a70c1cb77 100644 --- a/src/solprop/backends/host/adams.py +++ b/src/solprop/backends/host/adams.py @@ -141,8 +141,8 @@ def __init__( # Whether the stages left a quiet collar out at the last attempt: the step # after it returns is held to the collar's stable step (`_collar_reentry`). self._collar_was_quiet = False - # A floor an accepted step's next proposal fell below, raised once the step is - # recorded (`_decide`, `_raise_floor`). + # A floor an attempt's next proposal fell below, raised once the attempt is + # recorded (`_propose`, `_reject`, `_raise_floor`). self._floor: ConvergenceError | None = None def reset(self) -> None: @@ -619,6 +619,10 @@ def _decide( if not norm <= 1.0 and not (fixed and math.isfinite(norm)): self.ledger.rejected += 1 self._reject(dz, norm, order) + if fixed: + # A replay records no rejected attempt (`replay_step`): its floor is + # raised at once. + self._raise_floor() return False, norm z = backend._z keep = self._history[-policy.order :] if policy.order else [] @@ -669,9 +673,9 @@ def _propose(self, dz: float, factor: float) -> None: self._floor = error def _raise_floor(self) -> None: - """Raise the floor an accepted step's proposal fell below, once it is recorded. + """Raise the floor an attempt's proposal fell below, once it is recorded. - Every caller that records an accepted step calls this straight after, and every + Every caller that records an attempt calls this straight after, and every attempt calls it before it starts (:meth:`_afford`), so a floor is never passed over. """ @@ -769,6 +773,11 @@ def _reject(self, dz: float, norm: float, order: int) -> None: :data:`~solprop.stepper.multistep.RESTART_AFTER` in a row the method restarts at order one from the accepted state, paying one startup call. + A proposal below the floor is left as it was, and the floor is raised once the + march has recorded the rejected attempt (:meth:`_raise_floor`), so that the + completed trial has its row (issue #651), as an accepted step's floor is + (:meth:`_propose`). + - ``dz``: the rejected step, m. - ``norm``: its scaled defect, possibly not finite. """ @@ -782,9 +791,12 @@ def _reject(self, dz: float, norm: float, order: int) -> None: if outcome.restart: self._history = [] self.ledger.failure_restarts += 1 - self._backend._proposal = limit_step( - dz, outcome.factor, policy.dz_min, policy.dz_max - ) + try: + self._backend._proposal = limit_step( + dz, outcome.factor, policy.dz_min, policy.dz_max + ) + except ConvergenceError as error: + self._floor = error _ENGINE_MEMBERS = ( diff --git a/tests/backends/test_adams_traces.py b/tests/backends/test_adams_traces.py index 00d9a05e5..020ae1d66 100644 --- a/tests/backends/test_adams_traces.py +++ b/tests/backends/test_adams_traces.py @@ -49,6 +49,10 @@ def _corpus() -> list[dict[str, Any]]: # next proposal falls below the floor is recorded, and leaves the position at its # end, before the floor is raised. The oracle keeps the march as it was. "floor_short_landing", + # Issue #651: a rejection whose shrunk proposal falls below the floor is recorded + # before the floor is raised, as an accepted step's is. The corpus keeps the + # historical omission; it is not regenerated. + "floor_after_rejections", } @@ -91,3 +95,29 @@ def test_a_floor_after_an_accepted_landing_is_raised_once_the_step_is_committed( expected = {**oracle["end"], "in_flight": {}, "position": plane} assert trace["end"] == expected assert oracle["end"]["position"] == flight["z"] != plane + + +def test_a_floor_after_a_rejection_is_raised_once_the_attempt_is_recorded(): + # Issue #651: the march's last trial is rejected and its shrunk proposal is below + # the floor. Before the fix the floor was raised inside the rejection and the + # completed trial left no row; now it has one, and everything else -- the + # proposal, the history, the ledger, the message -- is the corpus's. + (entry,) = [ + one for one in ENTRIES if one["scenario"]["name"] == "floor_after_rejections" + ] + scenario = Scenario.from_json(entry["scenario"]) + oracle = entry["trace"] + assert oracle["end"]["stop"] == "ConvergenceError" + trace = run_scenario(scenario) + flight = oracle["end"]["in_flight"] + state = {key: value for key, value in trace["end"].items() if key in STATE} + row = { + **flight, + "end": flight["z"] + flight["dz"], + "step": flight["dz"], + "norm": scenario.norms[-1][0], + "accepted": False, + **state, + } + assert trace["events"] == [*oracle["events"], row] + assert trace["end"] == {**oracle["end"], "in_flight": {}} diff --git a/tests/backends/test_native_march_traces.py b/tests/backends/test_native_march_traces.py index 6b9547bc3..021d1796d 100644 --- a/tests/backends/test_native_march_traces.py +++ b/tests/backends/test_native_march_traces.py @@ -47,8 +47,9 @@ CORPUS = Path(__file__).parents[1] / "data" / "adams_traces.json.gz" -CHANGED = {"floor_short_landing"} -"""Scenarios phase 0's commit-order fix changed; the fixed march is the reference.""" +CHANGED = {"floor_short_landing", "floor_after_rejections"} +"""Scenarios a later fix changed -- phase 0's commit order, and the recorded rejection +before a floor (issue #651); the fixed march is the reference.""" def _corpus() -> list[dict[str, Any]]: @@ -217,3 +218,36 @@ def attempts(trace): assert attempts(found) == attempts(reference) assert found["end"] == reference["end"] + + +def test_a_rejection_below_the_floor_records_its_attempt_in_both_marches(): + # Issue #651: one order-one trial at z = 0 of 0.25 m, rejected with norm four, and + # its shrunk proposal (0.1 m) below the 0.2 m floor. Each march records the + # completed attempt once, then raises the floor with the accepted state, the + # proposal and the ledger as they were. + case = Scenario( + name="rejected_floor_accounting", + method={"order": 1, "variable_order": False}, + z0=0.0, + proposal=0.25, + dz_min=0.2, + dz_max=1.0, + z_end=1.0, + norms=[[4.0, 4.0, 4.0]], + ) + reference = run_scenario(case) + for trace in (reference, run_native(case, max_attempts=1)): + rows = [row for row in trace["events"] if "accepted" in row] + assert [ + (row["end"], row["step"], row["norm"], row["accepted"], row["order"]) + for row in rows + ] == [(0.25, 0.25, 4.0, False, 1)] + end = trace["end"] + assert end["stop"] == "ConvergenceError" + assert "below the floor dz_min = 2.000e-01 m" in end["message"] + assert (end["position"], end["proposal"]) == (0.0, 0.25) + assert end["history"] == [[0.0, "accepted", 0]] + # startup, predictor, rejected, accepted and rejected steps (`LEDGER`). + assert end["ledger"][:5] == [1, 1, 1, 0, 1] + assert end["in_flight"] == {} + assert run_native(case, max_attempts=1) == reference diff --git a/tests/backends/test_tdse_native.py b/tests/backends/test_tdse_native.py index aaadfa35c..0d978a4a3 100644 --- a/tests/backends/test_tdse_native.py +++ b/tests/backends/test_tdse_native.py @@ -517,8 +517,9 @@ def test_a_failed_evaluation_keeps_the_last_accepted_state(binding_class): def test_an_abandoned_attempt_s_failed_release_is_a_note_on_the_run_s_error(): - """Section 6.3: the step-size floor abandons the first attempt, whose release - fails; that failure rides on the floor's error as a note, on both.""" + """Section 6.3: the step-size floor follows the first attempt, recorded as + rejected (issue #651), whose release fails; that failure rides on the floor's + error as a note, on both.""" case = _case(length=1e-2, dz_max=1e-4, dz_init=1e-4, dz_min=9e-5, absorber=None) runs = _pair(case, ReleaseFails) errors = [runs[name][0].terminal.error for name in ("numpy_ref", "native")] From 0bde86eed667a157a677032b6aa6bd94883d8084 Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 18:02:27 +0000 Subject: [PATCH 3/4] native: keep the Adams march within the function-size limit Fold the two floor exits into one helper and merge the rejection's replay and floor branches, so run() stays under clang-tidy's 300-line threshold. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- native/src/adams_march.cpp | 47 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/native/src/adams_march.cpp b/native/src/adams_march.cpp index 104947a07..9010bf4e3 100644 --- a/native/src/adams_march.cpp +++ b/native/src/adams_march.cpp @@ -372,6 +372,13 @@ bool reached(const double position, const double target, const int ulps) { namespace { +/** End the march at the step floor that the proposal `next` fell below. */ +Result& floored(Result& result, const double next) { + result.reason = Reason::floor; + result.floor_proposal = next; + return result; +} + /** Record a native failure's stage, its solprop error class and its message. */ void fail(Result& result, const Stage stage, const std::exception& error) { result.reason = Reason::error; @@ -574,14 +581,8 @@ Result run(const Policy& policy, State& state, Engine& engine, const Bounds& bou } const double next = proposal(policy, step.dz, outcome.factor); const bool floor = next < policy.dz_min; - if (bounds.replay) { - // A replay cannot reject: the field has diverged (`replay_step`), and the - // attempt is not recorded; a floor is raised first, inside the rejection. - if (floor) { - result.reason = Reason::floor; - result.floor_proposal = next; - return result; - } + if (bounds.replay && !floor) { + // A replay cannot reject: its field has diverged (`replay_step`). state.proposal = next; result.reason = Reason::diverged; result.replay_dz = step.dz; @@ -589,21 +590,20 @@ Result run(const Policy& policy, State& state, Engine& engine, const Bounds& bou result.replay_norm = norm; return result; } - record(step.end, step.dz, norm, false, trial.order); if (floor) { - // A floor leaves the proposal and is the reason the march stops once the - // completed attempt is recorded (`HostAdamsMarch._reject`, solprop issue - // #651). The reference raises it inside its trial's scope, so a failure of - // the attempt's release is the host's to keep beside the floor, as an - // abandoned attempt's is, never in its place. - try { - attempt.end(HostOutcome::rejected); - } catch (...) { // NOLINT(bugprone-empty-catch) - kept by the host, see above + // A floor leaves the proposal; a replay's is raised unrecorded. Any other + // attempt is recorded first (`HostAdamsMarch._reject`, solprop issue #651); + // a failure of its release is the host's to keep beside the floor. + if (!bounds.replay) { + record(step.end, step.dz, norm, false, trial.order); + try { + attempt.end(HostOutcome::rejected); + } catch (...) { // NOLINT(bugprone-empty-catch) - kept by the host, see above + } } - result.reason = Reason::floor; - result.floor_proposal = next; - return result; + return floored(result, next); } + record(step.end, step.dz, norm, false, trial.order); state.proposal = next; try { attempt.end(HostOutcome::rejected); @@ -667,11 +667,8 @@ Result run(const Policy& policy, State& state, Engine& engine, const Bounds& bou fail(result, Stage::accept, error); return result; } - if (next < policy.dz_min) { - result.reason = Reason::floor; - result.floor_proposal = next; - return result; - } + if (next < policy.dz_min) + return floored(result, next); state.proposal = next; ++accepted; if (bounds.replay) From fbb154c875691e40890c0ffcc65e8663aa649e62 Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 19:28:33 +0000 Subject: [PATCH 4/4] native: keep the march fault hook's stage chain brace-consistent Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- native/src/bindings.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/native/src/bindings.cpp b/native/src/bindings.cpp index ff95fde9f..11df513d5 100644 --- a/native/src/bindings.cpp +++ b/native/src/bindings.cpp @@ -2067,17 +2067,16 @@ class BoundState { const std::string& category) { require_open(); native::engines::MarchFault fault; + // `trial_source` fails inside the trial's source evaluation, its prediction in + // the state. + fault.in_source = stage == "trial_source"; if (stage == "startup") fault.stage = march::Stage::startup; - else if (stage == "trial") + else if (stage == "trial" || fault.in_source) fault.stage = march::Stage::trial; else if (stage == "accept") fault.stage = march::Stage::accept; - else if (stage == "trial_source") { - // Inside the trial's source evaluation, its prediction in the state. - fault.stage = march::Stage::trial; - fault.in_source = true; - } else + else throw std::invalid_argument( "a march fault's stage is startup, trial, trial_source or accept"); if (category == "convergence")