Skip to content

Commit 7961607

Browse files
authored
Merge pull request #751 from gaoflow/fix/lsqr-calc-var-elementwise
Fix elementwise variance accumulation in lsqr (closes #639)
2 parents 1b9487f + 5938fef commit 7961607

3 files changed

Lines changed: 30 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
Changelog
22
=========
33

4+
# 2.8.0
5+
* Fixed element-wise variance accumulation in `pylops.optimization.cls_basic.LSQR`
6+
and `pylops.lsqr`, which previously assigned the same value to every entry of
7+
`var` (#639)
8+
49
# 2.7.0
510
* Added cubic spline interpolation operator via
611
`pylops.signalprocessing.interpspline.InterpCubicSpline` (also interfaceable via

pylops/optimization/cls_basic.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1271,8 +1271,9 @@ def step(self, x: NDArray, show: bool = False) -> NDArray:
12711271
self.ncp.add(self.v, self.w, out=self.w)
12721272
self.ddnorm = self.ddnorm + self.ncp.linalg.norm(self.dk) ** 2.0
12731273
if self.calc_var:
1274+
# accumulate the element-wise square of dk (one variance per unknown)
12741275
self.var = self.var + to_numpy_conditional(
1275-
self.var, self.ncp.dot(self.dk, self.dk)
1276+
self.var, self.ncp.square(self.dk)
12761277
)
12771278

12781279
# use a plane rotation on the right to eliminate the

pytests/test_solver.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,29 @@ def test_lsqr_pylops_scipy(par):
410410
assert_array_almost_equal(xinv_sp, x, decimal=4)
411411

412412

413+
@pytest.mark.skipif(
414+
int(os.environ.get("TEST_CUPY_PYLOPS", 0)) == 1, reason="Not CuPy enabled"
415+
)
416+
@pytest.mark.parametrize("par", [(par3), (par4)])
417+
def test_lsqr_calc_var(par):
418+
"""Compare PyLops and scipy LSQR variance computation for the diagonal of
419+
(A^H A)^-1 (issue #639)."""
420+
np.random.seed(10)
421+
422+
A = np.random.normal(0, 1, (par["ny"], par["nx"]))
423+
Aop = MatrixMult(A, dtype=par["dtype"])
424+
y = Aop * (np.ones(par["nx"]))
425+
426+
# niter is only a ceiling (atol/btol stop earlier); use scipy's default of
427+
# 2 * nx so both implementations run for the same number of iterations.
428+
niter = 2 * par["nx"]
429+
var = lsqr(Aop, y, x0=None, niter=niter, atol=1e-8, btol=1e-8)[9]
430+
var_sp = sp_lsqr(Aop, y, iter_lim=niter, atol=1e-8, btol=1e-8, calc_var=True)[9]
431+
432+
assert not np.allclose(var, var[0])
433+
assert_array_almost_equal(var, var_sp, decimal=6)
434+
435+
413436
@pytest.mark.parametrize(
414437
"par", [(par1), (par2), (par3), (par4), (par1j), (par2j), (par3j), (par3j)]
415438
)

0 commit comments

Comments
 (0)