Skip to content

Commit db94e80

Browse files
Jammy2211claude
andcommitted
fix: apply_over_sampling keeps the sparse operator; cache operated_mapping_matrix_list
apply_over_sampling rebuilt Imaging without sparse_operator (and without noise_covariance_matrix), so applying the sparse operator before over-sampling silently fell back to the dense InversionImagingMapping route. Both are now carried through; neither depends on over-sampling. operated_mapping_matrix_list is now a cached_property (imaging and interferometer), so the dense route convolves / NUFFT-transforms the mapping matrix once per likelihood evaluation instead of twice. apply_mask / apply_noise_scaling warn when they discard a (stale) operator; apply_sparse_operator_cpu gains the convolve_over_sample_size > 1 guard; the sparse methods and apply_noise_scaling carry convolve_over_sample_size_*. DR1 tile: operator-first 1011 -> 169.5 ms (sparse), dense 1166.5 -> 592.8 ms, logL bit-identical. test_autoarray: 1743 passed. Fixes #585 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9428eca commit db94e80

7 files changed

Lines changed: 232 additions & 6 deletions

File tree

‎autoarray/dataset/imaging/dataset.py‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,36 @@ def _validate_convolve_over_sample_size(
6161
)
6262

6363

64+
_SPARSE_OPERATOR_CONVOLVE_OVER_SAMPLE_SIZE_ERROR = (
65+
"The sparse linear algebra formalism precomputes PSF products at "
66+
"image resolution and is incompatible with an oversampled PSF "
67+
"(convolve_over_sample_size > 1)."
68+
)
69+
70+
71+
def _warn_sparse_operator_discarded(sparse_operator, method_name: str) -> None:
72+
"""
73+
Log a warning when a dataset rebuild discards an attached sparse operator.
74+
75+
The sparse operator precomputes PSF products of every pair of masked noise-map
76+
values, so it is invalidated by any change to the mask or noise-map. Rebuilds
77+
which make such changes (e.g. `apply_mask`, `apply_noise_scaling`) therefore do
78+
not carry it, and the returned dataset falls back to the (slower) mapping matrix
79+
formalism unless the operator is re-applied. This warning stops that fallback
80+
from happening silently.
81+
"""
82+
if sparse_operator is None:
83+
return
84+
85+
logger.warning(
86+
f"IMAGING - {method_name} changes the mask or noise-map, which invalidates the "
87+
f"sparse operator attached to this dataset, so it has been discarded. Pixelized "
88+
f"reconstructions will use the slower mapping matrix formalism unless the sparse "
89+
f"operator is re-applied (via apply_sparse_operator() or "
90+
f"apply_sparse_operator_cpu()) as the last dataset operation."
91+
)
92+
93+
6494
class Imaging(AbstractDataset):
6595
def __init__(
6696
self,
@@ -439,6 +469,10 @@ def apply_mask(self, mask: Mask2D) -> "Imaging":
439469
values=self.over_sample_size_pixelization.native, mask=mask
440470
)
441471

