Skip to content

Fix Adams failure semantics: restore state after a failed native source, record the rejection before its floor - #814

Open
jtravs wants to merge 4 commits into
mainfrom
fix/adams-failure-state
Open

jtravs wants to merge 4 commits into
mainfrom
fix/adams-failure-state

Conversation

@jtravs

@jtravs jtravs commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Two Adams failure-semantics fixes, one commit each.

#129: a failed native Adams source no longer leaves the prediction in the state

  • The bare std::swap(current_, adams_saved_) pairs in adams_fused_trial and adams_elementwise_trial become a SwappedAside scope guard (native/src/general/kernels.hpp), and adams_evaluate (coupled trial and startup) restores current_ before rethrowing.
  • New test hook adams_march_fault("trial_source", …) throws once at the start of the next source evaluation, while the prediction is in the state.
  • Regression test on the elementwise, fused and coupled paths: the checkpoint field and march snapshot equal those from a fault injected before the prediction (to 1e-12 relative). All three fail without the fix.

#651: record the completed rejected attempt before raising the floor

  • NumPy reference (host/adams.py): _reject holds the floor error, and march_to raises it after _record, as _propose already does for accepted steps. Native (adams_march.cpp) does the same. A replay is unchanged (no row; the floor is raised first). Proposal, state, ledger and message are unchanged.
  • On this floor path, a failure while releasing the attempt is kept as a note on the floor error, matching the reference (the TDSE abandoned-attempt test covers it).
  • As the issue asks, the pre-refactor trace corpus is not regenerated: floor_after_rejections joins the existing CHANGED sets, and an explicit test checks that the new trace equals the corpus trace plus exactly one rejected row. The issue's witness scenario is tested on both marches.
  • docs/dev/kernel-contract.md documents the floor ordering.

Checks: ruff, basedpyright, poe docstrings, clang-format, codespell; 252 targeted tests on CPU (Adams traces, native march traces, TDSE native, march lifecycle, session history, resident Adams, exponential march). Not run locally: clang-tidy, sanitizers, device builds (CI device-compile covers the builds; an NVIDIA A40 GPU test run will be reported here).

Closes #129
Closes #651

🤖 Generated with Claude Code

https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH

jtravs and others added 2 commits November 13, 2026 17:31
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH
@read-the-docs-community

read-the-docs-community Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 solprop | 🛠️ Build #35876910 | 📁 Comparing fbb154c against latest (a3ac8c4)

  🔍 Preview build  

2 files changed
± dev/kernel-contract.html
± _modules/solprop/backends/host/adams.html

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH
@jtravs

jtravs commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

GPU lane on an NVIDIA A40 (CUDA 12.8.1, AMPERE86 build) at 8281643:

  • native gpu tests: 31 passed, 1 skipped.
  • solprop gpu tests: 289 passed, 1 skipped, 6 failed. The 6 are segfaults on the radial route on the device (test_monitor_reduction radial-* split removal, test_native_session_device radial/collar cases). They reproduce identically on main at a3ac8c4, so they predate this PR; tracked separately.
  • The Adams march/trial modules (test_native_march_lifecycle, test_native_march_traces, test_resident_adams, test_adams_traces) pass against the device build: 138 passed (134 on main, plus this PR's 4).

The later commit 0bde86e only restructures host-side adams_march.cpp control flow to stay under clang-tidy's function-size limit; there is no device code change.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T19:30:24.286125Z fbb154c New commits
🔒 Security Review ✅ Completed 2026-11-13T19:31:05.835293Z fbb154c New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@jtravs

jtravs commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Pre-existing radial device crash filed as #815.

@codecov

codecov Bot commented Nov 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant