|
| 1 | +Two sites the PyAutoArray#464 `pixel_scales` scalar-widening sweep did not reach, both |
| 2 | +reproduced on `main` before being touched, both fixed together so the 1D and 2D siblings |
| 3 | +cannot drift apart again. |
| 4 | + |
| 5 | +**Shipped:** PyAutoArray#485, merge commit `9e475052`, 2026-08-23. Issue #484 closed. |
| 6 | + |
| 7 | +## The two sites |
| 8 | + |
| 9 | +| Site | Symptom on `main` | Fix | |
| 10 | +|---|---|---| |
| 11 | +| `Mask1D.__init__` (`mask_1d.py:71`) | `pixel_scales=1` stored the bare `1`; `.geometry.scaled_maxima` → `TypeError: 'int' object is not subscriptable` | route through `geometry_util.convert_pixel_scales_1d`, the call `Mask2D.__init__` already made | |
| 12 | +| `convert_shape_native_1d` (`geometry_util.py:27`) | `Array1D.full(shape_native=np.int32(5))` → `IndexError: invalid index to scalar variable` | test `validate.is_concrete_integer`, cast to a Python `int` | |
| 13 | + |
| 14 | +Site 1 was #464's *exact reported symptom*, still live on a public constructor after that |
| 15 | +PR shipped, because `Mask1D` hand-rolled its own `type(x) is float` check rather than |
| 16 | +calling the chokepoint. `Mask2D` already routed through `convert_pixel_scales_2d`, and |
| 17 | +`Grid1D.uniform` reaches it too — so this was a 1D/2D divergence, not a design choice. |
| 18 | + |
| 19 | +## The decision this task owned |
| 20 | + |
| 21 | +**A second predicate, not a reuse of `is_concrete_scalar`.** `shape_native` counts pixels |
| 22 | +rather than measuring them, so accepting a `float` there would silently widen a mistake |
| 23 | +worth surfacing. `validate.is_concrete_integer` is therefore integer-only (`int`, |
| 24 | +`np.integer`), inheriting the `bool` exclusion and tracer-safety of its sibling. Reusing |
| 25 | +`is_concrete_scalar` would have been the smaller diff and the wrong contract. |
| 26 | + |
| 27 | +**A deliberate contract change on `Mask1D`.** Routing through the chokepoint brings |
| 28 | +`validate.validate_pixel_scales` with it, so `Mask1D` now rejects `0`, negative and `nan` |
| 29 | +pixel scales — which `Mask2D` already did. Stated in the PR rather than suppressed; no test |
| 30 | +constructed a `Mask1D` that way and all 12 library call sites pass real scales, so nothing |
| 31 | +was adjusted to suit it. |
| 32 | + |
| 33 | +## Verified, not assumed |
| 34 | + |
| 35 | +- **Both defects reproduced first**, on `main`, before any edit — the prompt's own fix had |
| 36 | + already shipped, so the filed work was re-derived from a live repro rather than trusted. |
| 37 | +- **The originating prompt was already complete.** Its fix shipped as #464 / `8298d74e` on |
| 38 | + 2026-08-22 while the prompt sat in `draft/` unrecorded. Backfilled to |
| 39 | + `complete/2026/08/autoarray-pixel-scales-scalar-widening.md` rather than re-filed. |
| 40 | +- **`test_autoarray`: 1201 passed / 0 failed.** The 3 pynufft failures `8298d74e` baselined |
| 41 | + no longer occur, so there was nothing to baseline against. |
| 42 | +- **A `git stash` that proved nothing.** The first attempt to show the one `test_autocti` |
| 43 | + failure was pre-existing stashed an already-committed tree — a silent no-op, so the run |
| 44 | + was *with* the change. Redone by pointing `PYTHONPATH` at the canonical `main` checkout, |
| 45 | + where `test_serial_eper.py::test__region_list_from__array_2d_list_from` fails identically. |
| 46 | +- **Real-JAX, not a stand-in.** `jax.jit` compiled and ran with a traced `pixel_scales` |
| 47 | + passing through untouched, on top of the tracer-stand-in unit tests. |
| 48 | +- **Blast radius measured, not assumed.** PyAutoGalaxy and PyAutoLens only re-export |
| 49 | + `Mask1D` and construct none. `autocti_workspace` uses `ac.Mask1D`, which subclasses |
| 50 | + `aa.Mask1D` and *inherits* the fix — its full suite (270 passed / 1 pre-existing) and the |
| 51 | + dataset_1d simulator + modeling smoke both pass. |
| 52 | + |
| 53 | +## The test bug found on the way |
| 54 | + |
| 55 | +#464's own widening tests asserted value only. `np.float64(1.0) == (1.0,)` NumPy-broadcasts |
| 56 | +to `array([True])`, which is truthy — so **four of six parametrisations passed on an |
| 57 | +unwidened NumPy scalar and tested nothing**. The tests written for this task hit the same |
| 58 | +trap first and were caught by the fail-without-the-fix check; #464's were then tightened the |
| 59 | +same way, proven by reverting to the pre-#464 source and watching them fail. A widening test |
| 60 | +must assert tuple-ness *before* value. |
| 61 | + |
| 62 | +## Merged under a human override |
| 63 | + |
| 64 | +`pyauto-heart readiness` was red at both ship and merge time on `release validation FAILED` |
| 65 | +— an incomplete report (`failures: []`, 262 passed / 0 failed, only the `integrate` stage, |
| 66 | +pre-dating the task, against an older sha). Unrelated to this change and not repaired by it, |
| 67 | +so the `AUTONOMY.md` corrective-PR exception did not apply; both the PR-open and the merge |
| 68 | +were explicit human overrides. The other reason, `PyAutoFit: 2 commit(s) behind origin`, was |
| 69 | +cleared by a fast-forward pull. |
| 70 | + |
| 71 | +## Left open |
| 72 | + |
| 73 | +- **Tuple entries returned unnormalised** — `convert_pixel_scales_2d((1, 1))` → `(1, 1)`, |
| 74 | + contradicting the `Tuple[float, float]` annotation. Deliberately unfixed and unfiled: it |
| 75 | + alters return values on paths that work today. |
| 76 | +- **`release validation FAILED`** still red — a Heart-side artifact question |
| 77 | + (`validate --ingest` replacing rather than merging stages), not a library defect. |
| 78 | +- **API-gate false positive** — the gate blocked `autocti_workspace`'s |
| 79 | + `scripts/dataset_1d/modeling/start_here.py` on `aplt.subplot_dataset_1d`, which the script |
| 80 | + then used successfully at runtime once bypassed. |
| 81 | + |
| 82 | +## Original prompt |
| 83 | + |
| 84 | +# Scalar widening: the two sites the `pixel_scales` sweep did not reach |
| 85 | + |
| 86 | +Type: bug |
| 87 | +Target: autoarray |
| 88 | +Repos: |
| 89 | +- @PyAutoArray |
| 90 | +Difficulty: small |
| 91 | +Autonomy: supervised |
| 92 | +Priority: medium |
| 93 | +Status: active |
| 94 | +Filed: 2026-08-23 |
| 95 | +Issued: 2026-08-23 |
| 96 | + |
| 97 | +## Why this exists |
| 98 | + |
| 99 | +PyAutoArray#464 (`8298d74e`, 2026-08-22) fixed `convert_pixel_scales_1d` and |
| 100 | +`convert_pixel_scales_2d`: `type(x) is float` became `validate.is_concrete_scalar`, |
| 101 | +so `int`, `np.integer` and `np.floating` are all widened and cast to `float`. |
| 102 | + |
| 103 | +Re-running that prompt's repro on 2026-08-23 found the sweep did not reach every site. |
| 104 | +Two defects of the same class are live on `main`. Both are confirmed, not suspected. |
| 105 | +Completion record: `complete/2026/08/autoarray-pixel-scales-scalar-widening.md`. |
| 106 | + |
| 107 | +## Site 1 — `Mask1D.__init__` never routes through the chokepoint |
| 108 | + |
| 109 | +`autoarray/mask/mask_1d.py:71` still carries the original exact-type check and does its |
| 110 | +own widening rather than calling `convert_pixel_scales_1d`: |
| 111 | + |
| 112 | +```python |
| 113 | +if type(pixel_scales) is float: |
| 114 | + pixel_scales = (pixel_scales,) |
| 115 | +``` |
| 116 | + |
| 117 | +``` |
| 118 | +aa.Mask1D(mask=np.array([False, False, True]), pixel_scales=1).pixel_scales |
| 119 | + -> 1 # bare int, not (1.0,) |
| 120 | +...that mask's .geometry.scaled_maxima |
| 121 | + -> TypeError: 'int' object is not subscriptable |
| 122 | +``` |
| 123 | + |
| 124 | +This is #464's exact reported symptom, on a public constructor. `Mask2D.__init__` |
| 125 | +is unaffected — it calls `geometry_util.convert_pixel_scales_2d` at |
| 126 | +`autoarray/mask/mask_2d.py:218`. So this is a 1D/2D divergence, not a design choice, |
| 127 | +and `Grid1D.uniform` is fine because it goes through `geometry_util`. |
| 128 | + |
| 129 | +**Fix:** replace the hand-rolled widening with the call `Mask2D` makes: |
| 130 | + |
| 131 | +```python |
| 132 | +pixel_scales = geometry_util.convert_pixel_scales_1d(pixel_scales=pixel_scales) |
| 133 | +``` |
| 134 | + |
| 135 | +**Consequence to state, not suppress:** `convert_pixel_scales_1d` runs |
| 136 | +`validate.validate_pixel_scales` first, so `Mask1D` starts rejecting `0`, negative and |
| 137 | +`nan` pixel scales — exactly as `Mask2D` already does. No test in `test_autoarray` |
| 138 | +constructs a `Mask1D` with such a value, and the 12 library call sites all pass real |
| 139 | +scales, but read any failure this produces rather than adjusting the test. |
| 140 | + |
| 141 | +## Site 2 — `convert_shape_native_1d` keeps `type(x) is int` |
| 142 | + |
| 143 | +`autoarray/geometry/geometry_util.py:27`. `8298d74e` listed this as not-fixed-here. |
| 144 | +Reachable through `Array1D.full` / `zeros` / `ones`, whose sole call site is |
| 145 | +`autoarray/structures/arrays/uniform_1d.py:143` and which then does `shape_native[0]`: |
| 146 | + |
| 147 | +``` |
| 148 | +aa.Array1D.full(fill_value=1.0, shape_native=np.int32(5)) |
| 149 | + -> IndexError: invalid index to scalar variable |
| 150 | +``` |
| 151 | + |
| 152 | +**Fix:** widen to an **integer-only** test — `is_concrete_scalar` is the wrong predicate |
| 153 | +here, since `shape_native` is a pixel count and a `float` must not be silently widened. |
| 154 | +Use `isinstance(x, (int, np.integer)) and not isinstance(x, bool)`, cast to `int` so the |
| 155 | +result matches the `Tuple[int]` annotation. Prefer adding this as `is_concrete_integer` |
| 156 | +in `autoarray/validate.py` beside `is_concrete_scalar` (`validate.py:48`), so the |
| 157 | +function delegates its predicate the way its `convert_pixel_scales_*` siblings do. |
| 158 | + |
| 159 | +Do **not** add `validate.validate_shape_native` here — the function performs no |
| 160 | +validation today and adding it is a separate change. |
| 161 | + |
| 162 | +## Explicitly out of scope |
| 163 | + |
| 164 | +Tuple entries are still returned unnormalised: `convert_pixel_scales_2d((1, 1))` → `(1, 1)`, |
| 165 | +contradicting the `Tuple[float, float]` annotation. This changes return values on paths |
| 166 | +that currently work, so it needs its own change and its own suite read. Unfiled. |
| 167 | + |
| 168 | +## Verification |
| 169 | + |
| 170 | +- `Mask1D(mask=…, pixel_scales=1).pixel_scales == (1.0,)`; same for `np.float64(1.0)` |
| 171 | + and `np.int32(1)`; `.geometry.scaled_maxima` no longer raises. |
| 172 | +- A `(1.0,)` tuple is returned unchanged; a JAX tracer still passes through untouched, |
| 173 | + so `Mask1D` stays `jit`-safe. |
| 174 | +- `Mask1D` raises `ValueError` on `pixel_scales` of `0`, `-1` and `nan`, matching `Mask2D`. |
| 175 | +- `convert_shape_native_1d` widens `int` and `np.integer` to `(int,)` with a Python `int` |
| 176 | + entry; a `bool` is not widened; a `(5,)` tuple is unchanged. |
| 177 | +- `Array1D.full(fill_value=1.0, shape_native=np.int32(5))` builds. |
| 178 | +- Every new test confirmed to fail without the source change. |
| 179 | +- Full `test_autoarray` suite, not just the touched files — site 1 changes the `Mask1D` |
| 180 | + contract. Baseline the known pre-existing pynufft failures in `test_transformer.py` |
| 181 | + against a clean tree; `8298d74e` recorded 1145 passed / 1 skipped / 3 failed. |
| 182 | + |
| 183 | +Repro environment: `PYAUTO_SKIP_WORKSPACE_VERSION_CHECK=1`, |
| 184 | +`NUMBA_CACHE_DIR=/tmp/numba_cache`, `MPLCONFIGDIR=/tmp/matplotlib`, |
| 185 | +`PYAUTO_DISABLE_JAX=1`. |
| 186 | + |
| 187 | +## Provenance |
| 188 | + |
| 189 | +- Follow-up of: `complete/2026/08/autoarray-pixel-scales-scalar-widening.md` (PyAutoArray#464) |
| 190 | +- Grandparent: `complete/2026/08/autoarray-input-validation-guards.md` (PyAutoArray#440 / #333) |
| 191 | +- Not part of the @rhayes777 audit campaign (`planned.md` § `rhayes-audit-validation-phases-2-4`). |
0 commit comments