fix(#135): make Gaussian profile likelihood scale-safe - #147
Conversation
Add signed build-provenance attestations and verify release artifacts before PyPI Trusted Publishing. Keep permissions least-privilege and action pins immutable.
fc32bd7 to
561c2bb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba4785f2e3
ℹ️ 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".
…underflowed Three findings of the round-1 external review on PR #147. [gls.py:227] When the whitened residuals carry a finite, non-zero RMS below the smallest positive float64, ``log_rss`` and ``log_sigma`` are both finite and the profile likelihood is computable -- but ``exp(log_sigma)`` underflows to 0.0, and this branch returned success=False, log_likelihood=nan, likelihood_degenerate=True. ``_fit_with_model`` then scored the model inf and dropped it from selection. That is the ABSOLUTE SCALE BOUNDARY issue #135 set out to remove, put back one layer down: the same curve in different rate units either is or is not an AICc candidate. Reproduced at 6ddd053 before any change, on the reviewer's construction: one minimum-subnormal residual (5e-324) among 64 otherwise-zero points gives log_rss = -1488.8801438427624, log_sigma = -746.519513463061 and a profile log-likelihood of +47686.4367955108, while exp(-746.52) underflows because the smallest subnormal is exp(-744.44). The fit now keeps its finite log-space likelihood and stays selectable; only the scale-dependent evidence is withheld. GLSFitOutput gains ``scale_unavailable`` (additive, default False). ``sigma`` is NaN there, deliberately NOT 0.0: ``_ar1_resample`` consumes it as the innovation standard deviation, and 0.0 would draw identical replicates -- a zero-width confidence interval, which is the failure mode of an uncertainty pipeline rather than a conservative one. ``parametric_bootstrap`` refuses on the new flag rather than on ``success``, so "no interval" and "no estimate" stay distinguishable, and ``compute_relaxation_layer`` reports the CI as NaN through its existing RuntimeError handler. [aicc.py:40, twice] The likelihood replacement is a methodology change that can reorder AICc, change the selected model and change the reported rate, and the commit recorded it in neither place AGENTS.md requires. CITATION.cff now carries the pending correction (AGENTS.md:121-123) and CHANGELOG.md an [Unreleased] "Changed" entry for the likelihood itself plus a "Fixed" entry for the underflow repair above (AGENTS.md:195-197). Both state what the change is NOT: only the likelihood path was corrected, model selection is not scale-free in general, and per-model likelihoods are not manifest fields, so the behavioural change is invisible in input_hash -- archived analyses whose selected model matters must be re-run. Discrimination proven per GUARD LINE, 3/3, run DK-20260902T224741-cc2ae3b913bd (ledger Vero/Data/diskriminierung_runs.jsonl): the underflow detection, the NaN sigma and the bootstrap refusal each turn their test red at an assertion when removed, green again after restore. The first attempt scored ROT DURCH ABSTURZ because the test asserted the warning via ``pytest.warns``, whose failure is not attributable to a guard line; it now records warnings and asserts on them. 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: cc2cd2c7cb
ℹ️ 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".
…refused by arithmetic Round-2 external review of PR #147. - _types.py / relaxation.py: FitResult carries `scale_unavailable` and `_fit_with_model` copies it. Without it the persisted report showed only `bca_ci_beta = (nan, nan)` -- the value ANY bootstrap or jackknife failure produces -- and the reason lived in a warning the artefact drops. - aicc.py: the standardised RSS enters as `-0.5 * RSS`, so its representable range reaches `2 * float64.max`. The guard compared against `log(max)` and returned -inf for the octave above it; the bound now carries `+ log(2)` and the exponential is taken after subtracting `log(2)` where that is needed. Below that octave the arithmetic is untouched, so no existing likelihood moves by even one ulp (pinned by a bit-identity test). Beleglauf folgt (volle Suite, rund 5 min). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
…OverflowError The mutation run reported ROT DURCH ABSTURZ:OverflowError for this control: without the guard, math.exp raises before any assertion runs, so the death was not attributable and the case did not count as a proof. The outcome is captured and its TYPE checked with isinstance instead. 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: cee633f9a4
ℹ️ 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".
…n two logs
Thread PRRT_kwDOSXoNQ86evTEG, src/liouscope/fitting/aicc.py:90.
Round 2 widened the bound to log_max + log(2) and the boundary was still
decided in LOG SPACE, where at this magnitude it cannot be decided.
Measured, sigma=1 with a single residual sqrt(float_max)*sqrt(2):
log_rss = 710.475860073944
log_max + log(2) = 710.4758600739439 -- exactly ONE ulp lower
so the guard returned -inf, although the quantity the formula actually forms
is 1.7976931348623155e+308 = 0.9999999999999999 * float_max, finite.
The obvious smaller repair does not work either, and the code and a test now
say so: exp(log_rss - log(2)) raises OverflowError on this same input,
because one ulp of a logarithm near 710 is a factor of ~1e-16 in the value --
exactly the width of the decision being made.
So the halved sum is MATERIALISED from the residuals: halving each term
before accumulating keeps every partial sum inside float64 whenever the true
half-RSS is, and float64 addition itself decides the boundary. There is no
rounded threshold left to be one ulp wrong about, and an overflow is now a
measurement rather than a prediction.
Verified after the change:
reviewer's fixture -> -1.7976931348623155e+308 (was -inf)
4x float_max -> -inf
1e3x float_max -> -inf
64 residuals overflowing only in the SUM -> -inf (a per-term bound would
have missed this one)
ordinary/sigma=None/empty/nan/inf/all-zero inputs unchanged
The lower octave keeps 0.5 * exp(...) untouched, so no previously computable
likelihood moves by one ulp -- the promise round 2 made.
Baseline pr147-work @ cee633f: 1094 passed, exit 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
…teness
Follow-up inside the same finding, found by the mutation run
DK-20260903T074723-246b5d512bf2, which reported the finiteness check BLIND.
Measuring why exposed a latent state collapse rather than dead code:
half = inf -> without the check the subtraction already yields -inf
(measured), so the check is provably equivalent there
half = nan -> without the check the function returns nan; WITH a blanket
isfinite() check it would return -inf
and those are different answers. This function returns NaN by contract for
"model-selection likelihood unavailable"; -inf means "dropped from
selection". A guard that silently turns the first into the second is the
same substitution this whole review wave is about.
NaN is unreachable on this path -- a NaN residual is caught by the
isnan(log_rss) gate above, and every term 0.5*s*s is non-negative -- so the
condition now states the invariant it actually means.
M7 stays BLIND after the change and now provably so, rather than by
accident. 19/19 in tests/test_issue135_likelihood_scale.py, ruff clean,
mypy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FPKDUXBV4qXpWX8ZtpqgAZ
Status 2026-09-07 — the three green checks here are not test coverage, and one widely-held belief about this PR is wrongTwo separate things, and it matters that they are separated. 1. The head has never been tested. Confirmed.Three green checks, none of which runs a test. Cause, from on:
push: { branches: [main, "claude/**"] }
pull_request: { branches: [main] }This branch is The last real matrix on this branch ran at 2. Correcting a claim that has been circulating: the matrix at
|
bf6b33b
into
chatgpt/issue-130-scaled-norms
… representable The #124 rescaling divides the optimiser residuals by max|y|. SciPy's finite-difference probes step in absolute parameter units, so for a tiny scale the rescaled residual, Jacobian or cost leaves float64; least_squares then raised or failed and the fit was reported unsuccessful. That broke the #147 contract (an unrepresentable MLE scale withholds only the CI, not the fit): tests/test_issue135_likelihood_scale.py failed 2/21 after main was merged in, and _fit_with_model("M0") regressed vs main at 1e-150 and 1e-310. The rescaled solve now runs under np.errstate(call=...); any FP exception or non-finite cost/fun/jac sends the iteration to the raw residuals (exactly main's problem) with a RuntimeWarning. A finite rescaled solve that did not converge is still a failure. Adds a Jacobian-level regression test. Full-suite evidence run follows this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oBqQsooNFqonGmyBd9Mp
Scope
Focused statistical/numerical fix for #135, stacked only on the fully exact-head-verified #136 (
59f861a571c5ee06d3c5aa2917079fe10eb22641) so the xLASSQ-style scaling primitive is reused rather than duplicated.Statistical contract
1e-30variance floor from Gaussian profile likelihood;log(RSS)space;r -> c r, each non-degenerate model log-likelihood shifts by the common-n log|c|, preserving pairwise delta-logL and AIC/AICc ordering;sigmakeeps exact-zero residuals valid with finite likelihood.Numerical contract
scaled_log_sum_squares()reuses #136's exact power-of-two scaled accumulation and never materializes RSS. This avoids both underflow and overflow even whensum(r**2)is outside the float64 range.Downstream fail-closed propagation
GLSFitOutput.likelihood_degenerateis propagated intoFitResult; such fits havesuccess=False, AICc is non-selectable, and parametric bootstrap refuses to resample around the non-estimate.Regression/discrimination suite
tests/test_issue135_likelihood_scale.pycovers:1e-150 .. 1e150observable scaling;FitResultand non-selectability.Change boundary
Exactly seven files differ from #136; no classifier, spectral threshold, N_eff/AICc correction, or #128 anchor calibration is changed here. #128 remains a separate estimand/information-definition problem.
Exact-head verification
Final scientific head:
561c2bb7bf8653e229f552dfea945a3a03e10c6d(one commit directly above #136).Verification-only PR #148 targeted
mainbecause repository PR workflows are configured formainonly. On this exact head all evidence families completed successfully:The first candidate head failed before tests solely on Ruff I001 import ordering; the import block was minimally corrected, the POSIX EOF newline restored, and the final tree was re-squashed to the single commit above before this exact-head matrix was run.
This is slice-level evidence for #135; it is not evidence for unrelated stacked branches.