From be28b9cc5aa0ae1902ab34bd2c1b176993f36723 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 17:29:41 +0000 Subject: [PATCH] =?UTF-8?q?prompt:=20consolidate=20PyAutoMind#189=20into?= =?UTF-8?q?=20main=20=E2=80=94=20one=20phase-1=20record,=20follow-ups=20fi?= =?UTF-8?q?led?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sessions independently wrote a completion record for the prior-support Clipper. #189 was opened first, by the session that actually shipped PyAutoFit#1477; #190 (mine) landed later because I never listed open PRs before starting. This consolidates them so main holds one record, not two. complete/2026/08/prior-support-clipper.md is now the union. The detailed body comes from #189 and is materially richer than what I wrote: the bound-kind design decision (why one relative margin is wrong in two silent ways), eight traps measured against a running install, the verification log (bit-identity 10/10, core promise 8/8, guards verified by inversion), and the process lesson -- a first commit shipped an undefined `optimize` in LBFGS._fit and the full 1790-test suite passed against it, because nothing in the suite executes an LBFGS fit. My "what shipped after" section is appended, since #189 predates #1478/#1479/#1480 and so records follow-ups 1 and 2 as open when they are now fixed. #189's phase-2 harness traps salvaged into the campaign prompt -- the .completed short-circuit, fit() rebuilding search.paths so instance-level patches are discarded, search_internal being deleted on success, seeding both random AND numpy, and a box containing the optimum never exercising the clipper. Also that arm 3 does not exist yet (phase 1 ships the mask, no reset) and two phase-1 measurements to carry in as priors. Follow-ups 3 and 4 filed as prompts rather than left in a record: loggaussian_prior_declares_own_support.md and clipper_in_search_identifier.md. The second is Autonomy: human-required -- both answers orphan or collide someone's stored results, and it is best decided BEFORE phase 3 so the re-baseline and any re-keying are not entangled. One correction absorbed from #189: test_nautilus single_core_builds_no_pool PASSES in CI and fails only in local venvs (both 3.12 and 3.13, two independent sessions). "Not caused by this work" is right; "pre-existing on clean main" -- the looser phrasing used in #1479 and #1480's bodies -- is wrong. Recorded. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FzF2XmKQaqZRWZfZMxvTNR --- complete/2026/08/prior-support-clipper.md | 313 ++++++++++++------ complete/index.md | 2 +- dashboard.md | 14 +- .../loggaussian_prior_declares_own_support.md | 73 ++++ .../autofit/clipper_in_search_identifier.md | 92 +++++ .../autofit/clipper_validation_campaign.md | 53 ++- 6 files changed, 444 insertions(+), 103 deletions(-) create mode 100644 draft/bug/autofit/loggaussian_prior_declares_own_support.md create mode 100644 draft/feature/autofit/clipper_in_search_identifier.md diff --git a/complete/2026/08/prior-support-clipper.md b/complete/2026/08/prior-support-clipper.md index 3918e507..a00f9427 100644 --- a/complete/2026/08/prior-support-clipper.md +++ b/complete/2026/08/prior-support-clipper.md @@ -1,101 +1,224 @@ - library-prs: https://github.com/PyAutoLabs/PyAutoFit/pull/1477 - merge-commits: PyAutoFit `1f4b66a937e0012a99b078b1c8b85c52aaac7f0d` (2026-08-16) - issue: PyAutoFit#1476 (closed by the PR) -- summary: Added `AbstractClipper` / `ClipperNone` / `ClipperPriorBox` in - `autofit/non_linear/clipper.py`, modelled on `initializer.py`, giving the - gradient searches search-agnostic enforcement of prior support. Wired opt-in - into `AbstractMultiStartGradient` (imperative `project` after - `optax.apply_updates`) and `AbstractBFGS` (declarative `bounds` handed to - scipy). `project` returns the clipped mask alongside the vector so a caller can - zero optimiser momentum along clipped directions. This is **phase 1** of the - three-phase plan; phase 2 is the validation campaign in - `draft/feature/autofit/clipper_validation_campaign.md`, and phase 3 (flipping - the default) must not be written until phase 2 has run. -- validation: 22 tests in `test_autofit/non_linear/test_clipper.py` across five - classes — `TestBoundsExtraction`, `TestOrdering` (the parameter-ordering - correspondence the prompt flagged as load-bearing and silent if wrong), - `TestProject`, `TestClipperNone`, `TestSearchWiring`. Bit-identity under the - default was verified across 10 randomly generated models on **both** searches. -- release: not performed; the merged PR remains in the pending-release queue. - -## The problem it fixes - -The gradient searches step in physical parameter space against -`fom = -2 * (log_likelihood + sum(log_prior_list))`, with nothing constraining a -step to the prior box. A `UniformPrior` is `-inf` outside its limits, so a lane -that oversteps a hard prior edge reads as non-finite and is marked dead. - -The failure is worse than "frozen". Because `log_prior = -inf` is *constant* -outside the box, its derivative is zero, so the total gradient is the finite -**likelihood** gradient and `optax.apply_if_finite` never fires. The dead lane -keeps stepping for the rest of the run at full likelihood-and-gradient cost with -its output discarded, wandering far. On the reference `imaging/mge` cell that was -60.25% of lane-steps and 14 of 16 lane deaths — **while the likelihood never went -non-finite once** (autolens_profiling#128). - -## The inset is keyed on bound kind, never on unguarded `upper - lower` - -The finding most likely to be re-derived painfully if lost: - -- **two-sided finite** → relative margin. Not needed for prior support (those - bounds are inclusive) but to avoid parking a lane on a prior edge where the - model's own transforms are singular, as the interior start band already avoids. -- **unbounded** → no inset and no width arithmetic, since `-inf + inf` is `NaN` - and clipping against `NaN` bounds destroys the coordinate *and* the objective. -- **half-open and exclusive** (`LogGaussianPrior`'s `0`) → absolute - `strict_epsilon`, a relative margin being identically zero there. - -## Two traps paid for - -- **`LogGaussianPrior` reports `(-inf, inf)`** though its support is `(0, inf)`. - The real support is declared in the clipper, deliberately leaving the shared - `Prior` class untouched — changing it there would silently alter the objective - for the nested samplers, where the hard box currently works correctly. -- **scipy reads a `(lower, upper)` tuple as a sequence of `(min, max)` pairs**, - silently mis-fitting two-parameter models. The BFGS wiring therefore builds an - explicit `optimize.Bounds`. Plain `BFGS` *ignores* bounds behind a - `UserWarning` rather than rejecting them, so a real clipper on a - non-bound-supporting method raises instead. - -Also corrected the `AbstractMultiStartGradient` class docstring, which claimed -the rule steps on the unit-cube parameterization; `_broad_starts` maps draws to -physical parameters. - -## Default is `ClipperNone` and the change is bit-identical under it - -Deliberate, and it follows the precedent of PyAutoFit#1475. Flipping the default -shifts every stored multi-start benchmark, which is the comparability argument -PyAutoFit#1472 made when deferring its own policy change. That flip is phase 3, -and it is gated on the phase-2 re-baseline. - -## What this does NOT fix — carried forward - -- **Lane deaths from NaN gradients.** The prototype left 5/16 lanes dying on the - NaN-params path; clipping is not the mechanism for those. -- **Lanes ending pinned to a bound.** The prototype left 5/16 pinned because - parameters were projected while Prodigy's accumulated state kept pushing - outward. `project` returning the mask is what lets a caller fix this; nothing - yet *uses* the mask to reset momentum. That is a phase-2 arm. -- **The `ell_comps` trapping**, which the prior-exit deaths were masking — see - `draft/research/autolens_profiling/ell_comps_trapping_unmasked.md`. -- **The clip count is counted but not surfaced.** `n_clipped_lane_steps` is - accumulated per-lane (`multi_start_gradient/search.py:863`) and written into - `search_internal`, but `search.summary` carries none of it, and the LBFGS path - produces no count at all since scipy enforces the bounds. See - `draft/feature/autofit/clipper_usage_in_search_summary.md`. - -## Two incidental bugs surfaced by it, both still open - -Both appear on a code path the reference cell had apparently never taken, because -they need lanes to *survive*: - -1. `draft/bug/autofit/save_json_numpy_scalar_typeerror.md` — `float32` is not - JSON serializable and `paths/directory.py:80` is a bare `json.dump`. It fires - at the END of a successful run, so it starts firing exactly when the fix works. - **Confirmed still present at `1f4b66a`** — phase 1 did not fix it. -2. `draft/bug/autofit/crashed_run_poisons_resume.md` — the truncated file that - crash leaves makes the next same-named run resume into it and report a - zero-step no-op as a clean result. +- summary: Shipped `AbstractClipper` / `ClipperNone` / `ClipperPriorBox` in + `autofit/non_linear/clipper.py`, the search-agnostic prior-support enforcement + the `mge-lane-death` investigation (autolens_profiling#128) asked for. Phase 1 + of three; phase 2 is `draft/feature/autofit/clipper_validation_campaign.md`. +- validation: 22 tests, CI green on 3.12 / 3.13 / docs, plus the out-of-suite + verification logged below. +- release: not performed; merged PR remains in the pending-release queue. + +> **CONSOLIDATED 2026-08-16.** Two sessions independently wrote a completion +> record for this task. This file is the union. The detailed body below — +> the bound-kind design decision, the eight measured traps, the process lesson, +> the verification log and the phase-2 harness traps — comes from the session +> that actually shipped PyAutoFit#1477 (PyAutoMind#189, closed in favour of this +> file). The "What shipped after" section is from the session that did the +> follow-up work. Nothing was dropped in the merge. + +Shipped the search-agnostic `Clipper` — prior-support enforcement for the +gradient searches, as the pluggable sibling of `Initializer`. This is follow-up +(1) owed by the `mge-lane-death` investigation (autolens_profiling#128), which +found the MGE lane deaths are the **prior** term, not the likelihood. + +- issue: PyAutoFit#1476 +- pr: PyAutoFit#1477, **MERGED** 2026-08-16 as `1f4b66a` (squash), +798/-3 over + 7 files. Full CI green on both runs (`unittest` 3.12, `unittest` 3.13, + `docs / docs-build`). + +## What shipped + +`AbstractClipper` / `ClipperNone` / `ClipperPriorBox` in a new +`autofit/non_linear/clipper.py`, wired **opt-in** into the two exposed searches: +`AbstractMultiStartGradient` projects after `optax.apply_updates`; `AbstractBFGS` +hands box bounds to scipy (`L-BFGS-B` supports them natively — they were simply +never passed). `project` returns the clipped **mask** as well as the vector, so a +caller can zero optimiser momentum along clipped directions. + +Default is `ClipperNone` and the PR is **bit-identical** with it. Flipping the +default is PR 2 — `draft/feature/autofit/clipper_validation_campaign.md`, now +unblocked. + +## The design decision worth remembering: inset by BOUND KIND, never by width + +The obvious implementation, one relative `margin * (upper - lower)`, is wrong in +two separate and silent ways. Both stem from computing the width unconditionally. +Three cases instead: + +| Bound kind | Example | Inset | +|---|---|---| +| two-sided finite | Uniform, LogUniform, TruncatedGaussian | relative `margin * width` | +| unbounded | Gaussian | **none, and no width arithmetic at all** | +| half-open, exclusive | LogGaussian's `0` | absolute `strict_epsilon` | + +And note the two-sided margin is **not** for prior support — measurement showed +those bounds are *inclusive* (`UniformPrior.log_prior(2.0) = 0.0`, +`TruncatedGaussian.log_prior(1.0) = -0.5`), so `margin=0` would be valid. It +exists to avoid parking a lane exactly **on** a prior edge, where the model's own +transforms are singular — the same reason the broad-start band defaults to the +interior `(0.15, 0.85)` rather than `(0, 1)`. + +## Traps, all measured against a running install + +1. **The naive margin turns every unbounded prior into `NaN`.** `-inf + (inf - + -inf) * m` is `NaN`, and clipping against `NaN` bounds destroys the coordinate + and the whole objective (`sum(log_prior) = nan`). This would have made the + feature **actively harmful** on exactly the models it targets — the MGE + reference model carries `GaussianPrior`s — with a symptom indistinguishable + from the bug being fixed. Every bit-identity test still passes against it, + because `ClipperNone` never computes a margin. +2. **scipy reads `bounds=(lower_array, upper_array)` as a sequence of `(min, + max)` PAIRS.** At n=2 it returns a silently wrong fit (`[0.,1.]` where the + answer is `[1.,1.]`), no error, no warning; at every other n it raises + `ValueError`. So it fails loudly for most models and silently for + two-parameter ones. Build an explicit `optimize.Bounds`. This was the + *prompt's own* specified return type. +3. **`LogGaussianPrior` misreports its own support.** Its `TransformedMessage` + defaults limits to `±inf` and is never passed any, yet `log_prior_from_value` + is `-inf` for `value <= 0`. Declared in the clipper, prior class untouched. +4. **Plain `BFGS` does not reject bounds — it IGNORES them** behind a + `UserWarning` and returns the unconstrained optimum. "Guard or warn" is too + weak; raise. +5. **`prior.lower_limit` resolves for every prior type** via `Prior.__getattr__` + delegating to the message (`AbstractMessage` defaults `±inf`). No type switch + needed — except for trap 3. +6. **The NumPy and JAX paths disagree on support.** + `UniformPrior.log_prior_from_value` is `if xp is np: return 0.0` — + unconditional, no bound test. Only the JAX branch walls off the box, so LBFGS + is exposed only in its `analysis._use_jax` branch. +7. **float32 makes the box check asymmetric.** `2.0000001` is not representable + distinctly from `2.0` and reads as in-box, while `-1e-7` against a lower bound + of `0.0` is caught. A test asserting "overshoot is detected" must use a bound + near zero or float64, or it passes vacuously. +8. **The `AbstractMultiStartGradient` class docstring was factually wrong** — + claimed the rule steps "on the unconstrained (unit-cube) parameterization" + while `_broad_starts` maps draws to physical. That is the sentence that would + tell the next reader this class of bug cannot exist. Corrected. + +## The process lesson: a green suite is not coverage + +The first commit shipped an **undefined `optimize` in `LBFGS._fit`** — any real +`LBFGS.fit()` raised `NameError`. The **full 1790-test suite passed against it**, +because nothing in the library suite ever executes an LBFGS fit. It was caught +only by a randomised end-to-end stress run, after the code was already pushed. +Fixed in the second commit with a smoke test that runs a real `LBFGS.fit()`, +verified to fail with exactly that `NameError` if the import is removed again. + +When a change touches a path, check whether anything actually *executes* it +before trusting the suite. + +## Verification performed (beyond the committed tests) + +- **Bit-identity 10/10 on both searches** across randomly generated models mixing + every prior type — `no clipper arg` vs explicit `ClipperNone`. +- **Core promise 8/8** — with `ClipperPriorBox`, final `sum(log_prior)` finite + every time, lane deaths **0 in every case** vs 62–96 without. +- **End-to-end**: Gaussian fit with the truth outside the box, lane deaths + **249 → 0** with 252 clips; the clipped run pins `centre` at the upper bound, + which is the correct MAP answer under a prior excluding the truth, and an + independent reproduction of the momentum pinning. +- **Guards verified by inversion** — patching back to the naive width form makes + 5 tests fail, including both named regression guards. +- **Resume path** — a `search_internal` lacking `n_clipped_lane_steps` resumes + without `KeyError`. +- **Identifiers unchanged** — real fits produce a single identifier dir shared by + `no clipper` / `ClipperNone` / `ClipperPriorBox`, so existing on-disk results + are not orphaned. Flip side: two runs differing only in clipper currently + COLLIDE on one output dir — matters for PR 2's re-baseline. + +## Harness traps that cost time (for whoever writes the PR 2 measurements) + +- **`.completed` marker short-circuits `fit()`** — a resumed or re-run search + returns the cached result without entering `_fit`. Three successive versions of + a resume test "passed" while testing nothing. Also bites when a script is + re-run with stale output from its previous execution. +- **`fit()` rebuilds `search.paths`**, so an instance-level monkeypatch on + `paths.save_search_internal` is silently discarded. Patch at CLASS level. +- **The search_internal folder is deleted on successful completion**, so it + cannot be read back after the fit — capture it as it is written. +- **Two identically-constructed searches did not resolve to the same identifier + dir**, so "resume" silently started fresh. The reliable method is patching + `DirectoryPaths.load_search_internal` at class level. +- **Seed `random` AND `numpy` before every fit** — the initializer draws from + both, and an unseeded comparison reports a spurious bit-identity mismatch. + (This produced one false alarm on the bit-identity gate.) +- **A box containing the optimum never exercises the clipper.** The first + efficacy attempt measured 0 clips for exactly this reason; put the truth + outside the box. + +## Corrections issued + +`test_nautilus.py::test__single_core_builds_no_pool` **passes in CI**. It failed +only in the local py3.12 venv used for verification. It was correctly identified +as not caused by this task (verified by stashing), but was wrongly described as +"pre-existing on clean main" in an earlier revision of the PR body and in the +`active.md` notes; both were corrected. + +## Follow-ups owed (filed, not fixed) + +1. `float32` is not JSON serializable in result output — + `autofit/non_linear/paths/directory.py:80` `save_json` raises `TypeError` at + the end of a successful clipped run. Surfaced only because clipping let lanes + survive onto a code path this cell had never taken. +2. A crashed run poisons the next run of the same name: the half-written output + from (1) makes the next search with the same `name` fail with + `JSONDecodeError` while resuming — a 4-second no-op that *looks like* a clean + result. A new form of the cached-result hazard in + `complete/2026/08/multistart-nan-step-diagnostics.md`. +3. Declare `LogGaussianPrior`'s `(0, ∞)` support on the prior itself, retiring + the clipper's special case. +4. Decide whether the clipper should enter the search identifier — relevant to + PR 2's benchmark re-baseline (see "Identifiers unchanged" above). +5. **NUTS remains out of scope** — HMC entering a `-inf` region diverges rather + than freezing. Different mechanism, its own task. + +## Repos / worktree + +- PyAutoFit: `claude/autofit-clipper-prior-support-o3jotv` (merged, deletable). +- No worktree was created — this ran in a cloud session from a direct clone at + `/workspace/pyautofit`. + +## What shipped after this record was first written + +Three further PyAutoFit PRs landed the same day, all merged, all **unreleased**. +Anything running against a PyPI wheel has none of them. + +| PR | merge | what | +|---|---|---| +| #1478 | `bbceff6` | `Clipper`, `Clipped Lane-Steps`, `Clipped Lane-Step Rate` and `Constrained Lane-Steps` reported in `search.summary` | +| #1479 | `b6e89cd` | `NumpyEncoder` — closes follow-up 1 below (the `float32` `save_json` crash) | +| #1480 | `5c9244b` | atomic writes + corrupt-resume recovery — closes follow-up 2 below | + +So **follow-ups 1 and 2 in the list below are now FIXED**; they are left in place +because the reasoning that found them is the record. Their own records are +`complete/2026/08/save-json-numpy-scalar-typeerror.md` and +`complete/2026/08/crashed-run-poisons-resume.md`. Follow-ups 3, 4 and 5 remain +open and 3 and 4 are now filed as prompts: +`draft/bug/autofit/loggaussian_prior_declares_own_support.md` and +`draft/feature/autofit/clipper_in_search_identifier.md`. + +Two corrections that came out of doing that follow-up work: + +- **Follow-up 2's symptom was misdescribed.** Both records originally said the + poisoned rerun is "a 4-second no-op that looks like a clean result". That + **did not reproduce**. What reproduces is a hard `JSONDecodeError` on every + rerun of the same search name. The no-op variant presumably needs a surviving + `search_internal` whose restored `total_steps` short-circuits the loop — and + the crash path deletes that directory first, as noted under the harness traps + above. Do not cite the no-op as observed. +- **`n_clipped_lane_steps` was already in `search_internal` but nowhere else.** + #1478 carried it into `samples_info` and `search.summary`, and found that + `n_constrained_lane_steps` — PyAutoFit#1475's trapped-lane counter — had + reached `samples_info` when it shipped but was never printed, so it had been + invisible in `search.summary` all along. + +On the `test_nautilus.py::test__single_core_builds_no_pool` question raised under +"Corrections issued" above: it is now confirmed from both sessions. It **passes +in CI** and fails in local venvs on both 3.12 and 3.13, with the failure +reproducing on a stashed clean tree. So "not caused by this work" is right; +"pre-existing on clean `main`" is the wrong gloss, and the PR bodies for #1479 +and #1480 use that looser phrasing. Read it as environment-specific. ## Original prompt diff --git a/complete/index.md b/complete/index.md index f35c79bf..2e4e2096 100644 --- a/complete/index.md +++ b/complete/index.md @@ -93,7 +93,7 @@ _(curate hard-won records here — survives regeneration.)_ - [potential-correction-validation](2026/08/potential-correction-validation.md) - [power-law-omega-convergence](2026/08/power-law-omega-convergence.md) — Bounded the fixed 20-term JAX PowerLaw omega recurrence across the packaged slope and ellipticity priors, meas… - [pr-ci-for-own-test-suite](2026/08/pr-ci-for-own-test-suite.md) -- [prior-support-clipper](2026/08/prior-support-clipper.md) — Added `AbstractClipper` / `ClipperNone` / `ClipperPriorBox` in +- [prior-support-clipper](2026/08/prior-support-clipper.md) — Shipped `AbstractClipper` / `ClipperNone` / `ClipperPriorBox` in - [profile-validation-resample-recovery](2026/08/profile-validation-resample-recovery.md) — Shipped the approved narrow compatibility fix: invalid profile construction is now both a direct `ValueError` … - [purge-autocti-dataset-1d-overview](2026/08/purge-autocti-dataset-1d-overview.md) — no GitHub issue — the leftover from `autocti-util-dataset-export`, executed on direct human instruction "do th… - [pyautobrain-pr-test-ci](2026/08/pyautobrain-pr-test-ci.md) — auto-closed by the merge diff --git a/dashboard.md b/dashboard.md index 2829b60e..70fd2ea4 100644 --- a/dashboard.md +++ b/dashboard.md @@ -11,16 +11,17 @@ Tasks only — the organism's health lives with the Heart (`/health`), not here. | [In flight](#in-flight) (`active/`) | 8 | | [Parked](#parked) (`parked.md`) | 6 | | [Planned](#planned) (`planned.md`) | 7 | -| [Backlog](#backlog) (`draft/`) | 138 | +| [Backlog](#backlog) (`draft/`) | 140 | Live on GitHub: [open issues](https://github.com/search?q=org%3APyAutoLabs+is%3Aissue+is%3Aopen&type=issues) · [open pull requests](https://github.com/search?q=org%3APyAutoLabs+is%3Apr+is%3Aopen&type=prs) ## Start here -**Highest priority** (filed as `high`) — showing 12 of 32 +**Highest priority** (filed as `high`) — showing 12 of 33 - [pre_build stages untracked files, publishing uncommitted human work](draft/bug/pyautohands/pre_build_stages_untracked_wip.md) — pyautohands · small · supervised · high - [TRIAGE: needs manual review before routing](draft/triage/jax_zero_contour.md) — medium · safe · high +- [Decide whether the clipper belongs in the search identifier](draft/feature/autofit/clipper_in_search_identifier.md) — autofit · medium · human-required · high - [Clipper: demonstrate and validate on the profiling search cells](draft/feature/autofit/clipper_validation_campaign.md) — autofit · medium · supervised · high - [PyAutoLens RTD docs: three-regime restructure (multi_galaxy / group / cluster)](draft/docs/autolens/docs_three_regime_restructure.md) — autolens · medium · supervised · high - [The `ell_comps` trapping was masked, not cleared — characterise it](draft/research/autolens_profiling/ell_comps_trapping_unmasked.md) — autolens_profiling · medium · supervised · high @@ -30,7 +31,6 @@ Live on GitHub: [open issues](https://github.com/search?q=org%3APyAutoLabs+is%3A - [LACosmic per-frame CR masking option + decouple PSF-star pass from](draft/feature/pyautoreduce/lacosmic_cr_option_and_star_pass_decoupling.md) — pyautoreduce · medium · supervised · high - [Cluster package: point-source-default narrative + extended-source follow-up feature](draft/docs/workspaces/cluster_regime_narrative.md) — workspaces · medium · supervised · high - [multi_galaxy package: new regime package in autolens_workspace](draft/docs/autolens/multi_galaxy_package.md) — autolens · large · supervised · high -- [Tune the JAX multi-start optimizers into a standard option (MGE](draft/experiment/autolens_profiling/jax_optimizer_settings_tuning.md) — autolens_profiling · large · supervised · high **Quick wins** (small enough, and safe enough to run unattended) @@ -95,10 +95,10 @@ Scoped but not started; some are not yet prompt files. Full detail in [`planned. ## Backlog -**138** filed prompts, not started. Each section is sorted most-pickable first (priority, then size). +**140** filed prompts, not started. Each section is sorted most-pickable first (priority, then size).
-bug — 40 +bug — 41 - [pre_build stages untracked files, publishing uncommitted human work](draft/bug/pyautohands/pre_build_stages_untracked_wip.md) — pyautohands · small · supervised · high - [Release does not sync __version__ stamps and workspace pins back](draft/bug/pyautobuild/release_version_sync_back_to_main.md) — pyautobuild · medium · supervised · high @@ -115,6 +115,7 @@ Scoped but not started; some are not yet prompt files. Full detail in [`planned. - [jax_grad scripts fail assertions locally that PASS in CI](draft/bug/autolens_workspace_test/jax_grad_local_assertions_fail_but_pass_in_ci.md) — autolens_workspace_test · medium · supervised · medium - [ConstantZeroth regularization is broken twice over — dead code presenting](draft/bug/autoarray/constant_zeroth_broken_dead_code.md) — autoarray · small · supervised · normal - [PyNUFFT dev extra is incompatible with current SciPy on Python](draft/bug/autoarray/pynufft_scipy_pinv2_dev_extra.md) — autoarray · small · supervised · normal +- [`LogGaussianPrior` misreports its own support as `(-inf, inf)`](draft/bug/autofit/loggaussian_prior_declares_own_support.md) — autofit · small · supervised · normal - [`autofit.plot` functions accept `**kwargs` and silently discard them](draft/bug/autofit/plot_functions_discard_kwargs.md) — autofit · small · supervised · normal - [TEST_MODE bypass crashes on ordered-parameter assertion ties](draft/bug/autofit/test_mode_bypass_ordered_assertion_ties.md) — autofit · small · supervised · normal - [python_matrix smoke fails: autofit_workspace searches/mle.py needs optax not in smoke](draft/bug/autofit_workspace/searches_mle_optax_smoke_dependency.md) — autofit_workspace · small · safe · normal @@ -144,8 +145,9 @@ Scoped but not started; some are not yet prompt files. Full detail in [`planned.
-feature — 26 +feature — 27 +- [Decide whether the clipper belongs in the search identifier](draft/feature/autofit/clipper_in_search_identifier.md) — autofit · medium · human-required · high - [Clipper: demonstrate and validate on the profiling search cells](draft/feature/autofit/clipper_validation_campaign.md) — autofit · medium · supervised · high - [Make draft/ staleness detectable — `intake reconcile` measured, and the](draft/feature/pyautomind/draft_staleness_detection_signals.md) — pyautomind · medium · supervised · high - [LACosmic per-frame CR masking option + decouple PSF-star pass from](draft/feature/pyautoreduce/lacosmic_cr_option_and_star_pass_decoupling.md) — pyautoreduce · medium · supervised · high diff --git a/draft/bug/autofit/loggaussian_prior_declares_own_support.md b/draft/bug/autofit/loggaussian_prior_declares_own_support.md new file mode 100644 index 00000000..1f307d67 --- /dev/null +++ b/draft/bug/autofit/loggaussian_prior_declares_own_support.md @@ -0,0 +1,73 @@ +# `LogGaussianPrior` misreports its own support as `(-inf, inf)` + +Type: bug +Target: autofit +Repos: +- PyAutoFit +Difficulty: small +Autonomy: supervised +Priority: normal +Status: formalised + +Filed 2026-08-16. Follow-up 3 owed by the prior-support `Clipper` +(`complete/2026/08/prior-support-clipper.md`, PyAutoFit#1477), which worked +around it rather than fixing it. + +## The defect + +`LogGaussianPrior`'s support is `(0, inf)` — `log_prior_from_value` returns +`-inf` for `value <= 0`. But its `TransformedMessage` defaults its limits to +`±inf` and is never passed any, so the prior **reports** `(-inf, inf)`. + +Every other prior answers `lower_limit` / `upper_limit` truthfully via +`Prior.__getattr__` delegating to the message, which is why the `Clipper` needs +no type switch anywhere else. This one prior is the exception, and it is the +kind of exception that is invisible until something trusts the answer. + +## Why it matters now + +`ClipperPriorBox` **declares the real support in the clipper** rather than on +the prior — deliberately, to avoid touching a shared class late in that task, +and recorded as a follow-up rather than left silent. That special case is +correct but misplaced: any future consumer of `lower_limit` gets the wrong +answer unless it also knows to special-case this prior. + +The general hazard: a bound of `-inf` on a strictly positive parameter means a +consumer will not guard `0`, and `log(0)` / a division by it is the failure that +follows. + +## The fix + +Declare the support on `LogGaussianPrior` itself — pass the limits into the +`TransformedMessage`, or override `lower_limit` — then retire the clipper's +special case and its accompanying comment. + +## The care needed — why this is `supervised` and not `safe` + +Changing what a prior reports as its support is not local: + +- **The nested samplers work in unit-cube coordinates** and map through the + prior. Confirm a limits change does not alter that mapping, or every stored + nested-sampling result shifts. +- **`log_prior_from_value` must not change behaviour.** It is already correct; + only the *reported* limits are wrong. If the fix changes the density anywhere, + it has gone too far. +- **Check the identifier.** If `lower_limit` feeds the search identifier, a + change re-keys existing output directories and orphans stored results — the + same class of concern as + `draft/feature/autofit/clipper_in_search_identifier.md`. + +## Verify + +- `LogGaussianPrior(...).lower_limit == 0.0` (or whatever exclusive convention + is chosen — state it). +- `log_prior_from_value` is unchanged across a range of values either side of + zero, asserted against the pre-change values. +- `ClipperPriorBox.bounds_from_model` returns the same bounds for a model + containing a `LogGaussianPrior` **after** the clipper's special case is + removed as it did before — that equivalence is the whole point of the change. +- A nested-sampler unit-cube round-trip through the prior is unchanged. + + diff --git a/draft/feature/autofit/clipper_in_search_identifier.md b/draft/feature/autofit/clipper_in_search_identifier.md new file mode 100644 index 00000000..6dd59172 --- /dev/null +++ b/draft/feature/autofit/clipper_in_search_identifier.md @@ -0,0 +1,92 @@ +# Decide whether the clipper belongs in the search identifier + +Type: feature +Target: autofit +Repos: +- PyAutoFit +Difficulty: medium +Autonomy: human-required +Priority: high +Status: formalised + +Filed 2026-08-16. Follow-up 4 owed by the prior-support `Clipper` +(`complete/2026/08/prior-support-clipper.md`, PyAutoFit#1477). + +**`Autonomy: human-required` on purpose.** Either answer has a cost that lands +on someone's stored results, and the trade is a judgement about the project's +data, not a technical detail an agent should settle. + +## The fact + +The clipper does **not** enter the search identifier. Verified against +PyAutoFit `main` on 2026-08-16: + +``` +af.MultiStartAdam(name="x") -> 2bada4747f74bc46bf812605a762def9 +af.MultiStartAdam(name="x", clipper=ClipperNone()) -> 2bada4747f74bc46bf812605a762def9 +af.MultiStartAdam(name="x", clipper=ClipperPriorBox()) -> 2bada4747f74bc46bf812605a762def9 +``` + +Two runs differing only in prior-support enforcement — which can change the +answer, and demonstrably does — share one output directory. + +## Both answers cost something + +**Leave it out (status quo).** Existing on-disk results are never orphaned, and +phase 1 stays back-compatible by construction. But runs that differ in a +result-affecting setting collide, and stacked with the `.completed` +short-circuit a later run silently returns the earlier one's numbers. That is +live right now for the phase-2 campaign, whose arms 1 and 2 differ in *nothing +else* — see +`draft/feature/autofit/clipper_validation_campaign.md`, which works around it +with unique per-arm names. + +**Put it in.** Arms separate naturally and the identifier tells the truth about +what produced a result. But **every existing multi-start and BFGS output +directory re-keys**, so stored results are orphaned — they are not deleted, but +nothing finds them any more. That is the same comparability argument +PyAutoFit#1472 made when deferring its own policy change, and it collides with +phase 3, which wants a benchmark re-baseline it can compare against history. + +## Options, none pre-selected + +1. **Status quo**, with the collision documented wherever it bites. Cheapest; + leaves a footgun that has already been stepped on once. +2. **Include the clipper in `__identifier_fields__`.** Honest; orphans stored + results. +3. **Include it only when it is not `ClipperNone`.** Unclipped runs keep their + existing directories, clipped runs get their own. Back-compatible *and* + collision-free — but the identifier becomes conditional, which is a new + concept in that machinery and needs checking against how identifiers are + consumed (database, aggregator, `.completed` discovery). +4. **Leave the identifier alone and make the collision loud** — refuse to + short-circuit on `.completed` when the search config differs from the one + recorded in the completed run's `search.json`. Fixes the general + cached-result hazard rather than this one instance. + +Option 3 is the obvious-looking compromise and option 4 is the one that closes +the whole class; both need someone to confirm the consumers can take it. + +## Sequencing + +**Best decided before phase 3, not after.** Phase 3 flips the default to +`ClipperPriorBox` and carries a re-baseline; if the identifier changes at the +same time, the re-baseline and the re-keying are entangled and neither can be +read cleanly. Deciding this first — either way — leaves phase 3 with one +variable. + +## Whatever is chosen + +- State the decision and its reason in the record. A future reader hitting a + collision needs to know it was chosen, not overlooked. +- If the identifier changes, say plainly in the release notes that stored + results re-key, and check whether a migration is wanted. +- If it does not change, the phase-2 mitigation (unique `name` per arm) becomes + permanent advice for anyone comparing clipper settings, and belongs in the + `Clipper` docstring rather than only in a campaign prompt. + +## Out of scope + +- Flipping the default (phase 3). +- The `.completed` short-circuit's other manifestations, unless option 4 is + chosen — in which case they are the point. diff --git a/draft/feature/autofit/clipper_validation_campaign.md b/draft/feature/autofit/clipper_validation_campaign.md index f90f30a4..f45bd3ee 100644 --- a/draft/feature/autofit/clipper_validation_campaign.md +++ b/draft/feature/autofit/clipper_validation_campaign.md @@ -91,7 +91,58 @@ Mitigation — do all three: Not a bug in itself — it is what keeps existing runs' directories stable — but it is precisely wrong for this campaign. Whether the clipper *should* enter the -identifier is an open PyAutoFit question, deliberately not decided here. +identifier is now filed as +`draft/feature/autofit/clipper_in_search_identifier.md`, and phase 3's +re-baseline is the reason it matters. + +### Harness traps that already cost a session + +From the phase-1 session, which paid for every one of these +(`complete/2026/08/prior-support-clipper.md`): + +- **The `.completed` marker short-circuits `fit()`** — a resumed or re-run + search returns the cached result *without entering `_fit`*. Three successive + versions of a resume test "passed" while testing nothing. This is the second + half of the collision above. +- **`fit()` rebuilds `search.paths`**, so an instance-level monkeypatch on + `paths.save_search_internal` is silently discarded. **Patch at CLASS level.** +- **The `search_internal` folder is deleted on successful completion**, so it + cannot be read back after the fit. Capture it *as it is written*. +- **Two identically-constructed searches did not resolve to the same identifier + dir**, so an intended "resume" silently started fresh. The reliable method is + patching `DirectoryPaths.load_search_internal` at class level. +- **Seed `random` AND `numpy` before every fit** — the initializer draws from + both, and an unseeded comparison reports a spurious bit-identity mismatch. + This produced one false alarm on phase 1's bit-identity gate. +- **A box containing the optimum never exercises the clipper.** Phase 1's first + efficacy attempt measured 0 clips for exactly this reason. Put the truth + outside the box. + +### Arm 3 does not exist yet — do not silently drop it + +Phase 1 ships the clipped **mask** from `project`, which is what a momentum +reset needs, but **no reset is implemented** and the search does not touch +`opt_state` on a clip. So arm 3 either gets a small addition written first, or +this campaign runs arms 1–2 and reports the pinned-lane count as the input to +that decision. The prototype's 5/16 pinned lanes are why the arm exists. + +### Two phase-1 measurements to carry in as priors + +Both CPU/float32 on a toy Gaussian, so directional only: + +- With the truth deliberately **outside** the prior box, lane deaths went + **249 → 0** with 252 clips, and the clipped run pinned `centre` at the upper + bound. That is "pinning is a result, not a failure" reproducing in miniature, + and an independent confirmation of the momentum-pinning mechanism. +- **The negative control this prompt asks for already passes at unit level** — a + `GaussianPrior` coordinate is provably untouched by `ClipperPriorBox`, with a + regression test guarding it. The cell-level control is still worth running, + but a failure there would point at the cell, not at bounds extraction. + +Also relevant when reading source: the `AbstractMultiStartGradient` class +docstring used to claim the rule steps on the unit-cube parameterization. It +never did — `_broad_starts` maps draws to physical parameters. Corrected in +phase 1, but older checkouts still carry the false sentence. **The clip count is a validity check, not just a statistic.** Read `Clipped Lane-Steps` straight out of `search.summary`. **A `ClipperPriorBox`