Skip to content

Commit f2f7a4f

Browse files
authored
fix: reject invalid constructor inputs (#333 — B5-B8, B13) (#440)
Closes #333. Phase 2 (PyAutoArray half) of the @rhayes777 API audit epic #415. Five findings that were silently accepted now raise at construction, guarded at chokepoints so coverage exceeds the five reported sites: - B6 pixel_scales of 0.0 / negative / nan -> geometry_util.convert_pixel_scales_{1d,2d} - B8 shape_native with a zero-length axis -> Mask2D.__init__ - B7 annulus with inner >= outer -> circular_annular and elliptical_annular - B5 noise_map shape disagreeing with data -> AbstractDataset - B13 negative regularization coefficient -> all 14 schemes, not only Constant B13 closed a real leak: regularization_matrix_from squares the coefficient and hides the sign, but regularization_weights_from returns it unsquared, so a negative value fed negative regularization weights downstream. The shared helper lives in autoarray/validate.py — the home decision this task owned, which unblocks the PyAutoGalaxy#440 and PyAutoLens#532 siblings. Guards are tracer-safe: gated on is_concrete_scalar, verified against real JAX (construction under jax.jit works, jax.grad flows, concrete negatives rejected). 980 passed, +44 new tests, zero regressions.
1 parent 5867db0 commit f2f7a4f

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)