472+
_warn_sparse_operator_discarded(
473+
sparse_operator=self.sparse_operator, method_name="apply_mask"
474+
)
475+
442476
dataset = Imaging(
443477
data=data,
444478
noise_map=noise_map,
@@ -520,13 +554,19 @@ def apply_noise_scaling(
520554

521555
noise_map = Array2D(values=noise_map, mask=self.data.mask)
522556

557+
_warn_sparse_operator_discarded(
558+
sparse_operator=self.sparse_operator, method_name="apply_noise_scaling"
559+
)
560+
523561
dataset = Imaging(
524562
data=data,
525563
noise_map=noise_map,
526564
psf=self.psf,
527565
noise_covariance_matrix=self.noise_covariance_matrix,
528566
over_sample_size_lp=self.over_sample_size_lp,
529567
over_sample_size_pixelization=self.over_sample_size_pixelization,
568+
convolve_over_sample_size_lp=self.convolve_over_sample_size_lp,
569+
convolve_over_sample_size_pixelization=self.convolve_over_sample_size_pixelization,
530570
check_noise_map=False,
531571
)
532572

@@ -551,6 +591,10 @@ def apply_over_sampling(
551591
This function resets the cached properties so that the new over sampling is used in the grid and grid
552592
pixelization.
553593
594+
The `noise_covariance_matrix` and any attached `sparse_operator` are carried over, as neither depends on
595+
the over sampling (the sparse operator depends only on the noise-map, PSF kernel and mask). This means
596+
`apply_sparse_operator()` / `apply_sparse_operator_cpu()` may be called before or after this method.
597+
554598
Parameters
555599
----------
556600
over_sample_size_lp
@@ -566,12 +610,14 @@ def apply_over_sampling(
566610
data=self.data,
567611
noise_map=self.noise_map,
568612
psf=self.psf,
613+
noise_covariance_matrix=self.noise_covariance_matrix,
569614
over_sample_size_lp=over_sample_size_lp or self.over_sample_size_lp,
570615
over_sample_size_pixelization=over_sample_size_pixelization
571616
or self.over_sample_size_pixelization,
572617
convolve_over_sample_size_lp=self.convolve_over_sample_size_lp,
573618
convolve_over_sample_size_pixelization=self.convolve_over_sample_size_pixelization,
574619
check_noise_map=False,
620+
sparse_operator=self.sparse_operator,
575621
)
576622

577623
return dataset
@@ -620,9 +666,7 @@ def apply_sparse_operator(
620666

621667
if self.psf is not None and self.psf.convolve_over_sample_size > 1:
622668
raise exc.DatasetException(
623-
"The sparse linear algebra formalism precomputes PSF products at "
624-
"image resolution and is incompatible with an oversampled PSF "
625-
"(convolve_over_sample_size > 1)."
669+
_SPARSE_OPERATOR_CONVOLVE_OVER_SAMPLE_SIZE_ERROR
626670
)
627671

628672
logger.info(
@@ -645,6 +689,8 @@ def apply_sparse_operator(
645689
noise_covariance_matrix=self.noise_covariance_matrix,
646690
over_sample_size_lp=self.over_sample_size_lp,
647691
over_sample_size_pixelization=self.over_sample_size_pixelization,
692+
convolve_over_sample_size_lp=self.convolve_over_sample_size_lp,
693+
convolve_over_sample_size_pixelization=self.convolve_over_sample_size_pixelization,
648694
check_noise_map=False,
649695
sparse_operator=sparse_operator,
650696
)
@@ -668,6 +714,11 @@ def apply_sparse_operator_cpu(
668714
A new `Imaging` dataset with a precomputed Numba-based sparse operator attached,
669715
enabling efficient pixelized source reconstruction on CPU hardware.
670716
"""
717+
if self.psf is not None and self.psf.convolve_over_sample_size > 1:
718+
raise exc.DatasetException(
719+
_SPARSE_OPERATOR_CONVOLVE_OVER_SAMPLE_SIZE_ERROR
720+
)
721+
671722
try:
672723
import numba
673724
except ModuleNotFoundError:
@@ -711,6 +762,8 @@ def apply_sparse_operator_cpu(
711762
noise_covariance_matrix=self.noise_covariance_matrix,
712763
over_sample_size_lp=self.over_sample_size_lp,
713764
over_sample_size_pixelization=self.over_sample_size_pixelization,
765+
convolve_over_sample_size_lp=self.convolve_over_sample_size_lp,
766+
convolve_over_sample_size_pixelization=self.convolve_over_sample_size_pixelization,
714767
check_noise_map=False,
715768
sparse_operator=sparse_operator,
716769
)

‎autoarray/inversion/inversion/abstract.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,17 @@ def operated_mapping_matrix(self) -> np.ndarray:
299299
300300
If there are multiple linear objects, the blurred mapping matrices are stacked such that their simultaneous
301301
linear equations are solved simultaneously.
302+
303+
The dataset-specific `operated_mapping_matrix_list` is itself cached, so for a single linear object its
304+
matrix is returned directly rather than copied via `hstack`, avoiding holding two identical copies of
305+
the (large) operated mapping matrix in memory.
302306
"""
303-
return self._xp.hstack(self.operated_mapping_matrix_list)
307+
operated_mapping_matrix_list = self.operated_mapping_matrix_list
308+
309+
if len(operated_mapping_matrix_list) == 1:
310+
return operated_mapping_matrix_list[0]
311+
312+
return self._xp.hstack(operated_mapping_matrix_list)
304313

305314
@property
306315
def data_vector(self) -> np.ndarray:

‎autoarray/inversion/inversion/imaging/abstract.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def mapping_matrix_list(self) -> List[np.ndarray]:
116116
"""
117117
return [linear_obj.mapping_matrix for linear_obj in self.linear_obj_list]
118118

119-
@property
119+
@cached_property
120120
def operated_mapping_matrix_list(self) -> List[np.ndarray]:
121121
"""
122122
The `operated_mapping_matrix` of a linear object describes the mappings between the observed data's values and

‎autoarray/inversion/inversion/interferometer/abstract.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import numpy as np
22
from typing import Dict, List, Optional, Union
33

4+
from autonerves import cached_property
5+
46
from autoarray import exc
57
from autoarray.dataset.interferometer.dataset import Interferometer
68
from autoarray.inversion.inversion.dataset_interface import DatasetInterface
@@ -61,7 +63,7 @@ def transformer(self):
6163
def mask(self) -> Mask2D:
6264
return self.transformer.real_space_mask
6365

64-
@property
66+
@cached_property
6567
def operated_mapping_matrix_list(self) -> List[np.ndarray]:
6668
"""
6769
The `operated_mapping_matrix` of a linear object describes the mappings between the observed data's values

‎test_autoarray/dataset/imaging/test_dataset.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import copy
2+
import logging
23

34
import numpy as np
45
import pytest
56

67
import autoarray as aa
78

89
from autoarray import exc
10+
from autoarray.inversion.inversion.imaging_numba.sparse import (
11+
InversionImagingSparseNumba,
12+
)
913
from pathlib import Path
1014

1115
test_data_path = Path(Path(__file__).resolve().parent) / "files"
@@ -504,3 +508,95 @@ def test__convolve_over_sample_size__sparse_operator_guard():
504508

505509
with pytest.raises(aa.exc.DatasetException):
506510
dataset.apply_sparse_operator()
511+
512+
513+
def test__apply_over_sampling__keeps_sparse_operator_and_noise_covariance(
514+
masked_imaging_7x7, masked_imaging_covariance_7x7, delaunay_mapper_9_3x3
515+
):
516+
# The sparse operator depends only on the noise-map, PSF kernel and mask, so it
517+
# stays valid when only the over sampling changes and must not be dropped.
518+
dataset_operator_first = (
519+
masked_imaging_7x7.apply_sparse_operator_cpu().apply_over_sampling(
520+
over_sample_size_lp=2
521+
)
522+
)
523+
524+
assert dataset_operator_first.sparse_operator is not None
525+
526+
dataset_operator_last = masked_imaging_7x7.apply_over_sampling(
527+
over_sample_size_lp=2
528+
).apply_sparse_operator_cpu()
529+
530+
inversion_operator_first = aa.Inversion(
531+
dataset=dataset_operator_first,
532+
linear_obj_list=[delaunay_mapper_9_3x3],
533+
)
534+
inversion_operator_last = aa.Inversion(
535+
dataset=dataset_operator_last,
536+
linear_obj_list=[delaunay_mapper_9_3x3],
537+
)
538+
539+
assert isinstance(inversion_operator_first, InversionImagingSparseNumba)
540+
assert inversion_operator_first.reconstruction == pytest.approx(
541+
inversion_operator_last.reconstruction, 1.0e-8
542+
)
543+
assert inversion_operator_first.log_det_curvature_reg_matrix_term == pytest.approx(
544+
inversion_operator_last.log_det_curvature_reg_matrix_term, 1.0e-8
545+
)
546+
547+
# The noise covariance matrix is independent of over sampling and is also kept.
548+
dataset_covariance = masked_imaging_covariance_7x7.apply_over_sampling(
549+
over_sample_size_lp=2
550+
)
551+
552+
assert dataset_covariance.noise_covariance_matrix == pytest.approx(
553+
masked_imaging_covariance_7x7.noise_covariance_matrix, 1.0e-8
554+
)
555+
556+
557+
def test__apply_mask_and_noise_scaling__warn_when_sparse_operator_discarded(
558+
masked_imaging_7x7, caplog
559+
):
560+
dataset = masked_imaging_7x7.apply_sparse_operator_cpu()
561+
562+
with caplog.at_level(logging.WARNING, logger="autoarray.dataset.imaging.dataset"):
563+
masked = dataset.apply_mask(mask=masked_imaging_7x7.mask)
564+
565+
assert masked.sparse_operator is None
566+
assert "sparse operator" in caplog.text
567+
assert "apply_mask" in caplog.text
568+
569+
caplog.clear()
570+
571+
with caplog.at_level(logging.WARNING, logger="autoarray.dataset.imaging.dataset"):
572+
scaled = dataset.apply_noise_scaling(mask=masked_imaging_7x7.mask)
573+
574+
assert scaled.sparse_operator is None
575+
assert "sparse operator" in caplog.text
576+
assert "apply_noise_scaling" in caplog.text
577+
578+
# No warning when there is no operator to discard.
579+
caplog.clear()
580+
581+
with caplog.at_level(logging.WARNING, logger="autoarray.dataset.imaging.dataset"):
582+
masked_imaging_7x7.apply_mask(mask=masked_imaging_7x7.mask)
583+
584+
assert "sparse operator" not in caplog.text
585+
586+
587+
def test__convolve_over_sample_size__sparse_operator_cpu_guard():
588+
data = aa.Array2D.no_mask(values=np.ones((11, 11)), pixel_scales=1.0)
589+
noise_map = aa.Array2D.no_mask(values=np.ones((11, 11)), pixel_scales=1.0)
590+
kernel_fine = aa.Array2D.no_mask(values=np.ones((9, 9)), pixel_scales=0.5)
591+
psf = aa.Convolver(kernel=kernel_fine)
592+
593+
dataset = aa.Imaging(
594+
data=data,
595+
noise_map=noise_map,
596+
psf=psf,
597+
over_sample_size_pixelization=2,
598+
convolve_over_sample_size_pixelization=2,
599+
)
600+
601+
with pytest.raises(aa.exc.DatasetException):
602+
dataset.apply_sparse_operator_cpu()

‎test_autoarray/inversion/inversion/imaging/test_imaging.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,35 @@ def test__mapping_matrix_over_sampled_for__kxs__full_bin_reproduces_mapping_matr
267267
# s=2 rows mean-binned to image resolution also reproduce mapping_matrix.
268268
binned = m_s2.reshape(n_pix, 4, mapping_matrix.shape[1]).mean(axis=1)
269269
assert binned == pytest.approx(mapping_matrix, abs=1.0e-14)
270+
271+
272+
def test__operated_mapping_matrix_list__psf_convolution_performed_once_per_linear_obj(
273+
masked_imaging_7x7, delaunay_mapper_9_3x3, monkeypatch
274+
):
275+
# The dense (mapping) route reaches `operated_mapping_matrix_list` from the
276+
# curvature matrix / data vector (via `operated_mapping_matrix`) and again from
277+
# `mapped_reconstructed_data_dict`. It must be cached so the PSF convolution of
278+
# each linear object's mapping matrix happens once per inversion.
279+
calls = []
280+
281+
convolved_mapping_matrix_from = aa.Convolver.convolved_mapping_matrix_from
282+
283+
def counted(self, *args, **kwargs):
284+
calls.append(1)
285+
return convolved_mapping_matrix_from(self, *args, **kwargs)
286+
287+
monkeypatch.setattr(aa.Convolver, "convolved_mapping_matrix_from", counted)
288+
289+
inversion = aa.Inversion(
290+
dataset=masked_imaging_7x7,
291+
linear_obj_list=[delaunay_mapper_9_3x3],
292+
)
293+
294+
assert isinstance(inversion, aa.InversionImagingMapping)
295+
296+
inversion.log_det_curvature_reg_matrix_term
297+
inversion.reconstruction
298+
inversion.mapped_reconstructed_operated_data
299+
inversion.mapped_reconstructed_data
300+
301+
assert len(calls) == 1

‎test_autoarray/inversion/inversion/interferometer/test_interferometer.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,3 +1176,37 @@ def test__interferometer_mapping__curvature_matrix_and_data_vector_evaluated_onc
11761176

11771177
assert counts == {"curvature_matrix": 1, "data_vector": 1}
11781178
assert log_evidence_terms == reference
1179+
1180+
1181+
def test__operated_mapping_matrix_list__transform_performed_once_per_linear_obj(
1182+
interferometer_7_no_fft, rectangular_mapper_7x7_3x3, monkeypatch
1183+
):
1184+
# The dense (mapping) route reaches `operated_mapping_matrix_list` from the
1185+
# curvature matrix / data vector (via `operated_mapping_matrix`) and again from
1186+
# `mapped_reconstructed_data_dict`. It must be cached so the Fourier transform of
1187+
# each linear object's mapping matrix happens once per inversion.
1188+
calls = []
1189+
1190+
transformer_cls = type(interferometer_7_no_fft.transformer)
1191+
transform_mapping_matrix = transformer_cls.transform_mapping_matrix
1192+
1193+
def counted(self, *args, **kwargs):
1194+
calls.append(1)
1195+
return transform_mapping_matrix(self, *args, **kwargs)
1196+
1197+
monkeypatch.setattr(transformer_cls, "transform_mapping_matrix", counted)
1198+
1199+
inversion = aa.Inversion(
1200+
dataset=interferometer_7_no_fft,
1201+
linear_obj_list=[rectangular_mapper_7x7_3x3],
1202+
settings=aa.Settings(),
1203+
)
1204+
1205+
assert isinstance(inversion, aa.InversionInterferometerMapping)
1206+
1207+
inversion.log_det_curvature_reg_matrix_term
1208+
inversion.reconstruction
1209+
inversion.mapped_reconstructed_operated_data
1210+
inversion.mapped_reconstructed_data
1211+
1212+
assert len(calls) == 1

0 commit comments

Comments
 (0)