From 84b9ed42b583dd1473aeda259b8e9d7a3f153c10 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 15:07:01 +0000 Subject: [PATCH] docs: record that edge-zeroing is deliberately scoped to the positive-only solver use_edge_zeroed_pixels is consulted only when use_positive_only_solver is True. The positive-negative branch solves the full system regardless of its value, so the two are not independent switches -- but config/general.yaml lists them as two adjacent booleans with no hint of the dependency, and the nesting is only visible by reading the control flow in AbstractInversion.reconstruction. That has already been mistaken for a bug ("a setting silently ignored") by someone reading the branch without the context, twice deferred as work needing sign-off, and written up in three places as a defect before the author confirmed the scoping is deliberate. Documented in the three places a reader can arrive at it: - config/general.yaml -- the line itself now says the scoping exists and is intended, since this is the file that reads as two independent switches. - Settings.use_edge_zeroed_pixels -- gains a docstring stating the dependency and why it is called out. - AbstractInversion.reconstruction -- a comment at the nesting site itself, saying explicitly not to "fix" it by hoisting the check out of the branch. Documentation only; no behaviour change. test_autoarray/inversion: 346 passed. Not covered here: every workspace ships its own copy of config/general.yaml with the uncommented version of this line (confirmed in autolens_workspace), and the workspace config is the one users actually read. Propagating the comment there is a separate change across the workspace repos, recorded in PyAutoMind. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0133X4XhMV91SFjzV2mK4Ejh --- autoarray/config/general.yaml | 2 +- autoarray/inversion/inversion/abstract.py | 4 ++++ autoarray/settings.py | 11 +++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/autoarray/config/general.yaml b/autoarray/config/general.yaml index 2577bd21e..4d75ee632 100644 --- a/autoarray/config/general.yaml +++ b/autoarray/config/general.yaml @@ -3,7 +3,7 @@ psf: inversion: check_reconstruction: true # If True, the inversion's reconstruction is checked to ensure the solution of a meshs's mapper is not an invalid solution where the values are all the same. use_positive_only_solver: true # If True, inversion's use a positive-only linear algebra solver by default, which is slower but prevents unphysical negative values in the reconstructed solutuion. - use_edge_zeroed_pixels : true # If True, the edge pixels of a pixelization are set to zero, which prevents unphysical values in the reconstructed solution at the edge of the pixelization. + use_edge_zeroed_pixels : true # If True, the edge pixels of a pixelization are set to zero, which prevents unphysical values in the reconstructed solution at the edge of the pixelization. NOTE: this is applied ONLY when use_positive_only_solver is True -- with the positive-negative solver it has no effect. That scoping is deliberate, not an oversight. no_regularization_add_to_curvature_diag_value : 1.0e-3 # The default value added to the curvature matrix's diagonal when regularization is not applied to a linear object, which prevents inversion's failing due to the matrix being singular. use_border_relocator: false # If True, by default a pixelization's border is used to relocate all pixels outside its border to the border. nnls_jacobi_preconditioning: true # If True (default), the curvature matrix passed to jaxnnls.solve_nnls_primal is Jacobi-preconditioned (D Q D y = D q, x = D y). Fixes NaN backward-pass gradients on ill-conditioned Q and roughly halves forward solve time. Set False to restore the raw unpreconditioned solve. diff --git a/autoarray/inversion/inversion/abstract.py b/autoarray/inversion/inversion/abstract.py index 0a5efb72f..5163b6102 100644 --- a/autoarray/inversion/inversion/abstract.py +++ b/autoarray/inversion/inversion/abstract.py @@ -509,6 +509,10 @@ def reconstruction(self) -> np.ndarray: if self.settings.use_positive_only_solver: + # `use_edge_zeroed_pixels` is deliberately nested here rather than checked alongside + # `use_positive_only_solver`: edge-zeroing is scoped to the positive-only solver, and the + # positive-negative branch below solves the full system regardless of its value. This is + # intended, not an oversight -- do not "fix" it by hoisting the check out of this branch. if self.settings.use_edge_zeroed_pixels and self.has(cls=Mapper): # Use advanced indexing to select rows/columns diff --git a/autoarray/settings.py b/autoarray/settings.py index f03db4873..5ad59387e 100644 --- a/autoarray/settings.py +++ b/autoarray/settings.py @@ -169,6 +169,17 @@ def use_positive_only_solver(self): @property def use_edge_zeroed_pixels(self): + """ + Whether a mesh's edge pixels are excluded from the inversion and fixed to zero. + + This is consulted **only when `use_positive_only_solver` is `True`**. Under the + positive-negative solver the full system is solved and this setting has no effect, so the two + are not independent switches despite reading that way in `config/general.yaml`. + + That scoping is deliberate. It is called out here because the nesting is not visible from the + config, and has been mistaken for a bug (a setting "silently ignored") by someone reading the + control flow in `AbstractInversion.reconstruction` without it. + """ if self._use_edge_zeroed_pixels is None: return conf.instance["general"]["inversion"]["use_edge_zeroed_pixels"]