Missing input validation across Array2D, Grid2D, Mask2D, Imaging, regularization #333

Activity

1. Jammy2211 commented on Jul 28, 2026

   

   Jammy2211

   on Jul 28, 2026

   Collaborator

   More actions

   Thanks for this @rhayes777, and apologies for the long silence — this sat far longer than it should have.

   I've re-run every snippet against current main (autoarray @ 616e8b4c) and all five still reproduce exactly as you describe:

   • Array2D.no_mask(..., pixel_scales=0.0), -0.1 and nan are all accepted; the first then gives ZeroDivisionError: float division by zero on the first derive_grid call.
   • Mask2D.circular_annular(inner_radius=0.8, outer_radius=0.3).pixels_in_mask returns 0.
   • Grid2D.uniform(shape_native=(0, 0)) and (0, 5) both give shape_slim == 0.
   • Imaging with 10x10 data and 5x5 noise_map constructs, and reports shape_native == (10, 10) — the mismatch is swallowed.
   • reg.Constant(coefficient=-1.0) is accepted and stored as -1.0.

   One addition to your B13 analysis: you're right that constant.py:43 does coefficient * coefficient, which is why log_evidence is unchanged between +1.0 and -1.0. But regularization_weights_from (constant.py:127) returns the raw, un-squared self.coefficient, so a negative value does leak negative regularization weights into anything reading that method. That makes the case for rejecting it at construction stronger than "merely unphysical" — thanks for digging into the mechanism rather than just reporting the symptom.

   Your suggested fix shape is the one we'll follow — a shared _validate_* helper with a consistent ValueError message convention across the constructors.

   On the offer to PR: thank you, genuinely, but we'll take these on in-house. The constructors sit on paths that get JAX-traced, and we want to be careful the guards don't cost anything under tracing or break on tracer values — that's fiddly in a way that isn't fair to hand to a contributor.

   This is being tracked alongside the other four issues from your audit (#332, PyAutoGalaxy#440, PyAutoLens#531, PyAutoLens#532) as a single validation campaign. I'll link the tracking issue here once it's open, and keep this thread updated as the fixes land.
2. Jammy2211 commented on Jul 28, 2026

   

   Jammy2211

   on Jul 28, 2026

   Collaborator

   More actions

   As promised above — the tracking issue for this campaign is
   #415, which carries the verification evidence for all 16
   findings and the phase breakdown. Watch that one for overall progress; I'll keep
   updating this thread as the parts touching it land.

   Phase 1 (the three crash-level findings, which is where we're starting) is
   #416.
3. Jammy2211

   closed this as completedin #440on Aug 9, 2026
4. Jammy2211 commented on Aug 9, 2026

   

   Jammy2211

   on Aug 9, 2026 – with Claude

   Collaborator

   More actions

   Fixed and merged in #440 (f2f7a4f). Thanks again for the audit @rhayes777 — all five findings here are closed.

   Each now raises at construction with a message naming the parameter. The guards went in at chokepoints rather than at the five reported call sites, so a few of them cover more than was reported:

   • B6 — guarded at geometry_util.convert_pixel_scales_{1d,2d}, which every Mask2D factory and Grid2D.uniform funnel through.
   • B7 — circular_annular and elliptical_annular; the elliptical sibling had the identical hole.
   • B8 — Mask2D.__init__, which every factory returns through.
   • B5 — AbstractDataset, so Interferometer and every other dataset subclass are covered, not just Imaging.
   • B13 — all 14 regularization schemes, not only Constant. Thirteen siblings had the same hole.

   One correction to the B13 write-up, in your favour: you noted the identical log_evidence for ±1.0 and reasonably read the value as inert. It isn't. regularization_matrix_from squares the coefficient, which hides the sign — but regularization_weights_from returns it unsquared, so a negative coefficient was leaking negative regularization weights into every consumer of that method:

   Constant(-1.0).regularization_weights_from  ->  [-1. -1. -1. -1.]
   

   So that one was a live bug, not a cosmetic one.

   On the JAX point behind our earlier "we'll take this in-house" reply: the guards are gated on a concreteness check and pass tracers through untouched, so they cost nothing inside a trace. Verified against JAX 0.11.0 — construction under jax.jit works and jax.grad flows through a traced coefficient, while a concrete negative is still rejected.

   Zero is still accepted for coefficients, as a degenerate but meaningful "no regularization" request.

   Epic #415 stays open for the rest: #332 (adapt_images precondition legibility), PyAutoGalaxy#440 and PyAutoLens#532 — the last two now unblocked, since they import the shared autoarray/validate.py helper this PR established. Your question on #532 about whether z_lens > z_source should warn at all is still open and still wanted.

   ---

   Generated by Claude Code