Skip to content

Commit 5bd1472

Browse files
committed
fix: reject invalid constructor inputs (#333 — B5-B8, B13)
Five findings from @rhayes777's API audit, all still reproducing on main. Each was "accepted silently, then a confusing traceback (or nothing) several calls later"; each now raises at construction with a message naming the parameter. - B6 pixel_scales of 0.0 / negative / nan -> guarded at geometry_util.convert_pixel_scales_{1d,2d}, the chokepoint every Mask2D factory and Grid2D.uniform funnel through. - B8 shape_native with a zero-length axis -> guarded in Mask2D.__init__, which every factory returns through (Grid2D.uniform reaches it via no_mask -> all_false). - B7 annulus with inner >= outer -> guarded in circular_annular and in elliptical_annular, which had the identical hole. - B5 noise_map shape disagreeing with data -> guarded in AbstractDataset, so Imaging, Interferometer and every other subclass are covered. - B13 negative regularization coefficient -> guarded at all 14 schemes, not only the reported Constant. This closes a real leak, not a cosmetic one: regularization_matrix_from squares the coefficient (hiding the sign), but regularization_weights_from returns it unsquared, so a negative value fed negative regularization weights to every consumer of that method. The shared helper lives in autoarray/validate.py — the home decision this task owned, since the PyAutoGalaxy#440 and PyAutoLens#532 prompts import it rather than restating the same rules with different wording. Guards are tracer-safe. Coefficients are free model parameters, so under a traced fit a constructor receives a JAX tracer; every value guard is gated on is_concrete_scalar and passes non-concrete values straight through, so a Python truth-test is never applied to a tracer. Verified against real JAX: construction under jax.jit works and jax.grad flows through, while concrete negatives are still rejected. Shape checks need no gate — shapes are static under tracing. Tests: 44 new cases in test_autoarray/test_validate.py, one per finding built from the reporter's own snippets plus a control per finding so a guard cannot pass by rejecting everything. Suite is 980 passed; the 3 pynufft failures in test_transformer.py are pre-existing on clean main (baselined) and tracked separately. Closes #333. Epic #415 stays open for phases 3-4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013PgqSCLTemK5bApVAwhVM4
1 parent 5867db0 commit 5bd1472

18 files changed

Lines changed: 726 additions & 0 deletions

autoarray/dataset/abstract/dataset.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,25 @@ def __init__(
119119
"""
120120
) from e
121121

122+
# Guarding on the base class covers `Imaging`, `Interferometer` and every other
123+
# dataset subclass. Array shapes are static under JAX, so no tracer gate is
124+
# needed here (unlike the scalar guards in `autoarray.validate`).
125+
data_shape = getattr(data, "shape_native", None)
126+
noise_map_shape = getattr(noise_map, "shape_native", None)
127+
128+
if (
129+
data_shape is not None
130+
and noise_map_shape is not None
131+
and tuple(data_shape) != tuple(noise_map_shape)
132+
):
133+
raise exc.DatasetException(
134+
f"noise_map must have the same shape as data; got data with "
135+
f"shape_native {tuple(data_shape)!r} and noise_map with shape_native "
136+
f"{tuple(noise_map_shape)!r}. Every fit quantity pairs a data value "
137+
f"with its noise value pixel-by-pixel, so a mismatch has no "
138+
f"well-defined meaning"
139+
)
140+
122141
self.noise_map = noise_map
123142

124143
self.over_sample_size_lp = (

autoarray/geometry/geometry_util.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from typing import Tuple, Union
33

44
from autoarray import type as ty
5+
from autoarray import validate
56

67

78
def convert_shape_native_1d(shape_native: Union[int, Tuple[int]]) -> Tuple[int]:
@@ -46,8 +47,15 @@ def convert_pixel_scales_1d(pixel_scales: ty.PixelScales) -> Tuple[float]:
4647
-------
4748
Tuple[float]
4849
The pixel scale as a 1-element tuple `(float,)`.
50+
51+
Raises
52+
------
53+
ValueError
54+
If any entry is a concrete scalar which is not finite and above zero.
4955
"""
5056

57+
validate.validate_pixel_scales(pixel_scales=pixel_scales)
58+
5159
if type(pixel_scales) is float:
5260
pixel_scales = (pixel_scales,)
5361

@@ -205,8 +213,21 @@ def convert_pixel_scales_2d(pixel_scales: ty.PixelScales) -> Tuple[float, float]
205213
-------
206214
Tuple[float, float]
207215
The pixel scale as a 2-element tuple `(float, float)`.
216+
217+
Raises
218+
------
219+
ValueError
220+
If any entry is a concrete scalar which is not finite and above zero.
221+
222+
Notes
223+
-----
224+
This is the single chokepoint every ``Mask2D`` factory and ``Grid2D.uniform``
225+
funnel their ``pixel_scales`` through, so validating here covers them all rather
226+
than repeating a guard at each construction site.
208227
"""
209228

229+
validate.validate_pixel_scales(pixel_scales=pixel_scales)
230+
210231
if type(pixel_scales) is float:
211232
pixel_scales = (pixel_scales, pixel_scales)
212233

autoarray/inversion/regularization/abstract.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,48 @@
22
import numpy as np
33
from typing import Optional, TYPE_CHECKING
44

5+
from autoarray import validate
6+
57
if TYPE_CHECKING:
68
from autoarray.inversion.linear_obj.linear_obj import LinearObj
79

810

11+
def validate_coefficient(coefficient, name: str = "coefficient"):
12+
"""
13+
Raise if a regularization coefficient is a concrete scalar which is negative or
14+
non-finite.
15+
16+
Every regularization scheme calls this from its constructor, so the message for
17+
this class of mistake is written once here rather than per scheme.
18+
19+
Zero is permitted: it is a degenerate but meaningful request for no regularization.
20+
Negative is not, and is **not** inert despite appearances — see the note below.
21+
22+
Coefficients are free model parameters, so under a JAX-traced fit this receives a
23+
tracer rather than a number. `autoarray.validate` gates on concreteness before
24+
comparing, so the guard costs nothing inside a trace.
25+
26+
Parameters
27+
----------
28+
coefficient
29+
The regularization coefficient to validate.
30+
name
31+
The parameter's name, used in the error message (schemes with more than one
32+
coefficient pass their own, e.g. ``inner_coefficient``).
33+
"""
34+
validate.validate_non_negative_finite(
35+
value=coefficient,
36+
name=name,
37+
extra=(
38+
"A regularization coefficient sets the strength of the smoothing applied "
39+
"to the reconstruction, which cannot be negative. A negative value is not "
40+
"inert: `regularization_matrix_from` squares it, which hides the sign, but "
41+
"`regularization_weights_from` returns it unsquared and so leaks negative "
42+
"regularization weights into every consumer of that method"
43+
),
44+
)
45+
46+
947
class AbstractRegularization:
1048
is_split_regularization = False
1149
"""

autoarray/inversion/regularization/adapt.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from autoarray.inversion.linear_obj.linear_obj import LinearObj
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
9+
from autoarray.inversion.regularization.abstract import validate_coefficient
910

1011

1112
def adapt_regularization_weights_from(
@@ -193,7 +194,9 @@ def __init__(
193194

194195
super().__init__()
195196

197+
validate_coefficient(coefficient=inner_coefficient, name="inner_coefficient")
196198
self.inner_coefficient = inner_coefficient
199+
validate_coefficient(coefficient=outer_coefficient, name="outer_coefficient")
197200
self.outer_coefficient = outer_coefficient
198201
self.signal_scale = signal_scale
199202

autoarray/inversion/regularization/adapt_split_zeroth.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
from autoarray.inversion.regularization.adapt import Adapt
99
from autoarray.inversion.regularization.brightness_zeroth import BrightnessZeroth
1010
from autoarray.inversion.regularization import regularization_util
11+
from autoarray.inversion.regularization.abstract import validate_coefficient
1112

1213

1314
class AdaptSplitZeroth(Adapt):
1415
is_split_regularization = True
16+
1517
def __init__(
1618
self,
1719
zeroth_coefficient: float = 1.0,
@@ -77,6 +79,7 @@ def __init__(
7779
low signal regions.
7880
"""
7981

82+
validate_coefficient(coefficient=zeroth_coefficient, name="zeroth_coefficient")
8083
self.zeroth_coefficient = zeroth_coefficient
8184
self.zeroth_signal_scale = zeroth_signal_scale
8285

autoarray/inversion/regularization/brightness_zeroth.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from autoarray.inversion.linear_obj.linear_obj import LinearObj
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
9+
from autoarray.inversion.regularization.abstract import validate_coefficient
910

1011

1112
def brightness_zeroth_regularization_weights_from(
@@ -104,6 +105,7 @@ def __init__(
104105

105106
super().__init__()
106107

108+
validate_coefficient(coefficient=coefficient)
107109
self.coefficient = coefficient
108110
self.signal_scale = signal_scale
109111

autoarray/inversion/regularization/constant.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from autoarray.inversion.linear_obj.linear_obj import LinearObj
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
9+
from autoarray.inversion.regularization.abstract import validate_coefficient
910

1011

1112
def constant_regularization_matrix_from(
@@ -101,6 +102,7 @@ def __init__(self, coefficient: float = 1.0):
101102
The regularization coefficient which controls the degree of smooth of the inversion reconstruction.
102103
"""
103104

105+
validate_coefficient(coefficient=coefficient)
104106
self.coefficient = coefficient
105107

106108
super().__init__()

autoarray/inversion/regularization/constant_zeroth.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from autoarray.inversion.linear_obj.linear_obj import LinearObj
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
9+
from autoarray.inversion.regularization.abstract import validate_coefficient
910

1011

1112
def constant_zeroth_regularization_matrix_from(
@@ -76,7 +77,11 @@ class ConstantZeroth(AbstractRegularization):
7677
def __init__(self, coefficient_neighbor=1.0, coefficient_zeroth=1.0):
7778
super().__init__()
7879

80+
validate_coefficient(
81+
coefficient=coefficient_neighbor, name="coefficient_neighbor"
82+
)
7983
self.coefficient_neighbor = coefficient_neighbor
84+
validate_coefficient(coefficient=coefficient_zeroth, name="coefficient_zeroth")
8085
self.coefficient_zeroth = coefficient_zeroth
8186

8287
def regularization_weights_from(self, linear_obj: LinearObj, xp=np) -> np.ndarray:

autoarray/inversion/regularization/curvature_mask.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
99
from autoarray.operators import derivative_util
10+
from autoarray.inversion.regularization.abstract import validate_coefficient
1011

1112

1213
def curvature_reg_matrix_via_mask_from(mask, pixel_scale: float = 1.0) -> np.ndarray:
@@ -71,6 +72,7 @@ def __init__(self, coefficient: float = 1.0):
7172
The regularization coefficient which multiplies the matrix,
7273
setting the strength of the smoothing.
7374
"""
75+
validate_coefficient(coefficient=coefficient)
7476
self.coefficient = coefficient
7577

7678
super().__init__()

autoarray/inversion/regularization/exponential_kernel.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from autoarray.inversion.linear_obj.linear_obj import LinearObj
77

88
from autoarray.inversion.regularization.abstract import AbstractRegularization
9+
from autoarray.inversion.regularization.abstract import validate_coefficient
910

1011

1112
def exp_cov_matrix_from(
@@ -110,6 +111,7 @@ def __init__(
110111
convention assumes ``C_ii ~ 1``, which holds for this unweighted kernel but not
111112
for the adaptive one; see :func:`apply_jitter` for why and when to switch.
112113
"""
114+
validate_coefficient(coefficient=coefficient)
113115
self.coefficient = coefficient
114116
self.scale = scale
115117
self.jitter = jitter

0 commit comments

Comments
 (0)