diff --git a/dashboard.md b/dashboard.md index f49e8628..9592a05e 100644 --- a/dashboard.md +++ b/dashboard.md @@ -1,6 +1,6 @@ # PyAutoMind task dashboard - + Every task the Mind is holding, on one page: what is in flight, what is parked, and the whole backlog to pick from. Pick a line, then run `/start_dev ` to start it. @@ -17,11 +17,10 @@ Live on GitHub: [open issues](https://github.com/search?q=org%3APyAutoLabs+is%3A ## Start here -**Highest priority** (filed as `high`) — showing 12 of 31 +**Highest priority** (filed as `high`) — showing 12 of 30 - [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 -- [PyAutoFit mock scaffolding fills every parameter with 1.0, which is](draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.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 - [Optimize pixelized Prodigy settings on the laptop GPU](draft/research/autolens_workspace_developer/pixelized_prodigy_laptop_gpu_phase_2_settings.md) — autolens_workspace_developer · medium · human-required · 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 @@ -31,6 +30,7 @@ Live on GitHub: [open issues](https://github.com/search?q=org%3APyAutoLabs+is%3A - [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 - [Profile and speed up JAX likelihood-function compile times (all use](draft/feature/autolens_profiling/jax_compile_time_profiling.md) — autolens_profiling · large · supervised · high +- [Optimize MultiStartProdigy for pixelized meshes on the laptop GPU](draft/research/autolens_workspace_developer/pixelized_prodigy_laptop_gpu.md) — autolens_workspace_developer · large · human-required · high **Quick wins** (small enough, and safe enough to run unattended) @@ -101,7 +101,6 @@ Scoped but not started; some are not yet prompt files. Full detail in [`planned. bug — 40 - [pre_build stages untracked files, publishing uncommitted human work](draft/bug/pyautohands/pre_build_stages_untracked_wip.md) — pyautohands · small · supervised · high -- [PyAutoFit mock scaffolding fills every parameter with 1.0, which is](draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md) — autofit · medium · 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 - [EP hierarchical parent-scale collapse: cure the basin, or document the](draft/bug/autofit/ep_scale_collapse_basin_cure_or_caveat.md) — autofit · too-large · human-required · high - [`NFWTruncatedSph.potential_2d_from`: MGE potential fails `grad(psi)=alpha` self-consistency](draft/bug/autogalaxy/nfw_truncated_potential_accuracy.md) — autogalaxy · too-large · supervised · high @@ -112,6 +111,7 @@ Scoped but not started; some are not yet prompt files. Full detail in [`planned. - [Fix release-profile numerical inversion failures](draft/bug/health_fixes/numerical_inversion_failures.md) — health_fixes · too-large · supervised · high - [Fix release result/sample parameter-path regressions](draft/bug/health_fixes/samples_parameter_paths.md) — health_fixes · too-large · supervised · high - [`pixel_scales` given as an `int` (or `np.float64`) is never widened](draft/bug/autoarray/pixel_scales_int_not_widened_to_tuple.md) — autoarray · small · supervised · medium +- [PyAutoFit `MockResult` fills every parameter with 1.0, which is an](draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md) — autofit · small · supervised · medium - [Heart script_timing baselines are orphaned by path moves and filled](draft/bug/pyautoheart/script_timing_baselines_orphaned_and_window_filled.md) — pyautoheart · small · supervised · medium - [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 diff --git a/draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md b/draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md index ff8fa75e..04473cf9 100644 --- a/draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md +++ b/draft/bug/autofit/mock_all_ones_parameters_break_ell_comps_guard.md @@ -1,157 +1,231 @@ -# PyAutoFit mock scaffolding fills every parameter with 1.0, which is now an invalid `ell_comps` +# PyAutoFit `MockResult` fills every parameter with 1.0, which is an invalid `ell_comps` Type: bug Target: PyAutoFit Repos: - PyAutoFit -- autogalaxy_workspace_test -- autolens_workspace_test -Difficulty: medium +Difficulty: small Autonomy: supervised -Priority: high -Status: formalised — NOT started. Root cause is narrowed to PyAutoFit's mock - helpers but the exact call site is **not** pinned; see "What is not yet - known". Requires an environment that can run the full autogalaxy stack. +Priority: medium +Status: **shipped** (2026-08-13). PyAutoFit#1471 (fix + 4 regression tests) and + PyAutoGalaxy#569 (`MockResult` signature) both merged to `main`. + The originally-reported symptom was already gone before this work — it was + fixed in the workspace repos on 2026-08-10, hours after the Heart run that + reported it. This ticket fixed the underlying library defect. + +## TL;DR — what changed since this prompt was first filed + +1. **The 7 failing scripts all pass now.** They were fixed in the workspace repos + (`autogalaxy_workspace_test` #104 and `autolens_workspace_test` #256, both + committed 2026-08-10 18:54 -0400) — after Heart run 31356506626 sampled them. + This ticket was filed against stale evidence. +2. **The library defect those commits worked around is still on `main`**, and is + the real content of this ticket. +3. **Reproduction is no longer blocked.** The prompt previously said the full + autogalaxy stack "could not be installed from a cloud session". It can — see + "How to reproduce" below. Everything in this document was produced that way. +4. Three claims in the original prompt were wrong; they are corrected below so + nobody re-treads them. + +## Root cause (reproduced, not inferred) + +`af.m.MockResult.__init__` falls back to building its own summary when the caller +does not pass one: -## Symptom - -Seven aggregator integration scripts across the two `*_workspace_test` repos fail -with the same exception: - - ValueError: ell_comps must satisfy ell_comps[0]**2 + ell_comps[1]**2 < 1; - got (1.0, 1.0), whose magnitude is np.float64(1.4142135623730951) - -Observed in PyAutoHeart Workspace Smoke run 31356506626 (2026-08-10), legs -`autogalaxy_test / misc` and `autolens_test / misc`. - -Failing scripts (all `scripts/misc/aggregator/`): - -- `autogalaxy_workspace_test`: `ellipse.py`, `fit_imaging.py`, - `fit_interferometer.py`, `galaxies.py` -- `autolens_workspace_test`: `tracer.py`, `fit_imaging.py`, - `fit_interferometer.py` - -## Why this is a real bug and not a bad shipped value - -The guard is correct and correctly placed. `validate_ell_comps` sits on -`EllProfile`, the single base every elliptical profile inherits (`ag.Ellipse` -included), and enforces `f = sqrt(e_y**2 + e_x**2) < 1` because the axis ratio is -`q = (1 - f) / (1 + f)` — at `f >= 1` the ellipse degenerates to `q <= 0` and has -no geometric meaning. `(1.0, 1.0)` gives `f = 1.414`, `q = -0.17`. Nothing should -ever construct a profile with it. - -This is **not** the same failure as the sampler-draw legs (`guides`, etc.), which -were fixed by PyAutoGalaxy#568 making `ModelParameterException` a -`FitException` so searches resample. That fix does not help here: the aggregator -rebuilds instances from stored samples outside any likelihood call, so there is no -resample path to take. - -## Evidence - -1. **The value is a hardcoded fill, not a sampled value.** It prints as plain - `(1.0, 1.0)` — Python floats. The genuine sampler-draw failures in the same run - print as `np.float64(-0.7446446619131553)`. Different provenance. - -2. **Two places in PyAutoFit hardcode exactly this shape:** - - `autofit/non_linear/mock/mock_samples.py` — `MockSamples.default_sample_list` - builds `kwargs={path: 1.0 for path in self.model.paths}`. - - `autofit/non_linear/mock/mock_samples_summary.py` — `MockSamplesSummary.__init__` - sets `self._kwargs = {path: 1.0 for path in self.model.paths}`, which backs - both `max_log_likelihood_sample` and `median_pdf_sample`. - - A blanket `1.0` is a safe placeholder for most parameters and an invalid value - for any `ell_comps`. `_make_samples` in `mock_search.py` already does the right - thing (`prior.value_for(0.5)`), so the fix idiom exists in the same package. - -3. **All 7 scripts construct `MockSearch` identically and never pass - `samples_summary`:** - - ```python - search = ag.m.MockSearch( - samples=samples, - result=af.m.MockResult(model=model, samples=samples, - samples_summary=samples.summary()), - ) - ``` - - `MockSearch.__init__` therefore falls back to `MockSamplesSummary.default()`. - Whether that asymmetry (a real model in `MockResult`, a default summary on the - search) is the trigger or a red herring is the open question. - -## Hypothesis already tested and DISPROVEN — do not re-tread +```python +# autofit/non_linear/mock/mock_result.py +super().__init__( + samples_summary=samples_summary or MockSamplesSummary(model=model or ModelMapper()), + ... +) +``` -The obvious suspect is the helper copy-pasted into all 7 scripts: +and `MockSamplesSummary.__init__` fills every parameter with a blanket `1.0`: ```python -def parameter_list_with_physical_ell_comps(value): - parameter_list = model.prior_count * [value] - for index, path_tuple in enumerate(model.all_paths): - if "ell_comps" in path_tuple[0]: - parameter_list[index] = 0.1 - return parameter_list +# autofit/non_linear/mock/mock_samples_summary.py:26 +self._kwargs = {path: 1.0 for path in self.model.paths} if self.model else {} ``` -It looks broken — `all_paths` returns a tuple of `Path`s per prior and -`Path = Tuple[str, ...]`, so `path_tuple[0]` is a path tuple and `in` is -exact-element membership, which would not match a leaf named `ell_comps_0`. +That dict backs both `max_log_likelihood_sample` and `median_pdf_sample`. So any +`MockResult` constructed with a real `model` but no `samples_summary` carries an +all-ones instance — and `1.0` is an invalid `ell_comps` for every elliptical +profile. -**It is not broken.** `ell_comps` has a tuple default, so PyAutoFit builds a -`TuplePrior` attribute named `ell_comps`, and the path is -`('ellipses', '0', 'ell_comps', 'ell_comps_0')` — it contains a bare `'ell_comps'` -element, so the check matches. Reproduced by rebuilding `ellipse.py`'s exact model -(two `Ellipse` models with fixed `major_axis`, plus the nested multipole -collection) against installed autofit and running the real helper: +**The consumer is `Result.instance`, and the crash happens inside `search.fit()` — +not in the aggregator.** Verified traceback (pre-fix `ellipse.py`, libraries at +`main`): ``` -[2] ('ellipses', '0', 'ell_comps', 'ell_comps_0') -> 0.1 -[3] ('ellipses', '0', 'ell_comps', 'ell_comps_1') -> 0.1 -ellipses[0].ell_comps = (0.1, 0.1) +ellipse.py:53 search.fit(model=model, analysis=analysis) +abstract_search.py:739 search_internal, fitness = self._fit(...) +mock_search.py:101 return self._fit_fast(model=model, analysis=analysis) +mock_search.py:95 fitness([prior.mean for prior in model.priors_ordered_by_id]) +mock_search.py:82 if self.result.instance is None: <-- here +result.py:117 return self.samples_summary.instance +... +geometry_profiles.py:237 validate.validate_ell_comps(ell_comps=ell_comps) +autogalaxy.exc.ModelParameterException: ell_comps must satisfy + ell_comps[0]**2 + ell_comps[1]**2 < 1; got (1.0, 1.0), magnitude 1.4142135623730951 ``` -Index alignment is also fine: `all_paths` and `instance_from_vector` -(`prior_tuples_ordered_by_id`) both order by prior id. **Changing this helper is a -no-op — do not "fix" it.** +## Corrections to the original prompt — do not re-tread these + +- **"The path runs through serialization into the database and back out through + the aggregator."** Wrong. The exception is raised during `search.fit(...)`, + before `af.Aggregator.from_database` is ever called. No database round-trip is + involved. +- **"`MockSearch._fit_fast` evaluates at `[prior.mean ...]`, which is 0.0 for + `ell_comps` — valid."** The *vector* is indeed valid. The crash is on the next + line (`mock_search.py:82`, `self.result.instance`), which builds an instance + from the **summary**, not from the vector. +- **"`MockSamplesSummary.default()` uses an empty `Collection()`, so its `_kwargs` + is `{}`."** Correct, and that is exactly why the search-side summary was a dead + end. The reaching path is `MockResult`'s fallback `MockSamplesSummary(model=model)` + — a *different* construction site that was never checked. +- The exception type is `ModelParameterException`, not `ValueError`. +- The original "DISPROVEN — do not re-tread" note about + `parameter_list_with_physical_ell_comps` **stands**: `model.all_paths` and + `model.unique_prior_paths` (used by `Sample.from_lists`) are both sorted by + prior id, so the helper's index alignment is correct. + +## Second, independent defect: `ag.m.MockResult` narrows its parent's API -## What is not yet known - -Which call site actually feeds the all-ones instance to the aggregator. Two -candidates were checked and neither fits cleanly: - -- `MockSamplesSummary.default()` uses an empty `Collection()`, so its `_kwargs` - is `{}`, not a dict of 1.0s. -- `MockSearch._fit_fast` evaluates at `[prior.mean for prior in - model.priors_ordered_by_id]`, which is `0.0` for `ell_comps` — valid. +```python +# autogalaxy/analysis/mock/mock_result.py +class MockResult(af.m.MockResult): + def __init__(self, samples=None, instance=None, model=None, + analysis=None, search=None, + max_log_likelihood_galaxies=None, max_log_likelihood_tracer=None): +``` -So the path runs through serialization into the database and back out through the -aggregator, which is where it needs to be traced. +`samples_summary` is absent from the signature and is not forwarded, so +`ag.m.MockResult(..., samples_summary=...)` raises +`TypeError: MockResult.__init__() got an unexpected keyword argument 'samples_summary'`. +Callers therefore *cannot* avoid the all-ones fallback through `ag.m.MockResult` +at all — which is why the workspace fix had to switch to `af.m.MockResult`. +Confirmed by direct experiment. + +## Evidence: two controlled variants isolate the two causes + +Both run against library `main` with the pre-fix or post-fix `ellipse.py`: + +| Variant | Script parameters | `samples_summary` passed? | Result | +|---|---|---|---| +| Original (pre-fix) | `prior_count * [1.0]`, `* [10.0]` | no | fails at `(1.0, 1.0)` — **the reported symptom** | +| A | `prior_count * [1.0]`, `* [10.0]` | yes | fails at `(10.0, 10.0)` — script's own fill | +| B | physical (`ell_comps` → 0.1) | no | fails at `(1.0, 1.0)` — **library defect alone** | +| Current `main` | physical | yes | passes | + +Variant B is the decisive one: with physically-valid fixture values, the library +fallback still produces the exact reported `(1.0, 1.0)`. The library defect is +real and independent of the workspace fixture values. + +## The fix (written and validated) + +```diff +--- a/autofit/non_linear/mock/mock_samples_summary.py ++++ b/autofit/non_linear/mock/mock_samples_summary.py +@@ -23,7 +23,11 @@ class MockSamplesSummary(SamplesSummary): + self._max_log_likelihood_instance = max_log_likelihood_instance + self._prior_means = prior_means +- self._kwargs = {path: 1.0 for path in self.model.paths} if self.model else {} ++ self._kwargs = ( ++ {path: prior.value_for(0.5) for path, prior in self.model.path_priors_tuples} ++ if self.model ++ else {} ++ ) +``` -## Suggested approach +This matches the idiom already used by `_make_samples` in `mock_search.py` +(`prior.value_for(0.5)`), so the fix is consistent with the package's own +convention rather than a new one. + +### Validation actually run (libraries at `main`, Python 3.11 venv) + +| Suite | With fix | Baseline (no fix) | Verdict | +|---|---|---|---| +| PyAutoFit `test_autofit` | 1694 passed, 1 failed | 1694 passed, 1 failed | identical — no regression | +| PyAutoGalaxy `test_autogalaxy` | 1081 passed, 0 failed | — | clean | +| PyAutoLens `test_autolens` | 518 passed, 1 failed | 1 failed | identical — no regression | +| All 7 reported scripts | 7/7 pass | 7/7 pass | clean | +| Variant B (fixture fix reverted) | passes | fails `(1.0, 1.0)` | fix is load-bearing | + +The two pre-existing failures are unrelated and reproduce without the patch: +`test_autofit/graphical/functionality/test_messages.py::test_beta` and +`test_autolens/potential_correction/test_iterative_interferometer.py::test__solve_joint_optimization__identity_damping_finite`. + +## What was implemented + +Branch `claude/autofit-mock-ones-parameters-bug-sv303m` in both repos. + +**PyAutoFit** (`2581ecf`): +1. New shared helper `prior_median_kwargs(model)` in `mock_samples.py`. +2. `MockSamplesSummary.__init__` and `MockSamples.default_sample_list` both use + it instead of `{path: 1.0 ...}`. `_make_samples` in `mock_search.py` now + delegates to it too — the idiom it already used, in one place rather than three. +3. New `test_autofit/non_linear/samples/test_mock_placeholders.py` — 4 tests using + a guard class that mirrors the `ell_comps` constraint, so the regression is + covered inside PyAutoFit with no autogalaxy dependency. Verified to fail 3/4 + without the fix. + +**PyAutoGalaxy** (`96baf25`): `MockResult.__init__` accepts `samples_summary` and +forwards it to `super()`. `al.m.MockResult` *is* `ag.m.MockResult` (re-exported, +not a second subclass), so PyAutoLens is covered by the same change. + +### Post-implementation validation + +| Suite | Result | Baseline | Verdict | +|---|---|---|---| +| PyAutoFit | 1698 passed, 1 failed | 1694 passed, 1 failed | +4 new tests, no regression | +| PyAutoGalaxy | 1081 passed, 0 failed | 1081 passed, 0 failed | clean | +| PyAutoLens | 518 passed, 1 failed | 518 passed, 1 failed | no regression | +| All 7 reported scripts | 7/7 pass | 7/7 pass | clean | +| Variant B | passes | fails `(1.0, 1.0)` | fix is load-bearing | + +The latent call site `test_autogalaxy/analysis/analysis/test_analysis.py:40` is +green under the full PyAutoGalaxy suite above. + +## Remaining scope + +Optional tidiness only (original suggestion 4): have `MockSearch` inherit +`samples_summary` from a passed-in `result` instead of silently defaulting to +`MockSamplesSummary.default()`. With the fix above this is no longer a +correctness issue. Not done — it touches ~55 `MockSearch` call sites and belongs +in its own behaviour-preserving change. + +**Difficulty is `small`, not `too-large`.** The original sizing assumed a 3-repo +library+workspace coordination. The workspace half is already shipped; what is +left is a self-contained PyAutoFit change (plus an optional small PyAutoGalaxy +one), with all three library suites already shown green against it. + +## How to reproduce (this works from a cloud session) + +```bash +python3.12 -m venv venv && ./venv/bin/pip install autolens # pulls the full stack +./venv/bin/pip uninstall -y autofit autogalaxy autoarray autonerves autolens +# then put the source checkouts on PYTHONPATH: +export PYTHONPATH=:::: +export PYAUTO_SKIP_WORKSPACE_VERSION_CHECK=1 +cd autogalaxy_workspace_test/scripts/misc/aggregator && python ellipse.py +``` -1. Run one failing script (`autogalaxy_workspace_test/scripts/misc/aggregator/ellipse.py`) - against the full stack with a breakpoint or traceback on the guard, and record - the actual construction stack. **This needs a real autogalaxy environment** — - it could not be done from a cloud session (autoarray/jax/numba would not - install there). -2. Fix at the PyAutoFit mock layer: replace the blanket `{path: 1.0 ...}` with - prior-median values (`{path: prior.value_for(0.5) for path, prior in - model.path_priors_tuples}`), matching `_make_samples`. -3. Mind the blast radius: `MockSamples`, `MockSamplesSummary` and `MockSearch` - have roughly 55 call sites inside PyAutoFit alone, plus the PyAutoGalaxy and - PyAutoLens suites. Run all three suites, not just PyAutoFit's. -4. Consider whether `MockSearch` should inherit the `samples_summary` from a - passed-in `result` rather than silently defaulting. +Install the released stack first to get the dependency closure, then shadow the +four libraries with source checkouts via `PYTHONPATH` — an editable install of the +checkouts is refused on Python 3.11 because `autonerves` now requires `>=3.12`, +and the `PYTHONPATH` route sidesteps that gate. To reproduce the original failure, +check out `autogalaxy_workspace_test` at `40beb30^`. ## Notes - Do not relax or move the `ell_comps` guard. It is correct. -- Do not chase the `workspace-validation-report` artifact from a cloud session - (blocked at the egress proxy). Per-job logs via the Actions API carry the same - failures. -- Sibling work already shipped: the one genuinely unphysical shipped literal, - `ell_comps=(0.5, 0.9)` in HowToGalaxy `tutorial_3_fitting`, was corrected - separately. An AST scan of 454 `ell_comps` literals across - autogalaxy_workspace, autolens_workspace, HowToGalaxy, HowToLens and both - `*_workspace_test` repos found no other violating literal, so this ticket is - the whole remaining `ell_comps` surface. +- The workspace-side fixes (#104, #256) are legitimate and should stay: these are + integration fixtures, and physically-valid fixture values are the right thing + regardless of the library defect. They are not masking — after the library fix, + variant B shows the scripts pass on their own merit either way. - PyAutoHeart#27 is a different family (release-profile timeouts and a JAX exception, 2026-07-06); it is not related. +- Sibling work already shipped: the one genuinely unphysical shipped literal, + `ell_comps=(0.5, 0.9)` in HowToGalaxy `tutorial_3_fitting`, was corrected + separately. An AST scan of 454 `ell_comps` literals across the workspace repos + found no other violating literal.