Skip to content

Commit d91500d

Browse files
committed
fix: form the reconstruction covariance via Cholesky, not elementwise sqrt
`reconstruction_noise_map_with_covariance` applied np.sqrt elementwise to the whole inverse of curvature_reg_matrix. The off-diagonals of a covariance matrix are covariances and are routinely negative, so every one was NaN by construction -- for any matrix, however well-conditioned -- and each call emitted "RuntimeWarning: invalid value encountered in sqrt". The property's docstring promised a matrix accounting for "the covariance of the noise between pixels"; the entries carrying that covariance were the broken ones. The 1D reconstruction_noise_map was NOT affected: np.sqrt is elementwise, so it commutes with taking the diagonal. Replaced with reconstruction_covariance_matrix, which returns C directly and forms it from a Cholesky factorization. An A/B against np.linalg.inv refuted two hypotheses and confirmed one: - inv gives negative diagonals on well-formed SPD matrices: REFUTED (0 across cond 1e3-1e15, n=400, 20 trials each) - inv is materially less accurate on the diagonal: REFUTED (matches cho_solve; at cond 1e15 inv was marginally better) - near-coincident mesh vertices degrade the inverse: REFUTED (regularization keeps the matrix PD, cond ~6.8e7 at duplicated columns) - inv returns asymmetric output: CONFIRMED (5.2e-7 at cond 1e12, vs 2.6e-16 for cho_solve) - inv silently succeeds on indefinite matrices: CONFIRMED The last is the substantive one. cho_factor raises LinAlgError on a matrix with a negative eigenvalue; inv succeeds and returns a plausible-looking covariance. At eigenvalue -1e-8 all 300 diagonals came back negative; at -1.0, zero did -- no NaN, no warning, no error, and wrong numbers. A covariance is only defined for a positive-definite matrix, so this adds a definiteness check the code lacked entirely. Both existing call sites already catch LinAlgError (inversion_plots.py:169 and :395), verified rather than assumed. reconstruction_noise_map now computes sqrt(diag(C)) directly. It was correct only incidentally before, via the elementwise sqrt; the invariant is now stated, and it cannot silently become a variance if the matrix changes. The old name stays as a deprecated alias. Its values do change -- diagonal from std-dev to variance, off-diagonals from NaN to covariances -- so the warning says so explicitly. Tests: the only prior assertion checked [0, 0], a diagonal element, which is why this shipped. Added off-diagonal finiteness under -W error::RuntimeWarning, exact symmetry, the sqrt(diag(C)) invariant, the indefinite-matrix raise, and the deprecation. Repointed two plotter monkeypatch sites to the new property. Full suite: 1150 passed, 1 skipped. The 3 test_transformer.py pynufft failures are pre-existing on a6b07cd and unrelated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0133X4XhMV91SFjzV2mK4Ejh
1 parent a6b07cd commit d91500d

3 files changed

Lines changed: 160 additions & 23 deletions

File tree

‎autoarray/inversion/inversion/abstract.py‎

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import copy
2+
import warnings
23

34
import numpy as np
45
from typing import Dict, List, Optional, Type, Union
@@ -836,27 +837,78 @@ def log_det_regularization_matrix_term(self) -> float:
836837
return self._log_det_symmetric_from(self.regularization_matrix_reduced)
837838

