From 658d23468a14182b96728c9743668f8345f4d720 Mon Sep 17 00:00:00 2001 From: AHMETHAKANBEZIR1 Date: Thu, 15 Oct 2026 23:03:55 +0300 Subject: [PATCH 1/6] Fix: separate Split Bregman inner solver options --- examples/plot_tvreg.py | 8 +- pylops/optimization/cls_sparsity.py | 11 +++ pylops/optimization/sparsity.py | 11 +++ pytests/test_sparsity.py | 134 +++++++++++++++++++++++++++- 4 files changed, 161 insertions(+), 3 deletions(-) diff --git a/examples/plot_tvreg.py b/examples/plot_tvreg.py index 30692eb0..a3baa81e 100755 --- a/examples/plot_tvreg.py +++ b/examples/plot_tvreg.py @@ -76,6 +76,10 @@ ############################################################################### # Now we impose blockiness in the solution using the Split Bregman solver +# Inner solver options can also be supplied through ``kwargs_solver``. For +# example, use ``engine="pylops", kwargs_solver={"niter": 30, "tol": 1e-14}`` +# to tighten CGLS independently of the outer ``tol`` below. With the default +# SciPy engine, use LSQR options such as ``iter_lim``, ``atol``, and ``btol``. Dop = pylops.FirstDerivative(nx, edge=True, kind="backward") mu = 0.01 lamda = 0.3 @@ -92,7 +96,7 @@ epsRL1s=[lamda], tol=1e-4, tau=1.0, - **dict(iter_lim=30, damp=1e-10) + kwargs_solver=dict(iter_lim=30, damp=1e-10), )[0] plt.figure(figsize=(10, 5)) @@ -175,7 +179,7 @@ tol=1e-4, tau=1.0, show=False, - **dict(iter_lim=5, damp=1e-4) + **dict(iter_lim=5, damp=1e-4), )[0] xinv = np.real(xinv.reshape(ny, nx)) diff --git a/pylops/optimization/cls_sparsity.py b/pylops/optimization/cls_sparsity.py index 3f14cc13..5e1550ef 100644 --- a/pylops/optimization/cls_sparsity.py +++ b/pylops/optimization/cls_sparsity.py @@ -3302,6 +3302,8 @@ def solve( show: bool = False, itershow: tuple[int, int, int] = (10, 10, 10), show_inner: bool = False, + *, + kwargs_solver: dict | None = None, **kwargs_lsqr, ) -> tuple[NDArray, int, NDArray]: r"""Run entire solver @@ -3375,6 +3377,14 @@ def solve( three element of the list. show_inner : :obj:`bool`, optional Display iteration logs of the solver of the :math:`\mathbf{x}`-subproblem + kwargs_solver : :obj:`dict`, optional + .. versionadded:: 2.9.0 + + Keyword arguments for the inner solver, independent of the outer + stopping criteria. For example, ``kwargs_solver={"tol": 1e-14}`` + sets the inner CGLS tolerance without changing the outer ``tol``. + Entries override duplicate values in ``**kwargs_lsqr``. The dictionary + is not modified. **kwargs_lsqr Arbitrary keyword arguments for the solver of the :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm @@ -3409,6 +3419,7 @@ def solve( preallocate=preallocate, show=show, ) + kwargs_lsqr = {**kwargs_lsqr, **(kwargs_solver or {})} x = self.run( x, engine=engine, diff --git a/pylops/optimization/sparsity.py b/pylops/optimization/sparsity.py index 3cbf6fce..8cfde6c3 100644 --- a/pylops/optimization/sparsity.py +++ b/pylops/optimization/sparsity.py @@ -727,6 +727,8 @@ def splitbregman( show_inner: bool = False, callback: Callable | None = None, preallocate: bool = False, + *, + kwargs_solver: dict | None = None, **kwargs_lsqr, ) -> tuple[NDArray, int, NDArray]: r"""Split Bregman for mixed L2-L1 norms. @@ -814,6 +816,14 @@ def splitbregman( Pre-allocate all variables used by the solver. Note that if ``y`` is a JAX array, this option is ignored and variables are not pre-allocated since JAX does not support in-place operations. + kwargs_solver : :obj:`dict`, optional + .. versionadded:: 2.9.0 + + Keyword arguments for the inner solver, independent of the outer + stopping criteria. For example, ``kwargs_solver={"tol": 1e-14}`` + sets the inner CGLS tolerance without changing the outer ``tol``. + Entries override duplicate values in ``**kwargs_lsqr``. The dictionary + is not modified. **kwargs_lsqr Arbitrary keyword arguments for the solver of the :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm @@ -867,6 +877,7 @@ def splitbregman( show=show, itershow=itershow, show_inner=show_inner, + kwargs_solver=kwargs_solver, **kwargs_lsqr, ) return xinv, itn_out, cost diff --git a/pytests/test_sparsity.py b/pytests/test_sparsity.py index d35b1668..b09fea5c 100644 --- a/pytests/test_sparsity.py +++ b/pytests/test_sparsity.py @@ -23,7 +23,7 @@ VStack, ) from pylops.optimization.callback import CostToInitialCallback -from pylops.optimization.cls_sparsity import IRLS +from pylops.optimization.cls_sparsity import IRLS, SplitBregman from pylops.optimization.sparsity import fista, irls, ista, omp, spgl1, splitbregman # currently test spgl1 only if numpy<2.0.0 is installed... @@ -759,3 +759,135 @@ def test_SplitBregman_lambdaRL1s(par): Iop, y, [Dop], lambdaRL1s=[epsRL1], **kwargs_sb, **kwars_solver )[0] assert_array_almost_equal(xdef, xeps, decimal=12) + + +@pytest.mark.parametrize("solver_api", ["function", "class"]) +@pytest.mark.parametrize("preallocate", [False, True]) +@pytest.mark.parametrize("tight_inner", [False, True]) +def test_SplitBregman_inner_tolerance(solver_api, preallocate, tight_inner): + """Allow CGLS accuracy to be configured independently of the outer loop.""" + npp.random.seed(42) + nx = 33 + x = npp.zeros(nx) + x[: nx // 2] = 10 + x[nx // 2 : 3 * nx // 4] = -5 + y = x + npp.random.normal(0, 1, nx) + Iop = Identity(nx) + Dop = FirstDerivative(nx, edge=True) + options = dict( + niter_outer=2000, + niter_inner=5, + mu=1.0, + epsRL1s=[1.0], + tol=1e-10, + preallocate=preallocate, + ) + ref, _, _ = splitbregman( + Iop, y, [Dop], engine="scipy", iter_lim=20, damp=0, **options + ) + inner_options = dict(tol=1e-14, rtol=0.0, rtol1=0.0) + original = inner_options.copy() + new_options = dict(kwargs_solver=inner_options) if tight_inner else {} + if solver_api == "function": + result, _, _ = splitbregman( + Iop, + y, + [Dop], + engine="pylops", + niter=20, + damp=0, + **options, + **new_options, + ) + else: + result, _, _ = SplitBregman(Iop).solve( + y, + [Dop], + engine="pylops", + niter=20, + damp=0, + **options, + **new_options, + ) + error = float(npp.max(npp.abs(result - ref))) + if tight_inner: + assert error < 1e-6 + else: + assert error > 1e-3 + assert inner_options == original + + +@pytest.mark.parametrize("solver_api", ["function", "class"]) +@pytest.mark.parametrize("engine", ["scipy", "pylops"]) +def test_SplitBregman_inner_options_precedence(solver_api, engine): + """Explicit inner options override legacy keywords without mutating either.""" + nx = 15 + Iop = Identity(nx) + Dop = FirstDerivative(nx, edge=True) + y = npp.linspace(-1, 1, nx) + count_key = "iter_lim" if engine == "scipy" else "niter" + inner_options = {count_key: 20, "damp": 0.0} + if engine == "pylops": + inner_options.update(tol=1e-14, rtol=0.0, rtol1=0.0) + original = inner_options.copy() + outer_options = dict(niter_outer=8, niter_inner=2, epsRL1s=[0.2], tol=0.0) + legacy_options = {count_key: 0, "damp": 0.5} + if solver_api == "function": + ref = splitbregman( + Iop, y, [Dop], engine=engine, kwargs_solver=inner_options, **outer_options + ) + result = splitbregman( + Iop, + y, + [Dop], + engine=engine, + kwargs_solver=inner_options, + **outer_options, + **legacy_options, + ) + else: + ref = SplitBregman(Iop).solve( + y, [Dop], engine=engine, kwargs_solver=inner_options, **outer_options + ) + result = SplitBregman(Iop).solve( + y, + [Dop], + engine=engine, + kwargs_solver=inner_options, + **outer_options, + **legacy_options, + ) + npp.testing.assert_array_almost_equal(result[0], ref[0], decimal=12) + npp.testing.assert_array_almost_equal(result[2], ref[2], decimal=12) + assert result[1] == ref[1] == outer_options["niter_outer"] + assert inner_options == original + assert legacy_options == {count_key: 0, "damp": 0.5} + + +@pytest.mark.parametrize("outer_criterion", ["rtol", "rtol1"]) +def test_SplitBregman_inner_relative_tolerance(outer_criterion): + """Keep outer cost callbacks independent of inner CGLS stopping criteria.""" + Iop = Identity(15) + Dop = FirstDerivative(15, edge=True) + y = npp.linspace(-1.0, 1.0, 15) + costs = [] + inner_options = dict(niter=20, tol=1e-14, rtol=1e-12, rtol1=1e-12) + original = inner_options.copy() + _, iterations, cost = splitbregman( + Iop, + y, + [Dop], + niter_outer=100, + niter_inner=2, + epsRL1s=[0.2], + tol=0.0, + engine="pylops", + kwargs_solver=inner_options, + callback=lambda x: costs.append(x.copy()), + **{outer_criterion: 0.99}, + ) + assert 1 <= iterations < 100 + assert len(costs) == len(cost) == iterations + denominator = cost[0] if outer_criterion == "rtol" else npp.linalg.norm(y) + assert cost[-1] / denominator < 0.99 + assert inner_options == original From b2143b8ed06306ed1cc5b872b951483c899d9fea Mon Sep 17 00:00:00 2001 From: AHMETHAKANBEZIR1 Date: Sun, 18 Oct 2026 01:16:42 +0300 Subject: [PATCH 2/6] Refactor Split Bregman options and focused tests --- examples/plot_tvreg.py | 6 +- pylops/optimization/cls_sparsity.py | 19 ++-- pylops/optimization/sparsity.py | 17 ++-- pytests/test_sparsity.py | 141 ++++++++++------------------ 4 files changed, 72 insertions(+), 111 deletions(-) diff --git a/examples/plot_tvreg.py b/examples/plot_tvreg.py index a3baa81e..3fc5e406 100755 --- a/examples/plot_tvreg.py +++ b/examples/plot_tvreg.py @@ -76,10 +76,8 @@ ############################################################################### # Now we impose blockiness in the solution using the Split Bregman solver -# Inner solver options can also be supplied through ``kwargs_solver``. For -# example, use ``engine="pylops", kwargs_solver={"niter": 30, "tol": 1e-14}`` -# to tighten CGLS independently of the outer ``tol`` below. With the default -# SciPy engine, use LSQR options such as ``iter_lim``, ``atol``, and ``btol``. +# Note that inner solver options can be supplied through ``kwargs_solver``. +# Here, ``iter_lim`` and ``damp`` configure the SciPy LSQR solver. Dop = pylops.FirstDerivative(nx, edge=True, kind="backward") mu = 0.01 lamda = 0.3 diff --git a/pylops/optimization/cls_sparsity.py b/pylops/optimization/cls_sparsity.py index 5e1550ef..23aef066 100644 --- a/pylops/optimization/cls_sparsity.py +++ b/pylops/optimization/cls_sparsity.py @@ -3304,7 +3304,7 @@ def solve( show_inner: bool = False, *, kwargs_solver: dict | None = None, - **kwargs_lsqr, + **kwargs, ) -> tuple[NDArray, int, NDArray]: r"""Run entire solver @@ -3380,13 +3380,14 @@ def solve( kwargs_solver : :obj:`dict`, optional .. versionadded:: 2.9.0 - Keyword arguments for the inner solver, independent of the outer - stopping criteria. For example, ``kwargs_solver={"tol": 1e-14}`` - sets the inner CGLS tolerance without changing the outer ``tol``. - Entries override duplicate values in ``**kwargs_lsqr``. The dictionary + Keyword arguments for the inner solver (see ``engine``). + Entries override duplicate values in ``**kwargs``. The dictionary is not modified. - **kwargs_lsqr - Arbitrary keyword arguments for the solver of the + **kwargs + *Deprecated*, will be removed in v3.0.0. Simply kept for + back-compatibility with previous implementation. + + Another way to pass keyword arguments for the solver of the :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm (:py:func:`scipy.sparse.linalg.lsqr` or :py:func:`pylops.optimization.solver.cgls`, see ``engine``). @@ -3419,14 +3420,14 @@ def solve( preallocate=preallocate, show=show, ) - kwargs_lsqr = {**kwargs_lsqr, **(kwargs_solver or {})} + kwargs_solver = {**kwargs, **(kwargs_solver or {})} x = self.run( x, engine=engine, show=show, itershow=itershow, show_inner=show_inner, - **kwargs_lsqr, + **kwargs_solver, ) self.finalize(show) return x, self.iiter, self.cost diff --git a/pylops/optimization/sparsity.py b/pylops/optimization/sparsity.py index 8cfde6c3..9bb32923 100644 --- a/pylops/optimization/sparsity.py +++ b/pylops/optimization/sparsity.py @@ -729,7 +729,7 @@ def splitbregman( preallocate: bool = False, *, kwargs_solver: dict | None = None, - **kwargs_lsqr, + **kwargs, ) -> tuple[NDArray, int, NDArray]: r"""Split Bregman for mixed L2-L1 norms. @@ -819,13 +819,14 @@ def splitbregman( kwargs_solver : :obj:`dict`, optional .. versionadded:: 2.9.0 - Keyword arguments for the inner solver, independent of the outer - stopping criteria. For example, ``kwargs_solver={"tol": 1e-14}`` - sets the inner CGLS tolerance without changing the outer ``tol``. - Entries override duplicate values in ``**kwargs_lsqr``. The dictionary + Keyword arguments for the inner solver (see ``engine``). + Entries override duplicate values in ``**kwargs``. The dictionary is not modified. - **kwargs_lsqr - Arbitrary keyword arguments for the solver of the + **kwargs + *Deprecated*, will be removed in v3.0.0. Simply kept for + back-compatibility with previous implementation. + + Another way to pass keyword arguments for the solver of the :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm (:py:func:`scipy.sparse.linalg.lsqr` or :py:func:`pylops.optimization.solver.cgls`, see ``engine``). @@ -878,6 +879,6 @@ def splitbregman( itershow=itershow, show_inner=show_inner, kwargs_solver=kwargs_solver, - **kwargs_lsqr, + **kwargs, ) return xinv, itn_out, cost diff --git a/pytests/test_sparsity.py b/pytests/test_sparsity.py index b09fea5c..3fef584b 100644 --- a/pytests/test_sparsity.py +++ b/pytests/test_sparsity.py @@ -23,7 +23,7 @@ VStack, ) from pylops.optimization.callback import CostToInitialCallback -from pylops.optimization.cls_sparsity import IRLS, SplitBregman +from pylops.optimization.cls_sparsity import IRLS from pylops.optimization.sparsity import fista, irls, ista, omp, spgl1, splitbregman # currently test spgl1 only if numpy<2.0.0 is installed... @@ -761,118 +761,79 @@ def test_SplitBregman_lambdaRL1s(par): assert_array_almost_equal(xdef, xeps, decimal=12) -@pytest.mark.parametrize("solver_api", ["function", "class"]) @pytest.mark.parametrize("preallocate", [False, True]) @pytest.mark.parametrize("tight_inner", [False, True]) -def test_SplitBregman_inner_tolerance(solver_api, preallocate, tight_inner): - """Allow CGLS accuracy to be configured independently of the outer loop.""" - npp.random.seed(42) +def test_SplitBregman_inner_tolerance(preallocate, tight_inner): + """Pass CGLS stopping criteria through kwargs_solver.""" + np.random.seed(42) nx = 33 - x = npp.zeros(nx) - x[: nx // 2] = 10 - x[nx // 2 : 3 * nx // 4] = -5 - y = x + npp.random.normal(0, 1, nx) + y = np.random.normal(size=nx) Iop = Identity(nx) Dop = FirstDerivative(nx, edge=True) - options = dict( - niter_outer=2000, - niter_inner=5, + derivative = Dop.todense(backend=backend) + expected = np.linalg.solve(np.eye(nx) + derivative.T @ derivative, y) + inner_options = dict(tol=1e-14, rtol=0.0, rtol1=0.0) if tight_inner else None + result, _, _ = splitbregman( + Iop, + y, + [Dop], + niter_outer=1, + niter_inner=1, mu=1.0, epsRL1s=[1.0], tol=1e-10, preallocate=preallocate, + engine="pylops", + niter=100, + damp=0, + kwargs_solver=inner_options, ) - ref, _, _ = splitbregman( - Iop, y, [Dop], engine="scipy", iter_lim=20, damp=0, **options - ) - inner_options = dict(tol=1e-14, rtol=0.0, rtol1=0.0) - original = inner_options.copy() - new_options = dict(kwargs_solver=inner_options) if tight_inner else {} - if solver_api == "function": - result, _, _ = splitbregman( - Iop, - y, - [Dop], - engine="pylops", - niter=20, - damp=0, - **options, - **new_options, - ) - else: - result, _, _ = SplitBregman(Iop).solve( - y, - [Dop], - engine="pylops", - niter=20, - damp=0, - **options, - **new_options, - ) - error = float(npp.max(npp.abs(result - ref))) if tight_inner: - assert error < 1e-6 + assert_array_almost_equal(result, expected, decimal=6) else: - assert error > 1e-3 - assert inner_options == original + assert float(np.max(np.abs(result - expected))) > 1e-4 -@pytest.mark.parametrize("solver_api", ["function", "class"]) -@pytest.mark.parametrize("engine", ["scipy", "pylops"]) -def test_SplitBregman_inner_options_precedence(solver_api, engine): - """Explicit inner options override legacy keywords without mutating either.""" +@pytest.mark.parametrize( + "engine", ["scipy", "pylops"] if backend == "numpy" else ["pylops"] +) +def test_SplitBregman_inner_options_precedence(engine): + """Explicit inner options override conflicting legacy solver keywords.""" nx = 15 Iop = Identity(nx) Dop = FirstDerivative(nx, edge=True) - y = npp.linspace(-1, 1, nx) + y = np.linspace(-1, 1, nx) + derivative = Dop.todense(backend=backend) + expected = np.linalg.solve(np.eye(nx) + derivative.T @ derivative, y) count_key = "iter_lim" if engine == "scipy" else "niter" - inner_options = {count_key: 20, "damp": 0.0} + inner_options = {count_key: 100, "damp": 0.0} if engine == "pylops": inner_options.update(tol=1e-14, rtol=0.0, rtol1=0.0) original = inner_options.copy() - outer_options = dict(niter_outer=8, niter_inner=2, epsRL1s=[0.2], tol=0.0) - legacy_options = {count_key: 0, "damp": 0.5} - if solver_api == "function": - ref = splitbregman( - Iop, y, [Dop], engine=engine, kwargs_solver=inner_options, **outer_options - ) - result = splitbregman( - Iop, - y, - [Dop], - engine=engine, - kwargs_solver=inner_options, - **outer_options, - **legacy_options, - ) - else: - ref = SplitBregman(Iop).solve( - y, [Dop], engine=engine, kwargs_solver=inner_options, **outer_options - ) - result = SplitBregman(Iop).solve( - y, - [Dop], - engine=engine, - kwargs_solver=inner_options, - **outer_options, - **legacy_options, - ) - npp.testing.assert_array_almost_equal(result[0], ref[0], decimal=12) - npp.testing.assert_array_almost_equal(result[2], ref[2], decimal=12) - assert result[1] == ref[1] == outer_options["niter_outer"] + result, _, _ = splitbregman( + Iop, + y, + [Dop], + niter_outer=1, + niter_inner=1, + epsRL1s=[1.0], + tol=0.0, + engine=engine, + kwargs_solver=inner_options, + **{count_key: 0, "damp": 0.5}, + ) + assert_array_almost_equal(result, expected, decimal=6) assert inner_options == original - assert legacy_options == {count_key: 0, "damp": 0.5} -@pytest.mark.parametrize("outer_criterion", ["rtol", "rtol1"]) -def test_SplitBregman_inner_relative_tolerance(outer_criterion): - """Keep outer cost callbacks independent of inner CGLS stopping criteria.""" +@pytest.mark.parametrize("rtol,rtol1", [(0.99, 0.0), (0.0, 0.99)]) +def test_SplitBregman_inner_relative_tolerance(rtol, rtol1): + """Outer cost callbacks retain their own relative stopping thresholds.""" Iop = Identity(15) Dop = FirstDerivative(15, edge=True) - y = npp.linspace(-1.0, 1.0, 15) + y = np.linspace(-1.0, 1.0, 15) costs = [] - inner_options = dict(niter=20, tol=1e-14, rtol=1e-12, rtol1=1e-12) - original = inner_options.copy() + # rtol/rtol1 here configure outer callbacks; kwargs_solver configures CGLS. _, iterations, cost = splitbregman( Iop, y, @@ -882,12 +843,12 @@ def test_SplitBregman_inner_relative_tolerance(outer_criterion): epsRL1s=[0.2], tol=0.0, engine="pylops", - kwargs_solver=inner_options, + kwargs_solver=dict(niter=20, tol=1e-14, rtol=1e-12, rtol1=1e-12), callback=lambda x: costs.append(x.copy()), - **{outer_criterion: 0.99}, + rtol=rtol, + rtol1=rtol1, ) assert 1 <= iterations < 100 assert len(costs) == len(cost) == iterations - denominator = cost[0] if outer_criterion == "rtol" else npp.linalg.norm(y) + denominator = cost[0] if rtol else np.linalg.norm(y) assert cost[-1] / denominator < 0.99 - assert inner_options == original From fdff9c9a9e85f0f7f14091b95b7c86d0fce661cb Mon Sep 17 00:00:00 2001 From: mrava87 Date: Sun, 18 Oct 2026 21:04:53 +0100 Subject: [PATCH 3/6] feat: fix docstrings --- pylops/optimization/cls_sparsity.py | 13 ++++++------- pylops/optimization/sparsity.py | 14 ++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/pylops/optimization/cls_sparsity.py b/pylops/optimization/cls_sparsity.py index 23aef066..ea70f8fc 100644 --- a/pylops/optimization/cls_sparsity.py +++ b/pylops/optimization/cls_sparsity.py @@ -3380,17 +3380,16 @@ def solve( kwargs_solver : :obj:`dict`, optional .. versionadded:: 2.9.0 - Keyword arguments for the inner solver (see ``engine``). - Entries override duplicate values in ``**kwargs``. The dictionary - is not modified. + Keyword arguments for the solver of the + :math:`\mathbf{x}`-subproblem (:py:func:`scipy.sparse.linalg.lsqr` or + :py:func:`pylops.optimization.solver.cgls`, see ``engine``). + Entries override duplicate values in ``**kwargs``. **kwargs *Deprecated*, will be removed in v3.0.0. Simply kept for back-compatibility with previous implementation. - Another way to pass keyword arguments for the solver of the - :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm - (:py:func:`scipy.sparse.linalg.lsqr` or - :py:func:`pylops.optimization.solver.cgls`, see ``engine``). + Another way to pass keyword arguments to the solver of the + :math:`\mathbf{x}`-subproblem. Returns ------- diff --git a/pylops/optimization/sparsity.py b/pylops/optimization/sparsity.py index 9bb32923..611e102b 100644 --- a/pylops/optimization/sparsity.py +++ b/pylops/optimization/sparsity.py @@ -727,7 +727,6 @@ def splitbregman( show_inner: bool = False, callback: Callable | None = None, preallocate: bool = False, - *, kwargs_solver: dict | None = None, **kwargs, ) -> tuple[NDArray, int, NDArray]: @@ -819,17 +818,16 @@ def splitbregman( kwargs_solver : :obj:`dict`, optional .. versionadded:: 2.9.0 - Keyword arguments for the inner solver (see ``engine``). - Entries override duplicate values in ``**kwargs``. The dictionary - is not modified. + Keyword arguments for the solver of the + :math:`\mathbf{x}`-subproblem (:py:func:`scipy.sparse.linalg.lsqr` or + :py:func:`pylops.optimization.solver.cgls`, see ``engine``). + Entries override duplicate values in ``**kwargs``. **kwargs *Deprecated*, will be removed in v3.0.0. Simply kept for back-compatibility with previous implementation. - Another way to pass keyword arguments for the solver of the - :math:`\mathbf{x}`-subproblem of the Split Bregman algorithm - (:py:func:`scipy.sparse.linalg.lsqr` or - :py:func:`pylops.optimization.solver.cgls`, see ``engine``). + Another way to pass keyword arguments to the solver of the + :math:`\mathbf{x}`-subproblem. Returns ------- From 9d24519d4cfacf28068aa977b81767825077761e Mon Sep 17 00:00:00 2001 From: mrava87 Date: Sun, 18 Oct 2026 21:05:32 +0100 Subject: [PATCH 4/6] feat: fix tests and comment in tvreg example --- examples/plot_tvreg.py | 6 +-- pytests/test_sparsity.py | 89 +++++++++++----------------------------- 2 files changed, 28 insertions(+), 67 deletions(-) diff --git a/examples/plot_tvreg.py b/examples/plot_tvreg.py index 3fc5e406..352f9cac 100755 --- a/examples/plot_tvreg.py +++ b/examples/plot_tvreg.py @@ -75,9 +75,9 @@ plt.tight_layout() ############################################################################### -# Now we impose blockiness in the solution using the Split Bregman solver -# Note that inner solver options can be supplied through ``kwargs_solver``. -# Here, ``iter_lim`` and ``damp`` configure the SciPy LSQR solver. +# Now we impose blockiness in the solution using the Split Bregman solver. +# Note that ``kwargs_solver`` can be used to supply parameters to the inner +# solver. Dop = pylops.FirstDerivative(nx, edge=True, kind="backward") mu = 0.01 lamda = 0.3 diff --git a/pytests/test_sparsity.py b/pytests/test_sparsity.py index 3fef584b..598bc32f 100644 --- a/pytests/test_sparsity.py +++ b/pytests/test_sparsity.py @@ -761,18 +761,20 @@ def test_SplitBregman_lambdaRL1s(par): assert_array_almost_equal(xdef, xeps, decimal=12) -@pytest.mark.parametrize("preallocate", [False, True]) -@pytest.mark.parametrize("tight_inner", [False, True]) -def test_SplitBregman_inner_tolerance(preallocate, tight_inner): - """Pass CGLS stopping criteria through kwargs_solver.""" +def test_SplitBregman_kwargs_solver_cgls(): + """Check that CGLS parameters can be passed through kwargs_solver""" np.random.seed(42) - nx = 33 - y = np.random.normal(size=nx) + nx = 10 Iop = Identity(nx) Dop = FirstDerivative(nx, edge=True) + y = np.random.normal(size=nx) + + # Compute expected solution with dense solver derivative = Dop.todense(backend=backend) expected = np.linalg.solve(np.eye(nx) + derivative.T @ derivative, y) - inner_options = dict(tol=1e-14, rtol=0.0, rtol1=0.0) if tight_inner else None + + # Computed solution with SplitBregman using CGLS with tight inner tolerance + # to ensure full convergence to the expected solution. result, _, _ = splitbregman( Iop, y, @@ -782,73 +784,32 @@ def test_SplitBregman_inner_tolerance(preallocate, tight_inner): mu=1.0, epsRL1s=[1.0], tol=1e-10, - preallocate=preallocate, engine="pylops", - niter=100, damp=0, - kwargs_solver=inner_options, + kwargs_solver=dict(niter=100, tol=1e-14, rtol=0.0, rtol1=0.0), ) - if tight_inner: - assert_array_almost_equal(result, expected, decimal=6) - else: - assert float(np.max(np.abs(result - expected))) > 1e-4 + assert_array_almost_equal(result, expected, decimal=6) -@pytest.mark.parametrize( - "engine", ["scipy", "pylops"] if backend == "numpy" else ["pylops"] -) -def test_SplitBregman_inner_options_precedence(engine): - """Explicit inner options override conflicting legacy solver keywords.""" - nx = 15 +@pytest.mark.parametrize("rtol,rtol1", [(0.99, 0.0), (0.0, 0.99)]) +def test_SplitBregman_inner_relative_tolerance(rtol, rtol1): + """Check that rtol/rtol1 do not leak into the inner CGLS solver.""" + np.random.seed(42) + nx = 10 Iop = Identity(nx) Dop = FirstDerivative(nx, edge=True) - y = np.linspace(-1, 1, nx) - derivative = Dop.todense(backend=backend) - expected = np.linalg.solve(np.eye(nx) + derivative.T @ derivative, y) - count_key = "iter_lim" if engine == "scipy" else "niter" - inner_options = {count_key: 100, "damp": 0.0} - if engine == "pylops": - inner_options.update(tol=1e-14, rtol=0.0, rtol1=0.0) - original = inner_options.copy() - result, _, _ = splitbregman( - Iop, - y, - [Dop], + y = np.linspace(-1.0, 1.0, nx) + + # CGLS rtol/rtol1 are left at their defaults: if the outer ones leaked, + # CGLS would stop early and the solutions below would differ + kwargs = dict( niter_outer=1, niter_inner=1, - epsRL1s=[1.0], - tol=0.0, - engine=engine, - kwargs_solver=inner_options, - **{count_key: 0, "damp": 0.5}, - ) - assert_array_almost_equal(result, expected, decimal=6) - assert inner_options == original - - -@pytest.mark.parametrize("rtol,rtol1", [(0.99, 0.0), (0.0, 0.99)]) -def test_SplitBregman_inner_relative_tolerance(rtol, rtol1): - """Outer cost callbacks retain their own relative stopping thresholds.""" - Iop = Identity(15) - Dop = FirstDerivative(15, edge=True) - y = np.linspace(-1.0, 1.0, 15) - costs = [] - # rtol/rtol1 here configure outer callbacks; kwargs_solver configures CGLS. - _, iterations, cost = splitbregman( - Iop, - y, - [Dop], - niter_outer=100, - niter_inner=2, epsRL1s=[0.2], tol=0.0, engine="pylops", - kwargs_solver=dict(niter=20, tol=1e-14, rtol=1e-12, rtol1=1e-12), - callback=lambda x: costs.append(x.copy()), - rtol=rtol, - rtol1=rtol1, + kwargs_solver=dict(niter=20, tol=1e-14), ) - assert 1 <= iterations < 100 - assert len(costs) == len(cost) == iterations - denominator = cost[0] if rtol else np.linalg.norm(y) - assert cost[-1] / denominator < 0.99 + xref, _, _ = splitbregman(Iop, y, [Dop], **kwargs) + x, _, _ = splitbregman(Iop, y, [Dop], rtol=rtol, rtol1=rtol1, **kwargs) + assert_array_almost_equal(x, xref, decimal=10) From b4549b040bdd83ab6f4d80ef34a4b2f55a191707 Mon Sep 17 00:00:00 2001 From: mrava87 Date: Sun, 18 Oct 2026 21:07:43 +0100 Subject: [PATCH 5/6] minor: remove * in SplitBregman.solve --- pylops/optimization/cls_sparsity.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pylops/optimization/cls_sparsity.py b/pylops/optimization/cls_sparsity.py index ea70f8fc..f0b36b12 100644 --- a/pylops/optimization/cls_sparsity.py +++ b/pylops/optimization/cls_sparsity.py @@ -3302,7 +3302,6 @@ def solve( show: bool = False, itershow: tuple[int, int, int] = (10, 10, 10), show_inner: bool = False, - *, kwargs_solver: dict | None = None, **kwargs, ) -> tuple[NDArray, int, NDArray]: From b73fb81d768a8db4e034fb996211eb51fefcd9f5 Mon Sep 17 00:00:00 2001 From: mrava87 Date: Sun, 18 Oct 2026 21:22:12 +0100 Subject: [PATCH 6/6] minor: added seed to test_swts --- pytests/test_swts.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pytests/test_swts.py b/pytests/test_swts.py index c8addcd8..93196db4 100644 --- a/pytests/test_swts.py +++ b/pytests/test_swts.py @@ -78,6 +78,7 @@ def test_wrong_level(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWT_1dsignal(par): """Dot-test and inversion for SWT operator for 1d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype SWTop = SWT(dims=[par["nt"]], axis=0, wavelet="haar", level=3, dtype=par["dtype"]) @@ -109,6 +110,7 @@ def test_SWT_1dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWT_2dsignal(par): """Dot-test and inversion for SWT operator for 2d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype for axis in [0, 1]: @@ -151,7 +153,9 @@ def test_SWT_2dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWT_3dsignal(par): """Dot-test and inversion for SWT operator for 3d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype + for axis in [0, 1, 2]: SWTop = SWT( dims=(par["nt"], par["nx"], par["ny"]), @@ -194,6 +198,7 @@ def test_SWT_3dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWT2D_2dsignal(par): """Dot-test and inversion for SWT2D operator for 2d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype SWTop = SWT2D( @@ -231,6 +236,7 @@ def test_SWT2D_2dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWT2D_3dsignal(par): """Dot-test and inversion for SWT operator for 3d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype for axes in [(0, 1), (0, 2), (1, 2)]: @@ -275,6 +281,7 @@ def test_SWT2D_3dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWTND_3dsignal(par): """Dot-test and inversion for SWTND operator for 3d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype SWTop = SWTND( @@ -314,6 +321,7 @@ def test_SWTND_3dsignal(par): @pytest.mark.parametrize("par", [(par1), (par2), (par3), (par4)]) def test_SWTND_4dsignal(par): """Dot-test and inversion for SWTND operator for 4d signal""" + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype for axes in [(0, 1, 2), (0, 2, 3), (1, 2, 3), (0, 1, 3), (0, 1, 2, 3)]: @@ -359,6 +367,8 @@ def test_SWTND_4dsignal(par): @pytest.mark.parametrize("wavelet", ["db3", "bior2.2", "rbio3.3"]) def test_SWTs_wavelets(par, wavelet): """Dot-test for SWT, SWT2D and SWTND operators with non-haar wavelets""" + np.random.seed(42) + np.random.seed(42) dtype = np.empty(0, dtype=par["dtype"]).real.dtype dims = (par["nt"], par["nx"], par["ny"])