|
| 1 | +- issue: https://github.com/PyAutoLabs/PyAutoFit/issues/1533 (closed by the PR's `Closes` line) |
| 2 | +- completed: 2026-08-27 |
| 3 | +- pr: https://github.com/PyAutoLabs/PyAutoFit/pull/1534 (MERGED, merge `5c391fd`, head `1e3b1a0`, |
| 4 | + +90/-1 over 2 files, label `pending-release`) |
| 5 | +- summary: `AbstractMessage.natural_logpdf` clamped a genuine `-inf` to `-1.8e308` because |
| 6 | + `nan_to_num` was called with `nan=-inf` but default `posinf`/`neginf`. Fixed by passing all |
| 7 | + three, so only `NaN` is replaced. |
| 8 | +- validation: 2203 passed / 36 skipped; baseline measured on the branch point `6e2d8c8` with |
| 9 | + changes stashed, 2190 / 36. CI green on all four legs. |
| 10 | +- release: not performed; merged PR sits in the pending-release queue. |
| 11 | +- origin: the loose end noted in `complete/2026/08/optimisation-state-limit-guard-truthiness.md` |
| 12 | + and in PyAutoFit#1532's Shipped comment, filed and shipped the same day. |
| 13 | + |
| 14 | +## The defect |
| 15 | + |
| 16 | +`autofit/messages/interface.py:98`: |
| 17 | + |
| 18 | +```python |
| 19 | +return xp.nan_to_num(log_base + eta_t - log_partition, nan=-xp.inf) |
| 20 | +``` |
| 21 | + |
| 22 | +`nan=-xp.inf` is deliberate — an out-of-support `NaN` is zero density. But `posinf`/`neginf` were |
| 23 | +left at their defaults, and `np.nan_to_num` replaces `-inf` with `-sys.float_info.max`. So the call |
| 24 | +did the **opposite of its intent for the inputs that already had the right answer**: |
| 25 | + |
| 26 | +| value reaching the reduction | intended | actual | |
| 27 | +|---|---|---| |
| 28 | +| `NaN` | `-inf` | `-inf` | |
| 29 | +| `-inf` | `-inf` | `-1.7976931348623157e+308` | |
| 30 | + |
| 31 | +## What made it findable: the asymmetry is the proof |
| 32 | + |
| 33 | +`LogGaussianPrior(0.4, 1.3)` on `main` @ `6e2d8c8`: |
| 34 | + |
| 35 | +``` |
| 36 | +message.logpdf(-1.0) = -inf # log(-1) is NaN -> nan=-inf applies |
| 37 | +message.logpdf( 0.0) = -1.7976931348623157e+308 # log(0) is -inf -> default neginf clamps |
| 38 | +``` |
| 39 | + |
| 40 | +Two out-of-support points, two different answers, from one line. That asymmetry names the mechanism |
| 41 | +without needing to read the reduction — and the clamped value being *exactly* `-sys.float_info.max` |
| 42 | +confirms it, since that is `nan_to_num`'s documented default and nothing else in the stack produces |
| 43 | +it. `UniformPrior` and `LogUniformPrior` return `-inf` correctly, so the bug is invisible unless the |
| 44 | +expression reaches `-inf` rather than `NaN`. |
| 45 | + |
| 46 | +## Why a finite value there is not cosmetic |
| 47 | + |
| 48 | +`-1.8e308` is **finite**, and `isfinite` is the branch: |
| 49 | + |
| 50 | +- `optax.apply_if_finite` — the mechanism `autofit/non_linear/clipper.py` exists to exploit. That |
| 51 | + module's opening docstring says a step leaving the box "makes the objective non-finite". For a |
| 52 | + `LogGaussianPrior` landing exactly on `0.0`, it did not. |
| 53 | +- Two such terms summed overflow to `-inf`; one does not. So the behaviour depended on *how many* |
| 54 | + parameters were out of support — the kind of dependence that makes a bug look like a flake. |
| 55 | + |
| 56 | +## Deliberately left alone |
| 57 | + |
| 58 | +`TruncatedGaussianPrior`'s **message** returns finite `logpdf` well outside its limits (`-8.20` at |
| 59 | +`-1.0` for a `(0, 3)` support). Separate looseness in `TruncatedNormalMessage`, not this clamp; the |
| 60 | +prior-level `log_prior_from_value` is correct there, which is why the P6 property tests pass. The |
| 61 | +new general-property test **excludes it with a comment saying why**, rather than quietly asserting |
| 62 | +something weaker across all families so it would pass. Unfiled. |
| 63 | + |
| 64 | +## Process note: three stale-API misreadings in one session |
| 65 | + |
| 66 | +Worth recording because it cost more time than the fix did. GitHub's `pull_request` check endpoints |
| 67 | +served stale data repeatedly, and I misread it three times: |
| 68 | + |
| 69 | +1. On PyAutoFit#1532, `get_check_runs` reported two legs `in_progress` for ~50 minutes after they |
| 70 | + had finished in ~4. Reported as a stall; wrong. |
| 71 | +2. On PyAutoMind#350, `get_check_runs` reported `total_count: 0` indefinitely. Reported as "no CI |
| 72 | + configured" and escalated to the human; the workflows had in fact run and passed. |
| 73 | +3. On this PR, `list_workflow_jobs` showed one leg complete and two mid-`Run tests` — which I argued |
| 74 | + was *therefore* fresh and trustworthy, and used to diagnose a hang and name my own change as the |
| 75 | + likely cause. It was a stale snapshot caught mid-run; all three passed in under four minutes. |
| 76 | + |
| 77 | +The reliable read is `list_workflow_jobs` **with per-step `completed_at` timestamps**, and the |
| 78 | +tell is the timestamps themselves: a job showing `in_progress` whose sibling steps completed an hour |
| 79 | +of wall-clock ago is a stale snapshot, not a hang. Compare the reported step start against the |
| 80 | +elapsed real time before concluding anything. |
| 81 | + |
| 82 | +## Repos / worktree |
| 83 | + |
| 84 | +- PyAutoFit: `claude/loggaussian-prior-support-ngh59x` (merged, deletable). |
| 85 | +- No worktree — `web-github` against a direct clone. |
| 86 | + |
| 87 | +## Original prompt |
| 88 | + |
| 89 | +# `natural_logpdf` clamps a genuine `-inf` to `-1.8e308` |
| 90 | + |
| 91 | +Type: bug |
| 92 | +Target: priors |
| 93 | +Repos: |
| 94 | +- PyAutoFit |
| 95 | +Difficulty: small |
| 96 | +Autonomy: supervised |
| 97 | +Priority: normal |
| 98 | +Status: formalised |
| 99 | +Filed: 2026-08-27 |
| 100 | +Issued: 2026-08-27 |
| 101 | + |
| 102 | +Loose end from PyAutoFit#1532 (`complete/2026/08/optimisation-state-limit-guard-truthiness.md`), |
| 103 | +noted there and not filed at the time. |
| 104 | + |
| 105 | +## The defect |
| 106 | + |
| 107 | +`AbstractMessage.natural_logpdf` (`autofit/messages/interface.py:98`): |
| 108 | + |
| 109 | +```python |
| 110 | +return xp.nan_to_num(log_base + eta_t - log_partition, nan=-xp.inf) |
| 111 | +``` |
| 112 | + |
| 113 | +It passes `nan=-xp.inf` — deliberate, mapping an out-of-support NaN to zero density — |
| 114 | +but leaves `posinf`/`neginf` at their **defaults**. `np.nan_to_num`'s default replaces |
| 115 | +`-inf` with `-1.7976931348623157e+308` (negative float max). |
| 116 | + |
| 117 | +So the call does exactly the opposite of what it intends for half its inputs: |
| 118 | + |
| 119 | +| expression value | intent | actual | |
| 120 | +|---|---|---| |
| 121 | +| `NaN` | `-inf` | `-inf` ✅ | |
| 122 | +| `-inf` | `-inf` | **`-1.8e308`** ❌ | |
| 123 | + |
| 124 | +A genuine zero-density point is turned into a *finite* number. |
| 125 | + |
| 126 | +## Measured on `main` @ `6e2d8c8` |
| 127 | + |
| 128 | +`LogGaussianPrior(0.4, 1.3)`: |
| 129 | + |
| 130 | +| value | `message.logpdf` | `prior.log_prior_from_value` | |
| 131 | +|---|---|---| |
| 132 | +| `-1.0` | `-inf` | `-inf` | |
| 133 | +| `0.0` | **`-1.7976931348623157e+308`** | `-inf` | |
| 134 | + |
| 135 | +The asymmetry is the proof of mechanism: at `-1.0` the log transform gives `log(-1) = NaN`, |
| 136 | +which `nan=-inf` correctly maps; at `0.0` it gives `log(0) = -inf`, which the default |
| 137 | +`neginf` clamps. Confirmed equal to `-sys.float_info.max` exactly. |
| 138 | + |
| 139 | +`UniformPrior` / `LogUniformPrior` return `-inf` correctly outside their boxes, so this is |
| 140 | +not visible for every family — it needs an expression that reaches `-inf` rather than `NaN`. |
| 141 | + |
| 142 | +## Why it matters |
| 143 | + |
| 144 | +`-1.8e308` is **finite**, and a lot of this codebase branches on exactly that: |
| 145 | + |
| 146 | +- `np.isfinite(logpdf)` is `True` at a point of zero density. |
| 147 | +- `optax.apply_if_finite` — the mechanism `autofit/non_linear/clipper.py` is built around — |
| 148 | + would fire differently. That module's whole premise is that leaving the support makes the |
| 149 | + objective non-finite. |
| 150 | +- Two such terms summed overflow to `-inf`, one does not, so the behaviour depends on how |
| 151 | + many parameters are out of support. |
| 152 | + |
| 153 | +## The fix |
| 154 | + |
| 155 | +Preserve the infinities and replace only `NaN`: |
| 156 | + |
| 157 | +```python |
| 158 | +return xp.nan_to_num( |
| 159 | + log_base + eta_t - log_partition, nan=-xp.inf, neginf=-xp.inf, posinf=xp.inf |
| 160 | +) |
| 161 | +``` |
| 162 | + |
| 163 | +## Verify |
| 164 | + |
| 165 | +- `LogGaussianPrior(0.4, 1.3).message.logpdf(0.0) == -inf`. |
| 166 | +- `logpdf` unchanged at in-support points, pinned against pre-change values across every |
| 167 | + prior family. |
| 168 | +- `log_prior_from_value` unchanged everywhere — it was already correct and must stay so. |
| 169 | +- The JAX path agrees with the NumPy path (`jnp.nan_to_num` takes the same keywords). |
| 170 | + |
| 171 | +## Deliberately out of scope |
| 172 | + |
| 173 | +`TruncatedGaussianPrior`'s **message** returns finite `logpdf` well outside its limits |
| 174 | +(`-8.20` at `-1.0` for a `(0, 3)` support). That is a separate looseness in |
| 175 | +`TruncatedNormalMessage`, not this clamp — the prior-level `log_prior_from_value` is correct |
| 176 | +there, which is why the P6 property tests pass. File separately if it matters. |
0 commit comments