838839
@property
839-
def reconstruction_noise_map_with_covariance(self) -> np.ndarray:
840+
def reconstruction_covariance_matrix(self) -> np.ndarray:
840841
"""
841-
Returns the noise-map of the reconstruction as a two dimension matrix which accounts for the covariance
842-
of the noise between pixels.
842+
Returns the covariance matrix of the reconstruction, ``C = [F + reg_coeff*H]^-1``.
843+
844+
This is the inverse of the curvature matrix with regularization -- the same matrix used to solve for the
845+
reconstruction via the linear inversion. Its diagonal holds the variance of each reconstructed pixel and
846+
its off-diagonal entries the covariances between pixels; the off-diagonals are routinely negative, which is
847+
a property of the covariance and not an error.
848+
849+
For the RMS standard deviation of each pixel (the quantity used for scientific analysis) use
850+
`reconstruction_noise_map`, which takes the square root of this matrix's diagonal.
851+
852+
The inverse is formed from a Cholesky factorization rather than `np.linalg.inv`, for two reasons:
853+
854+
- `cho_factor` raises `LinAlgError` when the matrix is not positive-definite. `np.linalg.inv` raises only
855+
on an exactly singular matrix, so an indefinite `curvature_reg_matrix` -- which the inversion does
856+
encounter, hence `Settings.no_regularization_add_to_curvature_diag_value` -- previously returned a
857+
plausible-looking covariance with no error and no warning. A covariance is only defined for a
858+
positive-definite matrix, so failing here is correct and is handled by the callers in
859+
`inversion/plot/inversion_plots.py`.
860+
- `np.linalg.inv` is LU-based and exploits neither the symmetry nor the positive-definiteness this matrix
861+
has. Its output drifts out of symmetry as conditioning worsens (measured at ~5e-7 absolute at
862+
`cond ~ 1e12`, against ~3e-16 for the Cholesky solve), while a covariance matrix is symmetric by
863+
definition.
864+
865+
This property is NumPy-only: the input is coerced with `np.asarray`, so a JAX `curvature_reg_matrix`
866+
forces a device-to-host transfer. It is a post-fit diagnostic, not part of the likelihood, so it is not on
867+
the JIT path.
868+
869+
Returns
870+
-------
871+
The covariance matrix of the reconstruction, of shape [total_params, total_params].
872+
"""
873+
from scipy.linalg import cho_factor, cho_solve
874+
875+
matrix = np.asarray(self.curvature_reg_matrix)
876+
877+
covariance = cho_solve(
878+
cho_factor(matrix), np.eye(matrix.shape[0], dtype=matrix.dtype)
879+
)
880+
881+
# cho_solve is accurate but not bitwise symmetric; a covariance matrix is symmetric by definition.
882+
return 0.5 * (covariance + covariance.T)
843883

844-
The diagonal of this matrix is the noise-map of the reconstruction, which can be used for analysing the
845-
reconstruction with noise properties that are representative of the fit and therefore should be used
846-
for any scientific analysis (e.g. source reconstructions of strong lenses).
884+
@property
885+
def reconstruction_noise_map_with_covariance(self) -> np.ndarray:
886+
"""
887+
Deprecated alias of `reconstruction_covariance_matrix`.
847888
848-
This noise-map is defined as the RMS standard deviation of the noise in every pixel of the reconstruction.
849-
This definition is identical to the `noise_map` attributes of dataset objects.
889+
This property previously returned ``np.sqrt(np.linalg.inv(curvature_reg_matrix))`` -- an elementwise square
890+
root of the whole covariance matrix. Because the off-diagonal entries of a covariance matrix are routinely
891+
negative, every such entry was `NaN` by construction, for any input matrix however well-conditioned, and
892+
each call emitted `RuntimeWarning: invalid value encountered in sqrt`.
850893
851-
It is computed as the square root of the inverse of the curvature matrix with regularization, which is the
852-
same matrix used to solve for the reconstruction via the linear inversion.
894+
It now returns the covariance matrix itself, so the values differ: the diagonal holds variances rather
895+
than standard deviations, and the off-diagonals hold covariances rather than `NaN`.
853896
854897
Returns
855898
-------
856-
The noise-map of the reconstruction as a two dimension matrix which accounts for the covariance of the noise
857-
between pixels.
899+
The covariance matrix of the reconstruction (see `reconstruction_covariance_matrix`).
858900
"""
859-
return np.sqrt(np.linalg.inv(self.curvature_reg_matrix))
901+
warnings.warn(
902+
"`reconstruction_noise_map_with_covariance` is deprecated; use "
903+
"`reconstruction_covariance_matrix` instead. Note the values have changed: it now returns the "
904+
"covariance matrix, so its diagonal holds variances rather than standard deviations (the previous "
905+
"elementwise square root made every off-diagonal NaN). For the RMS noise of each pixel use "
906+
"`reconstruction_noise_map`.",
907+
DeprecationWarning,
908+
stacklevel=2,
909+
)
910+
911+
return self.reconstruction_covariance_matrix
860912

