fix(#122): repair the two-scale relaxation window instead of disclosing it - #127
fix(#122): repair the two-scale relaxation window instead of disclosing it#127marcohost33-maker wants to merge 40 commits into
Conversation
…on time The relaxation layer fits every rate it reports on a time grid, and a decay rate carries dimension 1/time, so the absolute default `linspace(0.0, 10.0, 80)` was an unstated claim about the caller's unit of time. Measured on an amplitude-damped qubit under the pure rescale `L -> cL` (identical physics, different unit of time): c beta_D/c beta_D_linear/c AICc A-class 1e+02 1.025 0.254 M0 A10 1e+00 1.029 0.482 M2 A5 1e-02 1.085 0.403 M0 A1 1e-04 1.167 0.965 M0 A12 1e-06 1.251 109.99 M0 A12 A 22% drift in beta_D, a factor ~430 error in the D17 linear rate, a model-selection flip, and four different mechanism classes for one system. Physical rates are MHz or GHz rather than O(1), so this was the common regime. The default window is now [0, RELAXATION_HORIZON / Delta] at 80 uniform samples: a fixed number of e-foldings of the slowest mode, the only window carried along by the rescaling. After the fix beta_D/c and beta_D_linear/c are invariant to <=1.2e-3 relative over twelve decades on both an amplitude-damped and a driven dephasing qubit, the AICc winner is stable, and the A-class is stable over c in [1e-6, 1]. The grid is uniform by requirement: the GLS layer whitens with a single AR(1) coefficient, which presumes a constant sample interval, so the transient layer's two-scale grid must not be reused here. Backward compatibility: HORIZON = 10 makes the grid bit-identical to the legacy linspace(0.0, 10.0, 80) at Delta = 1, and an unresolved gap (Delta <= 0 or NaN per #113) still returns the historical absolute window. `compute_relaxation_layer` gains an optional `gap=`; diagnose() forwards the D1 it already computed, and a direct caller who omits it gets it from `compute_spectral_layer` rather than a local re-derivation, so the certified zero-mode tolerance (#112) and the ambiguity rule (#113) cannot drift between the two entry points. An explicit t_grid remains authoritative. Audit trail: RelaxationResult gains additive, defaulted `t_grid_source` ("caller" / "gap_scaled" / "legacy_fixed") and `t_grid_span`. The run-manifest contract is unchanged and older serialised reports stay valid. Scope: this closes the time-grid unit dependence only. The solver's own convergence controls (#111) and the rate-dimensioned henrici_eta / resolvent_peak the classifier consumes (#101) remain open and are neither fixed nor asserted away; the latter is pinned as a measured boundary (henrici_eta == c exactly, flipping A5 -> A10 between c = 1 and c = 3). Test plan: 21 new tests in tests/test_relaxation_grid_scale.py; full suite 963 passed; anchors green; ruff and mypy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019c8r6BNjBTkpN9KemEG6up
The scaling tests interrogate different fields of the SAME sweeps, so each assertion was re-running an identical pipeline call. Caching per (system, rate unit) cuts the module from 71.5s to 41.6s locally without changing coverage -- worth it on a five-version CI matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019c8r6BNjBTkpN9KemEG6up
…r the next cut CITATION.cff's documented convention: unreleased changes go in the pending-for-next-cut block, and result-changing ones must be described as corrections rather than additions. This change alters numerical results for any run without an explicit t_grid whose gap is not 1, so it owes an entry (Definition-of-Done item 5). Bounded to what was measured, and explicitly NOT claiming general rate-unit invariance of the fitted rates: #111 (solver convergence controls) and #101 (rate-dimensioned henrici_eta gate) remain open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019c8r6BNjBTkpN9KemEG6up
Two findings from the Codex review. P1 -- widely separated rates. A uniform window scaled to 1/Delta cannot also resolve a much faster mode: the window must reach ~1/Delta to see the slowest mode relax while the step must stay under ~1/r_max to see the fastest at all, and 80 uniform samples straddle only ~8x separation. Confirmed: with rates 1e-6 and 1 the fast mode decays to exactly 0.0 within one step. Not a regression from the new window -- on that same system the previous absolute window put beta_D_linear 3.5e4 relative from the true gap against 0.58 for the gap-scaled one -- but silently fitting a component that was never sampled is what the fail-loud convention exists to prevent. The layer now measures samples_per_fast_efolding = 1/(r_max*dt) and warns below 1, recording the value. A disclosure, not a repair: widening the window is strictly worse for the reported quantity and log spacing would invalidate the AR(1) whitening. A curve-shape heuristic was tried first and rejected -- the share of decay in the first interval measures the fast component's AMPLITUDE, not whether it was RESOLVED (0.49 for a fully unsampled 1e-6/1 separation vs 0.46 for a mild 0.1/1 one). The spectral measure is also itself rate-unit invariant, pinned by test, so the guard cannot fire on one choice of time unit and not another. V5 (JC near the EP, r_max/Delta = 396) legitimately trips it; those tests acknowledge the disclosure by message filter with the reason stated inline. P2 -- the span does not identify the sampling. [0, 1, 10] and [0, 9, 10] share a span of 10 while describing different trajectories, and the report already serialised three 80-point curves whose abscissa was missing. RelaxationResult now stores the grid itself as a snapshot copy. Additive, defaulted, no manifest-schema change. Test plan: 8 new tests (fires on separation, silent on single-timescale at three rate units, invariant guard, grid identifies sampling, snapshot not alias, length matches the curves); full suite 971 passed; ruff and mypy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019c8r6BNjBTkpN9KemEG6up
Second-round Codex finding, against the guard added in the first round.
samples_per_fast_efolding used only dt = t[1] - t[0]. diagnose() permits any
non-negative grid start, so a late-starting grid leaves a lead-in from t=0
that no step size compensates for. Measured on linspace(100, 101, 101) with a
rate-1 mode: reported 100 samples per fast e-folding and emitted no warning,
while the amplitude at the first sample is e^-100 -- the entire
relative-entropy curve measures identically zero and the fit still returned a
confident beta_D = 1.0.
The denominator is now max(dt, t[0]), the largest interval the grid leaves
unsampled. That is exactly dt for any grid starting at zero, so the default
path is bit-for-bit unchanged (verified: 3.95 samples per e-folding at
c in {1e-4, 1, 1e4}, as before). The warning text now names which gap
dominates rather than always printing the step.
Test plan: 2 new tests (late-starting grid warns and reports 0.01; lead-in
inert when t[0] == 0); full suite 973 passed; ruff and mypy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019c8r6BNjBTkpN9KemEG6up
… (PR #115) The under-resolution disclosure rested on the claim that a non-uniform grid is impossible because "the GLS layer whitens with a single AR(1) coefficient, which presumes a constant sample interval". That is a property of the discrete parametrisation, not of the noise: the stationary continuous-time process has Corr(t, t+d) = exp(-theta d), so whitening with the per-step exp(-theta dt_k) is valid on any grid. - new fitting/car1.py: grid classification, CAR(1) theta MLE, constant-variance whitening + its log-Jacobian, exact ESS, exact-transition resampling - gls.fit_gls_ar1 switches on the GRID: uniform keeps the historical AR(1) path bit-for-bit, non-uniform gets CAR(1); GLSFitOutput.theta_car1 disambiguates - parametric_bootstrap resamples under whichever model the fit used - default_relaxation_grid takes a spectrum-derived fast_rate and builds a two-scale grid only when the uniform one cannot resolve it (fail-closed) - samples_per_fast_efolding now minimises over ALL decay modes and counts only intervals starting while the mode still has amplitude; the value is unchanged on uniform and late-starting grids Beleglauf folgt (volle Suite ~6 min). Erwartet rot: der Test, der die Offenlegung festnagelt -- er wird auf die Reparatur umgeschrieben.
tests/test_car1.py (new, 40 cases): AR(1)<->CAR(1) log-likelihood identity on a uniform grid, N_eff against the closed-form ESS (machine-exact), theta recovery across six decades asserted on MOVEMENT rather than value, fail-closed paths, per-step vs constant-rho whitening with a uniform positive control, resampler correlation at both scales, and a bootstrap-spread calibration against an independent 25-run Monte Carlo. tests/test_relaxation_grid_scale.py: the disclosure test becomes a repair test (no warning, gap_scaled_multiscale, car1, M2 wins, fast rate 1.10 vs true 1.0 where the uniform window reported 2.17e-05); plus the seed-displacement proof, the D17 linear-rate sharpening (3.5e4 / 0.58 / 0.021), the intermediate-scale disclosure that must REMAIN, a fine-head-coarse-tail grid the old measure would have waved through, and bit-identity positive controls for single-scale systems. Beleglauf (volle Suite, ~6 min) folgt.
CHANGELOG, CITATION.cff (numerical-results correction + what is explicitly NOT fixed), docs/explanation/layers-and-taxonomy.md and the diagnose() docstring. The stale rationale in test_default_grid_is_uniform is corrected: the contract survives, its justification does not. mypy 0 errors, ruff clean. Beleglauf (volle Suite, ~6 min) folgt als naechstes.
The 0.4231 / 0.0945 / 0.0728 triple came from the analysis that prompted this work and was never re-measured here. Re-run with this module's own code on the grid this layer actually builds (40 runs, theta = 0.7): 0.3738 constant rho vs 0.0728 per-step, with 0.0728 for both schemes on the uniform positive control. Same conclusion, own numbers.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d9b6b8cd1
ℹ️ 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".
CI job test (ubuntu-latest, 3.10) of PR #127 failed in the Type check step (run 33241287852, job 99070960246): relaxation.py:194: error: Returning Any from function declared to return "ndarray[Any, Any]" [no-any-return] Cause is mine, not inherited: default_relaxation_grid returns the result of np.concatenate, which numpy's own stubs type as Any up to 2.2.x and as ndarray from 2.3 onward. Python 3.10 resolves numpy 2.2.6 (2.3+ requires >= 3.11), 3.11-3.14 resolve 2.5.2 -- so the same code type-checks on four runners and fails on one. Reproduced locally in a real 3.10.20 venv (numpy 2.2.6, mypy 2.3.1 -- identical mypy to the green runners, so the stub is the variable). Fixed by the convention the repo already uses for this in transient.py (_transient_grid): wrap in np.asarray(..., dtype=float). Applied to the two concatenates in _resolution_detail as well -- there the Any did not surface as an error but silently disabled checking of everything computed from them. Runtime unchanged: concatenate already returns a float64 array. Beleglauf auf 3.10 folgt (volle Suite, ~6-10 min); der CI-Job brach vor den Tests ab, diese Version ist also noch ungemessen.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # CHANGELOG.md
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Round-17 review by chatgpt-codex-connector on PR #127. All four findings reproduced before building; the fourth contradicted my own earlier report and is corrected below. 1. numerics/linalg.py -- the zero-mode certificate accepted its PREMISE (exact trace preservation) on a scale six orders coarser than its CONCLUSION (an exact zero eigenvalue). Applicability is now capped at the backward-error scale the certificate itself claims on, derived rather than tuned: L - u r^H with u = vec(I)/sqrt(d) has an exact zero mode and lies at spectral distance tp_defect/sqrt(d) from L, so tp_defect <= sqrt(d)*bound is what the claim can carry. The rule now lives in ONE helper used by both certified_eigvals and certified_eig -- the duplicate was the vector this repository has already shipped twice. 2. diagnostics/relaxation.py -- residual_model was read off the grid geometry, i.e. it reported the whitening that should have happened. It is now derived from the fits, with car1_fallback_ar1 / car1_mixed / car1_unavailable for the states one flat label could not express. 3. diagnostics/spectral.py -- D1 was computed from a spectrum the certificate had just declared unusable and forwarded into default_relaxation_grid, so an eigenvalue of a failed solve set the relaxation window and every fitted rate; only the closing verdict was floored. Now NaN, as for the ambiguous case. 4. fitting/prony.py -- the grid-relative fallback seed was a REGRESSION on the two-scale grid: beta and omega were taken from the total span, i.e. the slow gap scale, although the grid carries a uniform fine head for exactly the fast dynamics. Measured on default_relaxation_grid(1e-4, fast_rate=1): 60 random curves with beta in [0.05,2], omega in [0.1,10] -> 2 spurious fits (e.g. omega 1.68 reported as 0.037) where the historical (1,1) seed found all 60; on a slower family 41 of 60 failed. Estimating from the longest uniform PREFIX gives 0 of 60 in both families. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JsDFcKo97HuF5ZLjxa3rRa
…del vocabulary CHANGELOG entries appended to the existing [Unreleased] Fixed section (which already carries both #127 and #129 items -- added, not reordered), including the explicit correction of my earlier report that the Prony seed change was not a regression: the reviewer's counter-example holds as a class. layers-and-taxonomy.md still documented residual_model as a two-value field. That line was found by turning the very defect class of finding 2 -- a label asserting what should have happened rather than what did -- against my own result, one layer up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JsDFcKo97HuF5ZLjxa3rRa
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3abb353b89
ℹ️ 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".
Sammelantwort auf die 5 offenen Review-ThreadsVier der fünf Punkte sind auf
Zu
|
…f D1 Fifth finding of the round-17 external review on PR #127, the one the previous fix commit (4d9a99f) did not cover. Reproduced at 21c99ee before any change. Where the zero-mode certificate is applicable but not resolved, the spectral layer warns that "D1/D3/D4 are NOT reliable for this system" and replaces D1 with NaN -- and then returned oscillating_gap (D3), spectral_spread (D4) and has_complex_pairs as finite values read off that same candidate spectrum. Measured on _stiff_with_fast_rate(1e8), eight ambiguous in-band modes, D1 already NaN: D3 = 0.0, D4 = 5.0e7, has_complex_pairs = False. None of the three is a neutral answer. 0.0 is the strongest "no oscillatory separation" verdict D3 can emit, and False asserts the ABSENCE of the oscillation that may be the very reason the spectrum is unresolved -- zero_tol excludes exactly the ambiguous in-band mode from that test. Withholding D1 alone was not a partial fix but an inconsistent one: it taught consumers that the layer withholds what it cannot stand behind, which made the surviving finite values more credible, not less. The predicate is "applicable and not resolved", the same one D1 uses, not the narrower "not certified": a certificate can be certified=True and still resolved=False when an in-band mode is ambiguous (#113), which is precisely the case in which the unresolved mode may be a slow oscillatory one. SpectralResult.has_complex_pairs is therefore bool | None. _gather_evidence maps None onto the evidence dict's NaN sentinel, which _strip_unavailable removes, so the A8 rung reports UNEVALUABLE rather than NOT_SUPPORTED -- float(None) would have raised and a silent 0.0 would have read as a measured "no oscillating pairs". D2/D2b are untouched: operator-derived, not read off the candidate spectrum. Over-correction control included: every shipped example keeps a finite D4 and a bool flag. Discrimination proven per GUARD LINE, 4/4, run DK-20260902T223317-e35245722d7c (ledger Vero/Data/diskriminierung_runs.jsonl): osc-NaN, spread-NaN, has_complex-None and the classifier's NaN mapping each turn their test red at an assertion when removed, green again after restore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71e5f39bfd
ℹ️ 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".
…s at the next Round-18 external review of PR #127. All four findings are the same class: a value is correctly withheld (NaN / no representable window) and the next consumer treats the absence as a reading. - classification.py: `gap` was OPTIONAL on the F5 reach leg, so a withheld D1 was answered with the gapless default 0.0 and F5 reported SUPPORTED. It is a REQUIRED key now; a MEASURED 0.0 still fires (#101 blind spot unchanged). - stability_report.py: D1/D3/D9 reached json.dumps(allow_nan=False) as bare floats, so only the fail-closed reports could not be written. Non-finite diagnostics are encoded; finite ones stay bare floats. - relaxation.py: horizon/gap overflows for a positive gap below horizon/float64.max and produced a [nan, inf, ...] grid. The derived window is guarded; the case joins the documented absolute-window fallback. - relaxation.py: the fitted CAR(1) theta now counts in the AICc parameter count on the non-uniform path (the correction is nonlinear in k). Beleglauf folgt (volle Suite, rund 10 min). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c07141badf
ℹ️ 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".
| if np.isfinite(theta): | ||
| whitened = whiten_car1(residuals_raw, t, theta) | ||
| jac = whiten_car1_log_jacobian(t, theta) | ||
| rho = float(np.exp(-theta * float(np.mean(np.diff(t))))) | ||
| else: | ||
| whitened = _whiten(residuals_raw, rho) | ||
| # Prais-Winsten exact AR(1) likelihood: _whiten keeps observation 0 | ||
| # (scaled by sqrt(1-rho^2)), so the transform has log-Jacobian | ||
| # 0.5*log(1-rho^2). Omitting it makes the reported value a hybrid of | ||
| # the exact and conditional likelihoods and biases cross-model AICc | ||
| # (each model fits its own rho) toward under-fitting high-rho models. | ||
| jac = 0.5 * float(np.log(max(1.0 - rho * rho, 1.0e-12))) | ||
| n = whitened.size | ||
| sigma = float(np.sqrt(max(np.dot(whitened, whitened) / max(n, 1), 1.0e-30))) | ||
| # Prais-Winsten exact AR(1) likelihood: _whiten keeps observation 0 | ||
| # (scaled by sqrt(1-rho^2)), so the transform has log-Jacobian | ||
| # 0.5*log(1-rho^2). Omitting it makes the reported value a hybrid of the | ||
| # exact and conditional likelihoods and biases cross-model AICc (each | ||
| # model fits its own rho) toward under-fitting high-rho models. | ||
| jac = 0.5 * float(np.log(max(1.0 - rho * rho, 1.0e-12))) | ||
| log_lik = gaussian_log_likelihood(whitened, sigma=sigma) + jac |
There was a problem hiding this comment.
On every non-uniform grid, estimate_car1_theta minimizes a conditional likelihood that excludes the first residual, but these lines then report the stationary exact likelihood, including that residual and re-estimating sigma from all n whitened values. Consequently the counted CAR(1) parameter is not fitted to the likelihood passed to AICc, and the deficit differs across candidate mean models because their first residuals differ. For an exact OU path on the default two-scale grid with true theta=1e-4, the conditional estimate hits 1e-5 while maximizing the exact expression used here gives about 1.02e-3 and lowers the -2 log L contribution by 3.68—enough to change close model selections. Either optimize the full stationary likelihood or report a consistently conditional likelihood.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deliberately left open, and this is a judgement rather than an omission.
Your reading holds: estimate_car1_theta minimises a conditional likelihood, while lines 180-194 report the stationary exact one. The two do not agree, and the report should not claim what the optimiser did not do.
What stops a fix here is that your comment offers two routes and they are not equivalent. Either would shift log_likelihood on every non-uniform grid, and that shift propagates: likelihood -> AICc -> model selection -> possibly the anchored expectations in tests/test_anchors.py. This repository's working agreement allows anchors to move only in a dedicated PR carrying a physics justification.
So this is a statistical modelling decision, not a repair, and picking one of the two routes at 3 a.m. to close a thread would be exactly the kind of quiet choice this review round has been about. It is recorded as an open item with both routes written down; the thread stays open until it is decided on its merits.
Third round on this branch set. Instead of patching the reported sites we measured each class across the branch first - underflow-capable norms 7 of 42 affected, absolute-vs-scale-relative thresholds 5 of 22, overflowing windows 3 of 4 - which surfaced five affected sites no comment had reported. Those are recorded separately rather than folded in here. Verified locally: 1091 / 1070 / 1097 tests collected on #121 / #127 / #147, ruff and mypy clean. Reverse-mutation runs are in an evidence ledger; blind mutations are counted and named rather than dropped. CI execution on GitHub not measured.
There was a problem hiding this comment.
Status check, 2026-09-07: still deliberately open, and re-verified as still applicable.
The state above is unchanged and the reasoning still holds. Recording the check so a later reader does not mistake an open thread for a forgotten one.
Verified today at e0a6479: the mismatch is still present. estimate_car1_theta minimises the conditional likelihood (first residual excluded); fitting/gls.py:180-225 reports the stationary exact one, including that residual and re-estimating sigma from all n whitened values. Nothing on this branch since 2026-09-03 has touched either site.
Why it is not being closed, restated for the record. The comment offers two routes — optimise the full stationary likelihood, or report a consistently conditional one — and they are not equivalent. Either shifts log_likelihood on every non-uniform grid, and that shift propagates likelihood -> AICc -> model selection -> A-class, which can reach the anchored expectations in tests/test_anchors.py. This repository's working agreement allows anchors to move only in a dedicated PR carrying a physics justification. Picking one of the two routes to clear a thread would be a statistical modelling decision made for procedural reasons — the exact thing this review round has been about.
One thing that has changed: this is no longer the only open decision of its kind on this branch. core/lindblad.py:187 is now in the same category — a threshold decision with anchor risk, measured and deliberately left open (see that thread). Both belong in the same follow-up: a decisions PR that states each choice, its physics justification, and the anchor movement it causes, rather than two quiet changes in a review round.
Related, and it is the same shape one layer up: the still-open diagnostics/relaxation.py:637 finding — that CAR(1)'s profiled stationary variance is estimated but not counted in k for AICc — moves the same -2 log L bookkeeping this thread is about. Whoever takes the follow-up should decide both together; fixing the parameter count against a likelihood that the optimiser did not maximise would correct one half of a two-part inconsistency and could easily make the selection worse, not better.
Not resolving. An open thread with a written reason is the correct state here; the thread closes when the decision is made on its merits, not when the branch merges.
There was a problem hiding this comment.
Correction to my own comment above — one sentence of it was wrong, and it was the sentence I had not measured.
I wrote "Nothing on this branch since 2026-09-03 has touched either site." That is false, and I checked it only after posting. git log on this worktree at e0a6479:
d68800a 2026-09-04 docs(#127): the AICc likelihood mismatch bites only where the theta search clamps
That commit touches exactly this site, and it is not cosmetic — it records a third route that my summary therefore omitted:
repair the search range so the estimate stops clamping. It would leave every interior fit bit for bit unchanged and so would not move the anchors, which is what makes the other two routes expensive. UNVERIFIED as a remedy.
That materially changes the decision this thread is waiting on. The two routes in the original finding are expensive because they move log_likelihood on every non-uniform grid and so carry anchor risk. A third route that leaves interior fits bit-for-bit unchanged would not — if it is sufficient, which d68800a explicitly does not claim. Whoever takes the follow-up should test that route first, because it is the only one of the three that could be decided without an anchor-moving PR.
What survives from my comment above, and is measured: the mismatch itself is still present at e0a6479 — estimate_car1_theta minimises the conditional likelihood, while fitting/gls.py:223-225 computes sigma from all n whitened values and reports the stationary exact likelihood:
n = whitened.size
sigma = float(np.sqrt(max(np.dot(whitened, whitened) / max(n, 1), 1.0e-30)))
log_lik = gaussian_log_likelihood(whitened, sigma=sigma) + jacThe thread stays open for the same reason as before, now with three routes on record instead of two.
PR #121 and PR #127 solved the same defect independently -- a withheld diagnostic reached json.dumps(..., allow_nan=False) as a bare float, so the fail-closed run was the only one whose report could NOT be written. The two fixes then collided in src/liouscope/io/stability_report.py, because they disagreed on what the written value should BE. This adopts the tagged form (#127) and drops _json_number (#121). The reason is not style. _json_number mapped every non-finite value to plain null, which erases the difference between a MEASURED infinity -- a Kreiss constant or Petermann factor that really did diverge, with documented per-diagnostic semantics -- and a value the run could not determine at all, which is what the NaN sentinel means. That distinction is the subject of the entire review wave both fixes came out of. A report that writes null for both can never give it back: the information is gone at the moment of writing, and no later reader can recover it. The tagged form keeps both facts on disk: value=null plus claim_status and a __nonfinite__ token of "nan" / "inf" / "-inf". A finite value with no claim_status stays a bare float, so every existing consumer of payload["diagnostics"]["D1_gap"] is unaffected -- the contract changes only where it previously lied. _diagnostic_entry is taken verbatim from c07141b (pr127-work), not rebuilt. Two implementations of one contract is the defect being repaired here. Evidence, measured on this branch: * baseline 6f4c198: 1084 passed, ruff clean, mypy 53 files clean * with the encoder swapped and the tests still untouched, exactly two nodes fell, both on `is None`, and both death messages name the distinction the change exists to preserve: round22::test_a_withheld_diagnostic_does_not_break_the_report assert {'__nonfinite__': 'nan', ...} is None round22::test_an_infinite_diagnostic_is_also_encoded_as_unavailable assert {'__nonfinite__': 'inf', ...} is None The finite-value positive control (round22::test_a_resolved_run_still_carries_its_numbers) stayed green, so the contract moved only for values that have no number. Both fallen nodes are lifted onto the tagged form; the inf case is renamed to what it now asserts and gains a nan-vs-inf discrimination step. The #127 contract tests in tests/test_stability_report.py are taken verbatim too, including their negative control that a measured value stays a bare float. Refs #121, #127 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
… round-off Three of the four open round-19 threads. gls.py:194 is NOT in this commit -- see the deferral note at the end. 1) THREAD PRRT_kwDOSXoNQ86evaQT -- src/liouscope/fitting/prony.py:104 The whole-grid uniformity test used NumPy's default atol=1e-8, an ABSOLUTE time. Measured on default_relaxation_grid(1e8, fast_rate=1e12): steps span a factor of 1.03e4 and every one is below 1e-8, so the grid was declared uniform, the two-scale prefix path never ran and Prony was applied across the discontinuity. Now atol=0.0 -- the convention _uniform_prefix_length already spells one screen above and that both were documented as sharing. No new constant. Negative control: linspace grids at spans 1e-9, 5, 1e9 remain uniform. 2) THREAD PRRT_kwDOSXoNQ86evaQU -- src/liouscope/diagnostics/relaxation.py:191 Round 18 stopped the [nan, inf, ...] grid and returned the absolute legacy window instead, which is a different wrong answer: the fits ran anyway. On an amplitude-damping generator rescaled by 1e-307 (gap 5e-308) that produced beta_D ~ -1.9e-18, beta_D_linear ~ 84, class A12 and the provenance tag t_grid_source="gap_scaled" for a grid that was [0, 10], while the unscaled twin reports rates near 1 and class A5. Now raises UnrepresentableRelaxationWindowError. The two states stay distinct: gap <= 0 / non-finite is NOT DETERMINABLE and keeps its documented fallback (measured: n=80, max=10.0, unchanged); a resolved gap whose window overflows is DETERMINED BUT NOT REPRESENTABLE. The round-18 test is rewritten, not deleted, and says why it turned. 3) THREAD PRRT_kwDOSXoNQ86evaQX -- src/liouscope/core/lindblad.py:121 plus the mirrored sparse calculation the finding names (src/liouscope/sparse/build.py:56). Gauge fixing removes the whole physical scale when H is numerically a pure gauge term, leaving round-off compared against round-off: measured, the exact Hamiltonian I written as Q @ I @ Q.conj().T has defect 4.98e-17 against a gauge-fixed scale of 1.99e-16 and both builders raised on it. Added an ADDITIVE machine-round-off allowance d * eps * |trace(H).real/d| -- the representation error of the component that was removed, the same backward-error idiom the repo uses for the Schur split. Bounded and measured: for a TRACELESS H the allowance is exactly 0.0, so those verdicts are bit-for-bit unchanged; the twelfth-round gauge hole (defect 1e-6 plus 1e9*I) is still rejected, allowance 4.44e-7 against a defect of 1e-6. Knowingly conceded and documented in the code: a defect of 2e-7 under a 1e9 gauge shift is now accepted, because 1e9*eps = 2.2e-7 is the resolution of the stored matrix. DEFERRED, not forgotten -- THREAD PRRT_kwDOSXoNQ86evaQW, fitting/gls.py:194. The finding is confirmed by reading: estimate_car1_theta minimises a CONDITIONAL likelihood while lines 180-194 report the stationary EXACT one. The reviewer offers two remedies and they are not equivalent; both move the reported log-likelihood on every non-uniform grid, hence AICc, hence model selection, hence potentially tests/test_anchors.py -- which AGENTS.md working agreement 3 says may only change in a dedicated PR carrying the physics rationale. Choosing between "optimise the full stationary likelihood" and "report a consistently conditional likelihood" is a statistical modelling decision, not a repair, so it is left to a decision rather than guessed at here. Baseline note: the pr127-work suite reported 1053 passed / 8 errors before any change here. All 8 were FileNotFoundError at tmp_path setup -- the pytest temp root was deleted by something outside this run. Re-run in isolation: 21/21 passed. True baseline 1061. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
The discrimination run DK-20260903T072514-64660e33dc01 reported the tests committed in a84a009 as follows, and two of the verdicts were about the tests rather than the code: * M1 Prony BLIND. The test asserted np.allclose(steps, steps[0], atol=0.0) in the TEST FILE -- it exercised the comparison operator, not the branch in prony.py, so reverting the fix changed nothing it could observe. A guard that reads the expression instead of the operation is not a guard. Replaced by the observable signature of the branch actually running: prony_seed(grid, y) == prony_seed(grid[:head], y[:head]), bit for bit, because that is literally what the prefix path returns. * M6/M7 Lindblad "ROT DURCH ABSTURZ:ValueError -- kein Beleg". Taking the allowance out makes the builder raise, so the test died at the exception rather than at an assertion, and a red that comes from an uncaught throw does not say which condition failed. Both builder calls now catch wide and assert on the TYPE with isinstance. Re-run DK-20260903T072811-97b02c2997ed: 5 of 9 discriminate. The remaining four are the intended negative controls and MUST stay blind -- M2 (a genuinely uniform grid), M5 (the not-determinable branch keeps its documented fallback), M8 (the twelfth-round gauge hole stays closed) and M9 (no-op ablation: removing a comment must not turn anything red). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce21a88ce
ℹ️ 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".
…ed open `build_liouvillian` removed the identity component with `np.trace(H).real / d`, which forms the total FIRST. Every entry of `H` can be finite while their sum is not, and an infinite shift makes both the gauge-fixed operator and the round-off allowance derived from it non-finite -- after which the comparison `defect > EPS_HERMITICITY * scale + allowance` is False for EVERY defect. The sparse builder carried the identical calculation, which the finding named explicitly. The shift now divides before summing (`numerics.linalg.overflow_safe_mean_real`). Each term is then bounded by `max|v| / n`, so no partial sum of the pairwise tree can exceed `max|v|` and a finite input has a finite mean -- removed by construction, not by threshold. The scaled form is entered ONLY when the direct sum is non-finite, so the healthy path is bit for bit unchanged; measured over 2000 random complex matrices, `np.trace(H).real` and `np.sum(np.diagonal(H)).real` agree in every bit in every case, and that equality is asserted in the suite rather than argued. Reading the neighbourhood found a SECOND route to the same fail-open that no comment reported. The shift lies between the smallest and the largest diagonal entry, so `H_ii - shift` can reach twice the largest entry and overflow even when the shift itself is finite. Measured on `diag(1.7e308, -1.7e308, 1.7e308)` with an off-diagonal defect: shift 5.67e307, gauge-fixed diagonal -inf, scale inf, accepted. Refusing a non-finite derived scale was tried first and is wrong -- it turns an exactly Hermitian operator of that shape into an error. The comparison is restated at half scale instead: `0.5 * H - 0.5 * shift * I` cannot overflow, and halving every term of an inequality is exact in binary floating point, so it is the same predicate and not a looser one. It is entered only when the direct `scale` is non-finite, so the healthy path keeps the original expression literally. Measured, both builders, before -> after: trace overflows, defect 1e300 ACCEPTED -> rejected gauge-fixed scale overflows, defect 1e300 ACCEPTED -> rejected trace overflows, exactly Hermitian ACCEPTED -> ACCEPTED gauge-fixed scale overflows, exactly herm. ACCEPTED -> ACCEPTED round-18 fixture 1e9*I + defect 1e-6 rejected -> rejected KNOWN RESIDUAL, deliberately not fixed here and raised separately: the reviewer's literal `[[1e308, 1], [0, 1e308]]` is still accepted. Its defect of 1 is below `d * eps * |shift| = 4.4e292`, the round-18 allowance for the removed identity component -- an independent mechanism with its own recorded justification, not the overflow this finding is about. Capping that allowance at the gauge-fixed scale is a threshold decision with anchor consequences. Baseline on this branch: 1070 passed at 3ce21a8, ruff and mypy clean before and after; 88 passed across the builder-adjacent files after the change. Reverse-mutation run DK-20260903T232909-4fc3b87f584f, 5 of 5 discriminate. The no-over-reject control was rewritten mid-run because the tool refused it: it died of the raised ValueError rather than of an assertion, and a test that dies of an exception cannot be attributed to the mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTsogBNZbmP5ajW765Gei7
Status 2026-09-07 — four open threads, all four judged; two repaired upstream, two deliberately openThread counts measured by GraphQL: 20 threads, 12 resolved, 4 open-but-outdated, 4 genuinely open. All four now carry a written verdict.
I am saying plainly which two I did not get to, rather than leaving the count ambiguous.
|
…against H alone The round-18 allowance d*eps*|gauge_shift| and main's gauge-fixed relative gate disagree on two review-pinned fixtures, and neither can separate them: for every H = c*I + N with N strictly upper triangular the relative defect defect/gauge_scale is exactly 1, and a gauge shift plus a change of units maps both I + 2**-53 e01 (must accept) and 1e308*I + 1e290 e01 (must reject) onto e01. The separating information is in the dissipator, so the defect is now compared against max(max|H_gauge|, max|sum gamma L^dag L|/2), the scale of H_eff = H - iK/2. With no dissipation this is main's gate verbatim; the allowance and its recorded gauge-dependent residual are gone. Tests: new file, 11 collected; 8 red on the allowance code (2 of them by a crash of the test's own premise, repaired here and re-proved separately). Blockwise commit (Codie, Liouscope #127 unblock), full suite follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
…round-18 residual closed Docstring-only notes in the round-19/20 review tests (no assertion changed) so the recorded "known residual" no longer reads as current behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
… reconciled per hunk 28 conflict hunks in 12 files, resolved one by one (merge, not rebase: a rebase would rewrite ~27 reviewed commits and need a force-push). - Hermiticity gate (lindblad.py, sparse/build.py): PR #127's generator- relative tolerance kept; with no dissipation it is main's gauge-fixed gate. The half-scale restatement is kept instead of main's refusal of a non-finite scale, because #127 round 20 pins an exactly Hermitian diag(1.7e308, -1.7e308, 1.7e308) as accepted. - overflow_safe_mean_real: the expected overflow of the direct sum no longer warns. Four main tests (PR #121 rounds 19/22) died of that RuntimeWarning under filterwarnings=error once the branches met. - Zero-mode applicability (linalg.py): main's inline conditions (non-finite fro, componentwise #130, no tiny floor) plus #127's derived cutoff sqrt(d)*bound as an additional refusal term; tiny floor removed from the helper for the round-23 reason. - relaxation/gls/bootstrap/_types: both sides' fields kept; k counts the CAR(1) theta (#127), non-succeeded fits are not selectable (#121), the log-space likelihood (#135) carries the CAR(1)/AR(1) Jacobian (#127). - classification/spectral/hypothesis tests: the same finding fixed on both sides; main's text kept, #127's extra matrix test kept alongside. Targeted set 353 passed; full suite follows as the evidence run (~11 min). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
…indblad gauge; refuse a non-finite scale again Equalita round-2 findings, reproduced before the change (10 collected, 5 red by assertion): - P1: a null dissipator 2**20*I (D[cI] = 0 exactly) was read as a dissipation scale 2**39 and excused an order-one defect. - P6: (H, L) and (H + (c/2i)(L - L^dag), L + cI) are one generator; the old scale saw c twice -- in L^dag L and in the coherent scale of the compensated H. Making only the jump operators traceless would have left the second route open (defect 1e-6 variant), so both scales are now read in the canonical gauge: L0 = L - tr(L)/d I and the compensated H0. The compensating term is Hermitian, the defect is still measured on H. - P4: the round-20 half-scale restatement accepted diag(1.7e308, 1.7e308, -1.7e308) and returned a generator containing inf (an overflowing gauge-fixed diagonal means an overflowing H_jj - H_kk, an entry of -i[H, .]). main's refusal of a non-finite scale is restored; the round-20 pin that required acceptance is reversed and the fixture now asserts the refusal, with the overflow measured in its premise. - New test pins the factor 1/2 of the dissipation scale from both sides (a mutation dropping it was 0 red before). The open physics question E3 (may large physical dissipation excuse a coherent defect at all) is isolated to one max() expression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
… for the generator-relative gate - "exactly 1 for every H = cI + N" restricted to exact arithmetic / an exactly computed shift (Equalita C1b: d=3, c=0.1, N=1e-30 gives 7.2e-14); the earlier "20,000 cases, maximum deviation 0.0" figure is withdrawn -- that probe silently skipped every case with an inexact shift. - "equals main without dissipation" made precise (shift formula can differ in the last bit); canonical Lindblad gauge, P4 refusal, open E3 stated. - CITATION.cff: #127 entry in the style of #109 (changes which inputs are ACCEPTED, not the numerics of accepted ones). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
…ation, with parity Equalita's mutation OWN_D (gamma dropped from the dense builder's compensation) was 0/202 red: every gauge test ran with unit rates. New test with two jump operators at rates 0.37 and 2.5, each shifted L_k -> L_k + c_k I with the rate-weighted compensation of H (same generator, asserted from the definition). Asserts dense == sparse on both pairs and an unchanged verdict: defect 1e-8 refused, 1e-12 accepted. c = 2**10 because at 2**20 the gamma*c^2 terms cancel only to 4.7e-5 relative (measured) and the premise would not hold. OWN_D rebuilt: red at the parity assertion (dense accepts the shifted pair, sparse refuses); restored: green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
…numpy 2.2 stubs mypy in the py3.10 CI job (numpy 2.2.6, mypy 2.3.1) typed `K = np.zeros((d, d))` as ndarray[tuple[int, int]] and rejected the loop update `K = K + gamma * (L0^H @ L0)`, whose stub type is ndarray[tuple[int, ...]]. Newer stubs (py3.14 locally) accept it, so the local chain was green and CI was not. Reproduced locally with the exact CI versions before the fix (1 error, exit 1); after the annotation mypy exits 0 under both numpy 2.2.6 and the 3.14 env. Type-only change, no runtime behaviour touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4ade4001b
ℹ️ 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".
| if grid_residual_model == "ar1": | ||
| residual_model = "ar1" | ||
| else: | ||
| whitened_car1 = [np.isfinite(fr.residual_theta_car1) for fr in fits.values()] | ||
| if not whitened_car1: |
There was a problem hiding this comment.
When rho_initial == rho_steady_state, every fit_gls_ar1 call returns success=False from the flat-curve guard before selecting or applying a residual model, but this logic reports "ar1" for a uniform grid and "car1_fallback_ar1" for a non-uniform grid because it checks only grid geometry/theta finiteness rather than successful fits. Fresh evidence beyond the earlier metadata finding is that the current tree’s early degenerate return leaves five unsuccessful FitResult entries, so whitened_car1 is nonempty even though no AR(1) fallback occurred; this makes persisted audit metadata claim whitening that was never performed. Derive availability from fr.success first, then classify the family only among successful fits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in stacked PR #154 (bb397ff). Measured on e556c96 for rho_initial == rho_steady_state over the two-scale grid: 5/5 fits success=False, label "car1_fallback_ar1". Availability is now read off FitResult.success first and the family is classified only among successful fits; that run reports "car1_unavailable", and a uniform grid with no successful fit reports the new label "ar1_unavailable" instead of "ar1". Pinned by three tests in tests/test_pr127_review_round21.py (unavailable on both grid types, plus an over-correction control where successful fits whose theta failed keep "car1_fallback_ar1").
Generated by Claude Code
…ATION entries The only conflict was additive: both branches appended a "pending for the next cut" comment block to CITATION.cff (#127 generator-relative Hermiticity gate; #113/#139 traceless restriction and exact per-column summation). Both are kept, #127 first. linalg.py and test_spectral_certificate.py merged automatically; because #139 touched the same numerics, the full suite, ruff and mypy (py3.10 with numpy 2.2.6 + py3.14) were run on the merge result before committing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ab22ade7a
ℹ️ 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".
| tp_rtol * fro, | ||
| np.sqrt(max(d, 1)) * bound, | ||
| ) |
There was a problem hiding this comment.
Fresh evidence after the _zero_mode_applicable fix is a 4×4 operator expressed in a basis whose first vector is vec(I)/sqrt(2), with leading block [[0, 1e-14], [1, 0]] and remaining eigenvalues −1 and −2: its trace defect is 1.41e-14, so this cutoff accepts applicability against a 4.44e-13 bound, but its exact nearest eigenvalues are approximately ±1e-7 and every solver correctly returns certified=False. Distance to a trace-preserving matrix does not bound eigenvalue displacement by the same distance for a nonnormal operator, so approximate imported generators can still be mislabeled as failed eigensolves and force D1 and classification to UNDEFINED; require numerically exact trace preservation or account for eigenvalue conditioning/pseudospectral displacement in the admissible band.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed on the mechanism: distance to the trace-preserving set bounds eigenvalue displacement only through the conditioning of a non-normal operator, so a trace-defect cutoff on _zero_mode_applicable cannot certify by itself. This is the open question of issue #117 (zero-mode conditioning for stiff non-normal generators); your [[0, 1e-14], [1, 0]] fixture with the +-1e-7 true eigenvalues is the concrete case that issue lacked, and I have added it there. Not fixed in this PR: the two routes (require exact trace preservation, or bound the displacement via pseudospectra / eigenvalue condition numbers) change what D1 withholds on imported generators and belong in a dedicated PR with the anchor consequences stated, per AGENTS.md working agreement 3.
Generated by Claude Code
| centred = r - float(np.mean(r)) | ||
| if float(np.dot(centred, centred)) <= 0.0: | ||
| return float("nan") |
There was a problem hiding this comment.
On a non-uniform grid, a residual series can have clear relative variation while this raw sum of squares underflows to zero. For example, the same 40-point residual vector gives theta≈0.0362 at unit amplitude but returns NaN when scaled by 1e-170; _profile_nll also begins drifting at 1e-160 because it squares the innovations directly. fit_gls_ar1 consequently switches to its discrete-AR(1) fallback, changing whitening, effective sample size, AICc/model selection, and bootstrap intervals solely with the residual amplitude. Normalize the residuals before estimation, or use scaled sum-of-squares arithmetic in both this degeneracy test and _profile_nll.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in stacked PR #154 (bb397ff). Reproduced on a 40-point OU series: theta = 0.0885 at unit amplitude, 4e-7 relative drift at 1e-150, NaN at 1e-170. Theta is amplitude-free (the profiled likelihood shifts by n log c^2 under r -> c r), so estimate_car1_theta now normalises the residuals to unit maximum before anything squares them and reads degeneracy off that maximum. After the fix the spread across amplitudes 1e-170 .. 1e150 is 7e-8 relative, which is the estimator's own round-off floor on a flat profile likelihood (the test tolerance is 1e-6: two decades above that floor, three below the pre-fix drift). Existing estimates move only at that floor; anchors unchanged.
Generated by Claude Code
| # exists, and ``default_relaxation_grid`` then keeps the uniform window | ||
| # -- fail-closed, so an unusable spectrum degrades to the historical | ||
| # behaviour plus its warning rather than to an invented timescale. | ||
| fast_rate = fastest_decay_rate(L_super) |
There was a problem hiding this comment.
When the primary complex eigensolver fails but compute_spectral_layer succeeds through its real-driver or Schur repair route, diagnose() reaches this line with an already repaired spectrum and gap, then immediately launches an unguarded raw np.linalg.eigvals solve through fastest_decay_rate. That call can repeat the original failure and abort the relaxation layer, and it can also derive the grid from the rejected primary spectrum rather than the spectrum D1 certified; despite the nearby comment, fastest_decay_rate does not convert solver failure to NaN. Forward the already computed spectral eigenvalues or their maximum decay rate alongside the gap instead of independently solving the same operator again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in stacked PR #154 (bb397ff). Measured with a spy on np.linalg.eigvals: two solves of the 16x16 generator inside compute_relaxation_layer even with gap forwarded (one in fastest_decay_rate, one in the resolution guard). compute_relaxation_layer now takes eigenvalues=, diagnose() forwards SpectralResult.eigenvalues next to the gap, and decay_rates / fastest_decay_rate / samples_per_fast_efolding accept the same keyword; a direct caller who omits both gap and eigenvalues gets both from one spectral-layer call. Zero re-solves in the pipeline now (tested via the spy and via a decay_rates wrapper asserting the spectrum arrives from diagnose()). Passing gap alone keeps the historical fresh solve, so no existing caller changes behaviour silently.
Generated by Claude Code
Cross-family design review — Hermiticity tolerance (E3)Verdict: the current generator-relative Hermiticity reference is not recommended for the ReasonThe structural residual is
For an API parameter documented as a Hamiltonian, Hermiticity is a property of
therefore couples two different contracts: Hamiltonian structure and total-generator timescale. The canonical traceless Lindblad gauge remains useful for generator-representation invariance, but it does not make dissipation an appropriate denominator for the Hamiltonian-structure check. Reference semantics agree with the standard GKSL split and with reference solver APIs that take a system Hamiltonian separately from collapse operators (e.g. QuTiP Recommended changeFor this gate, replace the open E3 expression reference = max(coherent_scale, dissipation_scale)with an H-internal reference (under the already chosen gauge semantics), i.e. Fixture consequenceThe existing fixture Recommended split:
Discriminating regressionAdd a strong, traceless physical dissipator in an unrelated scale/block while holding the same non-Hermitian Option verdicts:
This closes the requested cross-family design judgement. It does not by itself assert that the rest of PR #127 is merge-ready. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 100e1c083a
ℹ️ 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".
| # to ``coherent_scale`` alone -- and nothing else in the gate. | ||
| reference = coherent_scale | ||
| # Written as ``not <=`` so that a NaN defect cannot be accepted either. | ||
| if not defect <= EPS_HERMITICITY * reference: |
There was a problem hiding this comment.
When a physically pure-gauge Hamiltonian is produced by floating-point arithmetic, such as H = Q @ I @ Q.conj().T, gauge fixing leaves only roundoff, making the Hermiticity defect comparable to reference and causing this relative test to reject it. Fresh evidence in c335b67 is a numerical 2×2 unitary fixture with defect 4.45e-17 and gauge-fixed scale 1.52e-16, which now raises here because the new comparison removed the earlier roundoff allowance. Preserve an allowance for roundoff-only gauge removal without allowing a large identity shift to mask an independently supplied off-diagonal defect; the sparse twin needs equivalent handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed, and this is the impossibility result of the E3 review applied, not an omission. Measured on this head with Q = qr(random): Q I Q^H has defect 5.0e-17 against gauge-fixed scale 2.0e-16 (relative 0.22) and is refused; (H + H^H)/2 is accepted.
The allowance you ask to preserve "for round-off-only gauge removal" has to be a function of the identity component, f(|shift|), because the gauge-fixed remainder of such an input IS the round-off. That is exactly the round-18 term d*eps*|gauge_shift|, and it accepted [[1e308, 1], [0, 1e308]] (defect 1 against 4.4e292), whose generator does not preserve Hermiticity. The two inputs cannot be told apart by any predicate that reads H alone and respects H -> H + cI and H -> cH: a gauge shift plus a change of units maps both onto e01. Making the allowance depend on the dissipator instead was the generator-relative reference the cross-family review rejected for an API whose parameter is a Hamiltonian. So the choice is between refusing a numerically pure-gauge H and re-opening one of the two fail-opens; the gate refuses. In stacked PR #154 (bb397ff) the refusal message of both builders now says so and tells the caller to pass the Hermitian part; the dense/sparse twins already agree on this fixture (the round-19 test pins it).
Generated by Claude Code
| t_grid_source=t_grid_source, | ||
| t_grid_span=float(t_grid[-1] - t_grid[0]), | ||
| t_grid=t_grid.copy(), | ||
| samples_per_fast_efolding=fast_resolution, |
There was a problem hiding this comment.
On a grid with three or more separated timescales, _resolution_detail deliberately returns the minimum resolution over all modes, so the value stored here can describe an intermediate mode rather than the fastest one; the added three-scale test demonstrates exactly that case. Publishing only this scalar as samples_per_fast_efolding—whose public contract says it measures the fastest mode—while discarding worst_rate, blind, and blind_start makes the persisted report misattribute which component was missed once the warning stream is gone. Record the offending mode/interval in RelaxationResult or rename the metric and its contract to identify it as the worst-resolved mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in stacked PR #154 (bb397ff). On the three-scale fixture (1e-6, 1e-3, 1) the scalar is 0.00195 and belongs to the 1.001e-3 mode; _resolution_detail knew (rate, blind interval 5.1e5, start 9.99) and only the warning said so. RelaxationResult now carries worst_resolved_rate, worst_resolved_blind_interval and worst_resolved_blind_start (additive, NaN-defaulted), the test asserts they are the values the warning was built from, and the docstring of samples_per_fast_efolding says it is the worst-resolved mode; the name is kept for compatibility rather than renamed. Docs updated in layers-and-taxonomy.md.
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e556c96dc9
ℹ️ 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".
| # measures its defect against the scale of the GENERATOR, not of H alone (PR | ||
| # #127): max(gauge-fixed max|H0|, max|sum_k gamma_k L0_k^dag L0_k| / 2), read in | ||
| # the canonical Lindblad gauge (traceless jump operators, compensated H0). As | ||
| # with issue #109 this changes which inputs are ACCEPTED, not the numerics of | ||
| # accepted ones: a numerically pure-gauge Hamiltonian with a dissipator is now |
There was a problem hiding this comment.
For releases built from this commit, this entry documents a different acceptance contract from the code: build_liouvillian and its sparse twin set the tolerance reference to coherent_scale only, and the new tests explicitly reject non-Hermitian Hamiltonians despite dissipation, while these lines claim the reference is max(coherent, dissipation) and that a dissipator can make a numerically pure-gauge Hamiltonian acceptable. The matching CHANGELOG entry repeats the superseded rule and calls the dissipation question open, so published methodology and rerun guidance would misdescribe which inputs this version accepts; update both records to the settled coherent-component contract.
AGENTS.md reference: AGENTS.md:L99-L100
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in stacked PR #154 (bb397ff). CITATION.cff and the matching CHANGELOG.md entry now record the settled contract: the defect is compared against the coherent scale alone, read in the canonical Lindblad gauge; the dissipation scale is diagnostic; I + 2**-53 e01 with sigma- is refused; the E3 question is closed (cross-family review 2026-09-12: generator-relative rejected, H-only recommended). Both records also state that the generator-relative reference never shipped in a release. Round-21 entries for the other three fixes added under [Unreleased] / Fixed.
Generated by Claude Code
Status 2026-09-14 — review round 21 delivered as stacked PR #154, ready to merge into this branchThread state on this PR (measured via GraphQL): 30 threads, 21 resolved, 9 open. Of the 9 open ones, 5 are answered by code in #154 and 4 stay open on purpose.
PR #154 ( What this PR needs from Marco, in order:
No push to this branch was made by me; #154 is the only delivery. Generated by Claude Code |
3 participants
Summary
Unblocks PR #127. Merges
main(38f8652) and resolves the one factualconflict between the two branches: the Hermiticity gate.
I + 2**-53 e01with a sigma- jump operator as ACCEPTED. PR fix(#118): close the three review findings that survived PR #107 #121 round 19/22pins
1e308*I + 1e290 e01without jump operators as REJECTED. A gauge shiftand a change of units map both onto
e01, so no criterion that readsHalone and respects both symmetries can separate them.
main's gauge-fixedgate rejected both. fix(#122): repair the two-scale relaxation window instead of disclosing it #127's
d*eps*|gauge_shift|allowance accepted both,and it also accepted
[[1e308, 1], [0, 1e308]], whose generator does notpreserve Hermiticity.
His compared against the scale of theGENERATOR,
max(max|H0_gauge|, max|sum gamma L0^dag L0| / 2), read in thecanonical Lindblad gauge. That gauge uses traceless jump operators and a
compensated
H0, so the scale is a function of the generator alone. It isinvariant under
H -> H + c*Iand underL -> L + c*I, and it scales withthe units. Without jump operators it reduces to
main's gate. A non-finitegauge-fixed scale is refused, as on
main.fix(#122): repair the two-scale relaxation window instead of disclosing it #127 fixed the same finding,
main's text is kept. The zero-modeapplicability check is
main's conditions plus fix(#122): repair the two-scale relaxation window instead of disclosing it #127's derived cutoffsqrt(d)*bound.overflow_safe_mean_realno longer warns on its expectedoverflow.
a defect in the coherent part (cross-family review requested). The answer
changes only the one
max(...)that forms the reference.History note (AGENTS.md): this branch was advanced with a merge commit, not a
rebase, so no force-push is needed. That keeps us clear of the history-handling
incident of 2026-05-16, when an unverified branch delete plus GitHub GC wiped
about 20 files that the backup triple then recovered. Pre-SHA of the remote
branch before the push:
e0a6479.Test plan
pytest -q: 1366 passed (702.43 s), exit 0 on 5bc1c44 (local, py 3.14.4)pytest tests/test_anchors.py -v: 21 passed, exit 0;test_anchors.pybyte-identical tomainruff check src tests benchmarks: exit 0mypy src/liouscope: exit 0(allowance restored; no dissipation excuse; no traceless gauge; K
traceless but H uncompensated; factor 1/2 dropped; non-finite refusal
removed)
test 3.10-3.14+qutip-cross-check 3.11/3.12(runs after push)🤖 Generated with Claude Code
https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp