Skip to content

fix: reject invalid constructor inputs (#333 — B5-B8, B13) - #440

Merged
Jammy2211 merged 1 commit into
mainfrom
feature/autoarray-input-validation-guards
Aug 9, 2026
Merged

Jammy2211 merged 1 commit into
mainfrom
feature/autoarray-input-validation-guards

Conversation

@Jammy2211

Copy link
Copy Markdown
Collaborator

Closes #333. Implements #439. Phase 2 (PyAutoArray half) of the @rhayes777 API audit epic #415, which stays open for phases 3-4.

What this fixes

Five findings from @rhayes777's 2026-05-23 audit, all still reproducing on main when this branch was cut. Each was "accepted silently, then a confusing traceback (or nothing at all) several calls later"; each now raises at construction with a message naming the offending parameter.

ID Input Before After
B6 pixel_scales of 0.0 / -0.1 / nan accepted; ZeroDivisionError on the first derive_grid, or a silently flipped coordinate system ValueError naming pixel_scales
B7 circular_annular(inner_radius=0.8, outer_radius=0.3) accepted, pixels_in_mask == 0 MaskException naming both radii
B8 Grid2D.uniform(shape_native=(0, 5)) accepted, shape_slim == 0 MaskException naming the axis
B5 Imaging(data 10x10, noise_map 5x5) built; shape_native reported (10, 10), mismatch swallowed DatasetException reporting both shapes
B13 reg.Constant(coefficient=-1.0) accepted, stored as -1.0 ValueError naming coefficient

Guards sit at chokepoints rather than at the five reported call sites, so coverage is wider than the report:

  • B6 — geometry_util.convert_pixel_scales_{1d,2d}, which every Mask2D factory and Grid2D.uniform funnel through.
  • B8 — Mask2D.__init__, which every factory returns through (Grid2D.uniform reaches it via no_mask → all_false).
  • B7 — circular_annular and elliptical_annular, which had the identical hole.
  • B5 — AbstractDataset.__init__, so Interferometer and every other subclass are covered, not just the reported Imaging.
  • B13 — all 14 regularization schemes, not only the reported Constant. Thirteen siblings had the same hole.

B13 is a real leak, not a cosmetic one

The reporter read a negative coefficient as inert because log_evidence is identical for +1.0 and -1.0. That is regularization_matrix_from squaring it (constant.py:43) and hiding the sign. But regularization_weights_from returns the coefficient unsquared, so a negative value fed negative regularization weights to every consumer of that method. Confirmed empirically before fixing:

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

Rejecting at construction is what closes it.

The shared _validate_* home — the decision this task owned

autoarray/validate.py, public. PyAutoArray is the floor PyAutoGalaxy and PyAutoLens both build on, so the blocked sibling prompts (PyAutoGalaxy#440, PyAutoLens#532) import it — from autoarray import validate — rather than restating the same rules with different wording in three repos.

Message shape, applied everywhere: name the parameter, state the rule, show the received value, plus an optional sentence of guidance.

pixel_scales must be a finite positive number; got -0.1. A pixel scale is the
scaled-units size of one pixel, so it must be above zero: ...

Tracer safety (the binding constraint from phase 1)

Coefficients are free model parameters, so under a traced fit a constructor receives a JAX tracer, not a number — a plain if value < 0 would raise TracerBoolConversionError. Every value guard is gated on is_concrete_scalar and passes non-concrete values straight through. Shape checks need no gate: shapes are static under tracing.

This was verified against real JAX (0.11.0), not just asserted:

  • construction of reg.Constant / reg.Adapt inside jax.jit with tracer coefficients — works
  • jax.grad through a traced coefficient — flows (grad 16.0 for sum((c·1)²) at c=2 over 4 params, as expected)
  • a concrete -1.0 outside a trace — still rejected

The committed tests stay numpy-only per phase 1, and assert the property against the concreteness gate itself.

API Changes

No signatures, names or return types change. The change is in accepted input domain: inputs that were previously accepted and produced degenerate or misleading objects now raise at construction.

  • pixel_scales must be finite and > 0 (previously 0.0, negative and nan were accepted).
  • shape_native must have no zero-length axis (previously accepted, giving shape_slim == 0).
  • Annulus constructors require inner_radius < outer_radius (previously accepted, giving an empty mask).
  • A dataset's noise_map must match its data shape (previously accepted).
  • Regularization coefficients must be finite and >= 0 (previously any value accepted). Zero is still permitted — a degenerate but meaningful "no regularization" request.

No workspace script relies on any of the newly-rejected inputs; the full library suite contained no test that did either (see below).

Test Plan

  • New: test_autoarray/test_validate.py — 44 cases. One regression test per finding built from the reporter's own snippets, asserting the failure and that the message names the parameter; plus a control per finding asserting the valid input still works, so no guard can pass by rejecting everything.
  • Full suite: 980 passed, 52 skipped.
  • Pre-existing failures, not caused by this branch: the 3 pynufft tests in test_autoarray/operators/test_transformer.py fail identically on unmodified main — baselined by stashing this branch's changes and re-running. Tracked separately.
  • Zero regressions.

One note worth recording: the anticipated risk was that guards this central would surface existing tests constructing degenerate objects on purpose, each needing triage. That count was zero — nothing in the suite relied on a zero pixel scale, an empty shape, a swapped annulus, a mismatched noise map, or a negative coefficient.

Out of scope

  • PyAutoGalaxy#440 profile guards and PyAutoLens#532 Tracer guards — sibling prompts, unblocked by this PR's helper.
  • PyAutoArray#332 adapt_images precondition legibility — phase 3, independent.
  • z_lens > z_source — phase 4, HELD on the reporter's answer.
  • Adjacent defect found, deliberately not fixed here: convert_pixel_scales_2d tests type(pixel_scales) is float, so an int pixel scale is never widened to a tuple. Worth its own issue.

Generated by Claude Code

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
@Jammy2211 Jammy2211 added the pending-release PR queued for the next release build label Aug 9, 2026 — with Claude
@Jammy2211
Jammy2211 merged commit f2f7a4f into main Aug 9, 2026
2 checks passed
@Jammy2211
Jammy2211 deleted the feature/autoarray-input-validation-guards branch August 25, 2026 18:15
@Jammy2211 Jammy2211 removed the pending-release PR queued for the next release build label Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants