Skip to content

Commit 84b9ed4

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0133X4XhMV91SFjzV2mK4Ejh
1 parent 9930323 commit 84b9ed4

3 files changed

Lines changed: 16 additions & 1 deletion

File tree

autoarray/config/general.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ psf:
33
inversion:
44
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.
55
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.
6-
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.
6+
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.
77
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.
88
use_border_relocator: false # If True, by default a pixelization's border is used to relocate all pixels outside its border to the border.
99
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.

autoarray/inversion/inversion/abstract.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,10 @@ def reconstruction(self) -> np.ndarray:
509509

510510
if self.settings.use_positive_only_solver:
511511

512+
# `use_edge_zeroed_pixels` is deliberately nested here rather than checked alongside
513+
# `use_positive_only_solver`: edge-zeroing is scoped to the positive-only solver, and the
514+
# positive-negative branch below solves the full system regardless of its value. This is
515+
# intended, not an oversight -- do not "fix" it by hoisting the check out of this branch.
512516
if self.settings.use_edge_zeroed_pixels and self.has(cls=Mapper):
513517

514518
# Use advanced indexing to select rows/columns

autoarray/settings.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,17 @@ def use_positive_only_solver(self):
169169

170170
@property
171171
def use_edge_zeroed_pixels(self):
172+
"""
173+
Whether a mesh's edge pixels are excluded from the inversion and fixed to zero.
174+
175+
This is consulted **only when `use_positive_only_solver` is `True`**. Under the
176+
positive-negative solver the full system is solved and this setting has no effect, so the two
177+
are not independent switches despite reading that way in `config/general.yaml`.
178+
179+
That scoping is deliberate. It is called out here because the nesting is not visible from the
180+
config, and has been mistaken for a bug (a setting "silently ignored") by someone reading the
181+
control flow in `AbstractInversion.reconstruction` without it.
182+
"""
172183
if self._use_edge_zeroed_pixels is None:
173184
return conf.instance["general"]["inversion"]["use_edge_zeroed_pixels"]
174185

0 commit comments

Comments
 (0)