From 55e4359a7788f49f81e987607e47f2fc6c64e228 Mon Sep 17 00:00:00 2001 From: Brendt Wohlberg Date: Tue, 12 Dec 2023 09:28:45 -0700 Subject: [PATCH] Resolve #479 (#480) * Add test for FiniteDifference scaling * Remove stack-specific operations --- scico/linop/_stack.py | 20 +---------- scico/operator/_stack.py | 50 +--------------------------- scico/test/linop/test_diff.py | 10 ++++++ scico/test/linop/test_linop_stack.py | 15 --------- scico/test/operator/test_op_stack.py | 15 --------- 5 files changed, 12 insertions(+), 98 deletions(-) diff --git a/scico/linop/_stack.py b/scico/linop/_stack.py index 8d8ef68ea..db8b1b7b2 100644 --- a/scico/linop/_stack.py +++ b/scico/linop/_stack.py @@ -9,8 +9,6 @@ from __future__ import annotations -import operator -from functools import partial from typing import Optional, Sequence, Union import scico.numpy as snp @@ -18,7 +16,7 @@ from scico.operator._stack import DiagonalStack as DStack from scico.operator._stack import VerticalStack as VStack -from ._linop import LinearOperator, _wrap_add_sub +from ._linop import LinearOperator class VerticalStack(VStack, LinearOperator): @@ -70,22 +68,6 @@ def __init__( def _adj(self, y: Union[Array, BlockArray]) -> Array: # type: ignore return sum([op.adj(y_block) for y_block, op in zip(y, self.ops)]) # type: ignore - @partial(_wrap_add_sub, op=operator.add) - def __add__(self, other): - # add another VerticalStack of the same shape - return VerticalStack( - [op1 + op2 for op1, op2 in zip(self.ops, other.ops)], - collapse_output=self.collapse_output, - ) - - @partial(_wrap_add_sub, op=operator.sub) - def __sub__(self, other): - # subtract another VerticalStack of the same shape - return VerticalStack( - [op1 - op2 for op1, op2 in zip(self.ops, other.ops)], - collapse_output=self.collapse_output, - ) - class DiagonalStack(DStack, LinearOperator): r"""A diagonal stack of linear operators. diff --git a/scico/operator/_stack.py b/scico/operator/_stack.py index 000baf95c..9e16f05ea 100644 --- a/scico/operator/_stack.py +++ b/scico/operator/_stack.py @@ -20,7 +20,7 @@ from scico.numpy.util import is_nested from scico.typing import BlockShape, Shape -from ._operator import Operator, _wrap_mul_div_scalar +from ._operator import Operator def collapse_shapes( @@ -144,54 +144,6 @@ def _eval(self, x: Array) -> Union[Array, BlockArray]: return snp.stack([op(x) for op in self.ops]) return BlockArray([op(x) for op in self.ops]) - def scale_ops(self, scalars: Array): - """Scale component operators. - - Return a copy of `self` with each operator scaled by the - corresponding entry in `scalars`. - - Args: - scalars: List or array of scalars to use. - """ - if len(scalars) != len(self.ops): - raise ValueError("Expected scalars to be the same length as self.ops.") - - return self.__class__( - [a * op for a, op in zip(scalars, self.ops)], collapse_output=self.collapse_output - ) - - def __add__(self, other): - # add another VerticalStack of the same shape - return self.__class__( - [op1 + op2 for op1, op2 in zip(self.ops, other.ops)], - collapse_output=self.collapse_output, - ) - - def __sub__(self, other): - # subtract another VerticalStack of the same shape - return self.__class__( - [op1 - op2 for op1, op2 in zip(self.ops, other.ops)], - collapse_output=self.collapse_output, - ) - - @_wrap_mul_div_scalar - def __mul__(self, scalar): - return self.__class__( - [scalar * op for op in self.ops], collapse_output=self.collapse_output - ) - - @_wrap_mul_div_scalar - def __rmul__(self, scalar): - return self.__class__( - [scalar * op for op in self.ops], collapse_output=self.collapse_output - ) - - @_wrap_mul_div_scalar - def __truediv__(self, scalar): - return self.__class__( - [op / scalar for op in self.ops], collapse_output=self.collapse_output - ) - class DiagonalStack(Operator): r"""A diagonal stack of operators. diff --git a/scico/test/linop/test_diff.py b/scico/test/linop/test_diff.py index 1a13cc475..d484fdce7 100644 --- a/scico/test/linop/test_diff.py +++ b/scico/test/linop/test_diff.py @@ -24,6 +24,16 @@ def test_eval(): ) snp.testing.assert_allclose(Ax[1], snp.array([[-1, 1, -1], [0, -1, 0]])) # along rows + # test scale + B = 2.0 * A + Bx = B @ x + + snp.testing.assert_allclose( + Bx[0], # down columns x[1] - x[0], ..., append - x[N-1] + 2.0 * snp.array([[0, 1, -1], [-1, -1, 0]]), + ) + snp.testing.assert_allclose(Bx[1], 2.0 * snp.array([[-1, 1, -1], [0, -1, 0]])) # along rows + def test_except(): with pytest.raises(TypeError): # axis is not an int diff --git a/scico/test/linop/test_linop_stack.py b/scico/test/linop/test_linop_stack.py index f3604b16a..37f77d4c8 100644 --- a/scico/test/linop/test_linop_stack.py +++ b/scico/test/linop/test_linop_stack.py @@ -100,21 +100,6 @@ def test_algebra(self, collapse_output, jit): np.testing.assert_allclose((S @ x)[0], (H @ x + G @ x)[0]) np.testing.assert_allclose((S @ x)[1], (H @ x + G @ x)[1]) - # result of adding two conformable stacks should be a stack - assert isinstance(S, VerticalStack) - assert isinstance(H - G, VerticalStack) - - # scalar multiplication - assert isinstance(1.0 * H, VerticalStack) - - # op scaling - scalars = [2.0, 3.0] - y1 = S @ x - S2 = S.scale_ops(scalars) - y2 = S2 @ x - - np.testing.assert_allclose(scalars[0] * y1[0], y2[0]) - class TestBlockDiagonalLinearOperator: def test_construct(self): diff --git a/scico/test/operator/test_op_stack.py b/scico/test/operator/test_op_stack.py index 695bebe95..c981cdf26 100644 --- a/scico/test/operator/test_op_stack.py +++ b/scico/test/operator/test_op_stack.py @@ -86,21 +86,6 @@ def test_algebra(self, collapse_output, jit): np.testing.assert_allclose((S(x))[0], (H(x) + G(x))[0]) np.testing.assert_allclose((S(x))[1], (H(x) + G(x))[1]) - # result of adding two conformable stacks should be a stack - assert isinstance(S, VerticalStack) - assert isinstance(H - G, VerticalStack) - - # scalar multiplication - assert isinstance(1.0 * H, VerticalStack) - - # op scaling - scalars = [2.0, 3.0] - y1 = S(x) - S2 = S.scale_ops(scalars) - y2 = S2(x) - - np.testing.assert_allclose(scalars[0] * y1[0], y2[0]) - class TestBlockDiagonalOperator: def test_construct(self):