fix(#127 review round 21): close four review findings, align release metadata with the H-only gate - #154
Conversation
…elease metadata with the H-only gate Four findings from the 2026-09-11/12 external review of PR #127, each reproduced on e556c96 before the change and pinned by a test that fails without it (tests/test_pr127_review_round21.py, 16 tests): - residual_model claimed a whitening for runs in which no fit succeeded: availability is now read off FitResult.success first and the family is classified only among successful fits; new label "ar1_unavailable" for a uniform grid with no successful fit (measured: "car1_fallback_ar1" with 5/5 success=False on rho_initial == rho_steady_state). - estimate_car1_theta returned NaN when the residuals' raw sum of squares underflowed (NaN at amplitude 1e-170, 4e-7 drift at 1e-150), so the residual AMPLITUDE alone switched fit_gls_ar1 to the AR(1) fallback: residuals are normalised to unit maximum before anything squares them. - the relaxation layer re-solved the spectrum with a bare np.linalg.eigvals twice per run: compute_relaxation_layer takes eigenvalues=, diagnose() forwards SpectralResult.eigenvalues, decay_rates/fastest_decay_rate/ samples_per_fast_efolding accept the same keyword. - the persisted report could not say which mode samples_per_fast_efolding described: RelaxationResult gains worst_resolved_rate, worst_resolved_blind_interval and worst_resolved_blind_start (additive, NaN-defaulted), the docstring of the scalar is corrected. CITATION.cff and CHANGELOG.md described the superseded generator-relative Hermiticity reference and called the dissipation question open; both now record the settled coherent-component contract (cross-family review, 2026-09-12). The refusal message of both builders gains a hint to pass the Hermitian part for a round-off-only pure-gauge H. Verification (local, Python 3.11.15, no QuTiP): pytest -q 1419 passed, 12 skipped, exit 0; tests/test_anchors.py 21 passed (byte-identical); ruff check exit 0; mypy src/liouscope exit 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JKJek43ESjfnQB79Hejxp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb397ff096
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion layer Codex review on PR #154: with the certified spectrum now forwarded unconditionally, an applicable-but-unresolved zero-mode certificate (the predicate under which the spectral layer withholds D1/D3/D4) still fed the relaxation layer, whose resolution warning and the new worst_resolved_* fields then named a specific missed mode read off the rejected candidate spectrum. Measured on the stiff four-level network at fast rate 1e8: worst_resolved_rate = 1e8, blind interval 0.127, start 0.0, samples_per_fast_efolding = 7.9e-8, with D1 = NaN in the same report. compute_relaxation_layer gains spectrum_resolved=; diagnose() derives it from the same certificate predicate it already uses for D16 and forwards the eigenvalues only when True. When False the layer derives nothing from the spectrum: legacy window (the withheld gap already implies it), no resolution guard, no warning, no eigensolve of its own, and the four spectrum-derived fields are NaN. A direct caller who omits the gap gets the flag from the same spectral-layer call. Three tests added (diagnose path, no-eigensolve spy, direct-caller path). pytest -q: 1422 passed, 12 skipped, exit 0; ruff 0; mypy 0; anchors byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JKJek43ESjfnQB79Hejxp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 141badf619
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…supplied grids too Codex review on 141badf: a direct caller who supplies t_grid but neither eigenvalues nor spectrum_resolved skipped the spectral-layer call, so the resolution guard launched a bare eigensolve and, on the stiff unresolved fixture, persisted worst_resolved_rate ~= 1e8 with an UnderResolvedTransientWarning -- the missed mode the pipeline path had just withheld. spectrum_resolved is now bool | None. Whenever the caller supplies neither a spectrum nor a verdict, the layer runs the spectral layer once (the same call that already served the default window) and reads both the verdict and the eigenvalues from it, for caller grids as well; passing eigenvalues alone counts as the caller's assertion that they are trustworthy. Four tests added: caller grid on the unresolved fixture (NaN fields, no warning, no bare eigensolve beyond the certified solver's own), caller grid on a healthy generator still names the mode, explicit eigenvalues are honoured. Two spy tests corrected: a second spy wrapped the first and double-counted. pytest -q: 1425 passed, 12 skipped, exit 0; ruff 0; mypy 0; anchors byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JKJek43ESjfnQB79Hejxp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e17d2b06
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nvalues Codex review on e7e17d2: a caller grid with an explicit spectrum_resolved=True and no eigenvalues skipped the spectral layer, and the resolution guard ran the bare eigensolve the contract exists to prevent -- 1e8 again on the stiff unresolved fixture. True is an assertion about the eigenvalues the caller passes; without them there is nothing it can be about, so the pair is now a ValueError instead of a silent re-solve. Test added on a healthy and on the stiff generator. pytest -q: 1426 passed, 12 skipped; ruff 0; mypy 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JKJek43ESjfnQB79Hejxp
Summary
Stacked on PR #127 (
fix/115-zweiskalen-car1), four commits. Closes four of the nine unresolved review threads on #127 and alignsCITATION.cff/CHANGELOG.mdwith the Hermiticity contract that the cross-family review settled on 2026-09-12 (H-only, dissipation is diagnostic). Each finding was reproduced one556c96before the change and is pinned by a test that fails without it (tests/test_pr127_review_round21.py, 24 tests, measured pre-fix values in the docstrings).relaxation.py:897residual_modelreported"car1_fallback_ar1"with 5/5success=False(stationary run)FitResult.successfirst; family classified among successful fits only; new label"ar1_unavailable"car1.py:205estimate_car1_thetareturned NaN at residual amplitude1e-170(4e-7 drift at1e-150), switching the whitening on amplitude alonerelaxation.py:818np.linalg.eigvalssolves inside the relaxation layer, after the spectral layer had certified a spectrumcompute_relaxation_layer(eigenvalues=, spectrum_resolved=);diagnose()forwardsSpectralResult.eigenvaluesonly when the zero-mode certificate is resolved; zero re-solves in the pipelinerelaxation.py:989samples_per_fast_efoldingdescribes (three-scale grid: the intermediate one)RelaxationResult.worst_resolved_rate / _blind_interval / _blind_start, additive, NaN-defaulted; docstring correctedCITATION.cff:159max(coherent, dissipation)reference and called E3 openlindblad.py:213Q I Q^His refusedThree Codex rounds on this PR (all five threads resolved):
141badfgates the spectrum forwarding on the certificate verdict (an applicable-but-unresolved certificate yields NaN resolution metadata instead of a missed mode read off the rejected spectrum);e7e17d2extends that gate to caller-supplied grids, which used to skip the spectral layer and launch a bare eigensolve;4dd8c61refusesspectrum_resolved=Truewithouteigenvalueswith aValueErrorinstead of re-solving.Not touched here, on purpose: the CAR(1) likelihood / parameter-count trio (
gls.py:194,gls.py:119,relaxation.py:637) is one modelling decision that moves anchors and is waiting on Marco per the 2026-09-07 status on #127; the non-normal zero-mode displacement (linalg.py:1054) is issue #117.Scope
CITATION.cff,CHANGELOG.md,docs/explanation/layers-and-taxonomy.mdVerification
pytest -qpasses locally: 1426 passed, 12 skipped (QuTiP not installed), exit 0 on4dd8c61(Python 3.11.15)tests/test_anchors.pyunchanged (byte-identical tomain), 21 passedruff check src tests benchmarksexit 0;mypy src/liouscopeexit 0MANIFEST_SCHEMA.json: n/a — the run-manifest contract is untouched; the newRelaxationResultfields are additive and defaulted.github/workflows/: not touchedtest 3.10-3.14+qutip-cross-check 3.11/3.12: 7/7 SUCCESS on4dd8c61(runs 34765925307 / 34765925315, 2026-09-13 15:32-15:42 UTC);mergeable_state = clean; all review threads resolvedQuality contract
Reproducibility note
Seed 1 with
bootstrap_B=5in the new tests; fixtures are the two- and three-qubit amplitude-damping generators already used bytests/test_relaxation_grid_scale.pyand the stiff four-level jump network oftests/test_spectral_certificate.py. Existing CAR(1) estimates move only at the estimator's round-off floor (~1e-8 relative); no anchor moves. Numerics of accepted Hermiticity inputs are unchanged; the message text of the refusal grew by one sentence.History note (AGENTS.md): new branch from the #127 head, no history rewritten, no force-push; the 2026-05-16 incident (unverified branch delete + GH-GC, ~20 files recovered from the backup triple) is why this stacks instead of amending. Merge into
fix/115-zweiskalen-car1as a merge commit, not a squash, so the head SHA stays reachable from #127.Linked issues
Refs #127, #122, #115. Does not close an issue.
🤖 Generated with Claude Code
https://claude.ai/code/session_018JKJek43ESjfnQB79Hejxp