861913
@property
862914
def reconstruction_noise_map(self):
@@ -870,15 +922,15 @@ def reconstruction_noise_map(self):
870922
The noise-map of the reconstruction is the RMS standard deviation of the noise in every pixel of the
871923
reconstruction. This definition is identical to the `noise_map` attributes of dataset objects.
872924
873-
It is computed as the square root of the diagonal of the `reconstruction_noise_map_with_covariance` matrix,
874-
which is the same matrix used to solve for the reconstruction via the linear inversion.
925+
It is computed as the square root of the diagonal of `reconstruction_covariance_matrix`, which is the
926+
inverse of the same matrix used to solve for the reconstruction via the linear inversion.
875927
876928
Returns
877929
-------
878930
The noise-map of the reconstruction as a one dimensional ndarray, which does not account for the covariance
879931
of the noise between pixels.
880932
"""
881-
return np.diagonal(self.reconstruction_noise_map_with_covariance)
933+
return np.sqrt(np.diag(self.reconstruction_covariance_matrix))
882934

883935
@property
884936
def reconstruction_noise_map_dict(self) -> Dict[LinearObj, np.ndarray]:

‎test_autoarray/inversion/inversion/test_abstract.py‎

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import warnings
2+
13
import numpy as np
24
import pytest
35

@@ -681,14 +683,97 @@ def test__reconstruction_noise_map__asymmetric_curvature_reg_matrix__correct_dia
681683

682684
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
683685

684-
assert inversion.reconstruction_noise_map_with_covariance[0, 0] == pytest.approx(
685-
np.sqrt(2.5), 1.0e-2
686-
)
686+
assert inversion.reconstruction_covariance_matrix[0, 0] == pytest.approx(2.5, 1.0e-2)
687687
assert inversion.reconstruction_noise_map == pytest.approx(
688688
np.sqrt(np.array([2.5, 1.0, 0.5])), 1.0e-3
689689
)
690690

691691

692+
def test__reconstruction_covariance_matrix__off_diagonals_are_finite_and_negative():
693+
"""
694+
The off-diagonal entries of a covariance matrix are covariances and are routinely negative.
695+
696+
`reconstruction_covariance_matrix` previously applied `np.sqrt` elementwise to the whole inverse, so every
697+
negative off-diagonal became NaN by construction -- for any matrix, however well-conditioned -- while
698+
emitting `RuntimeWarning: invalid value encountered in sqrt`. Only the [0, 0] diagonal element was asserted,
699+
so nothing caught it.
700+
"""
701+
curvature_reg_matrix = np.array([[1.0, 1.0, 1.0], [1.0, 2.0, 1.0], [1.0, 1.0, 3.0]])
702+
703+
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
704+
705+
with warnings.catch_warnings():
706+
warnings.simplefilter("error", RuntimeWarning)
707+
covariance = inversion.reconstruction_covariance_matrix
708+
709+
assert np.all(np.isfinite(covariance))
710+
711+
# this matrix has anti-correlated pixels, so the off-diagonals are genuinely negative
712+
assert covariance[0, 1] < 0.0
713+
assert covariance == pytest.approx(np.linalg.inv(curvature_reg_matrix), 1.0e-8)
714+
715+
716+
def test__reconstruction_covariance_matrix__is_exactly_symmetric():
717+
"""A covariance matrix is symmetric by definition; `np.linalg.inv` drifts out of symmetry."""
718+
rng = np.random.default_rng(1234)
719+
q, _ = np.linalg.qr(rng.standard_normal((25, 25)))
720+
curvature_reg_matrix = (q * np.logspace(0, 9, 25)) @ q.T
721+
curvature_reg_matrix = 0.5 * (curvature_reg_matrix + curvature_reg_matrix.T)
722+
723+
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
724+
725+
covariance = inversion.reconstruction_covariance_matrix
726+
727+
assert covariance == pytest.approx(covariance.T, abs=1.0e-15)
728+
729+
730+
def test__reconstruction_noise_map__is_sqrt_of_covariance_diagonal():
731+
"""
732+
The invariant, asserted directly rather than via hand-computed values.
733+
734+
`reconstruction_noise_map` used to be `np.diagonal(...)` of an already-square-rooted matrix, which was
735+
correct only incidentally -- because `np.sqrt` is elementwise. It now takes the square root of the
736+
covariance diagonal itself, so the relationship is stated rather than emergent.
737+
"""
738+
curvature_reg_matrix = np.array([[1.0, 1.0, 1.0], [1.0, 2.0, 1.0], [1.0, 1.0, 3.0]])
739+
740+
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
741+
742+
assert inversion.reconstruction_noise_map == pytest.approx(
743+
np.sqrt(np.diag(inversion.reconstruction_covariance_matrix)), 1.0e-12
744+
)
745+
746+
747+
def test__reconstruction_covariance_matrix__raises_on_a_non_positive_definite_matrix():
748+
"""
749+
A covariance is only defined for a positive-definite matrix.
750+
751+
`np.linalg.inv` raises only on an exactly singular matrix, so an indefinite `curvature_reg_matrix` returned
752+
a plausible-looking covariance with no error and no warning. The Cholesky factorization rejects it, and the
753+
plotting and CSV callers already catch `LinAlgError`.
754+
"""
755+
# symmetric, non-singular, but indefinite (eigenvalues +1 and -1)
756+
curvature_reg_matrix = np.array([[0.0, 1.0], [1.0, 0.0]])
757+
758+
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
759+
760+
assert np.isfinite(np.linalg.inv(curvature_reg_matrix)).all() # inv is silent here
761+
762+
with pytest.raises(np.linalg.LinAlgError):
763+
inversion.reconstruction_covariance_matrix
764+
765+
766+
def test__reconstruction_noise_map_with_covariance__is_deprecated_alias():
767+
curvature_reg_matrix = np.array([[1.0, 1.0, 1.0], [1.0, 2.0, 1.0], [1.0, 1.0, 3.0]])
768+
769+
inversion = aa.m.MockInversion(curvature_reg_matrix=curvature_reg_matrix)
770+
771+
with pytest.warns(DeprecationWarning, match="reconstruction_covariance_matrix"):
772+
covariance = inversion.reconstruction_noise_map_with_covariance
773+
774+
assert covariance == pytest.approx(inversion.reconstruction_covariance_matrix, 1.0e-12)
775+
776+
692777
def test__max_pixel_list_from_and_centre__returns_top_pixels_and_brightest_centre():
693778

694779
source_plane_mesh_grid = aa.Grid2DIrregular(

‎test_autoarray/inversion/plot/test_inversion_plotters.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ def test__inversion_subplot_of_mapper__singular_curvature_reg_matrix(
7979

8080
monkeypatch.setattr(
8181
type(inversion),
82-
"reconstruction_noise_map_with_covariance",
83-
property(lambda self: np.sqrt(np.linalg.inv(np.zeros((params, params))))),
82+
"reconstruction_covariance_matrix",
83+
property(lambda self: np.linalg.inv(np.zeros((params, params)))),
8484
)
8585

8686
with pytest.raises(np.linalg.LinAlgError):
@@ -107,8 +107,8 @@ def test__save_reconstruction_csv__singular_curvature_reg_matrix(
107107

108108
monkeypatch.setattr(
109109
type(inversion),
110-
"reconstruction_noise_map_with_covariance",
111-
property(lambda self: np.sqrt(np.linalg.inv(np.zeros((params, params))))),
110+
"reconstruction_covariance_matrix",
111+
property(lambda self: np.linalg.inv(np.zeros((params, params)))),
112112
)
113113

114114
with pytest.raises(np.linalg.LinAlgError):

0 commit comments

Comments
 (0)