From d26f379737578488e197e18aaa0122b57db22bad Mon Sep 17 00:00:00 2001 From: Samuel Farrens Date: Wed, 27 Mar 2019 14:10:31 +0100 Subject: [PATCH 1/5] Added pycodestyle tests and updated release --- README.rst | 4 ++-- docs/source/index.rst | 4 ++-- modopt/info.py | 4 ++-- setup.cfg | 4 +++- setup.py | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/README.rst b/README.rst index b9705b23..3b21d0bc 100644 --- a/README.rst +++ b/README.rst @@ -23,9 +23,9 @@ ModOpt :Author: Samuel Farrens `(samuel.farrens@cea.fr) `_ -:Version: 1.2.0 +:Version: 1.3.0 -:Date: 21/11/2018 +:Date: 27/03/2019 :Documentation: |link-to-docs| diff --git a/docs/source/index.rst b/docs/source/index.rst index 55297abb..ee22b5d8 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -8,9 +8,9 @@ ModOpt Documentation :Author: Samuel Farrens -:Version: 1.2.0 +:Version: 1.3.0 -:Date: 21/11/2018 +:Date: 27/03/2019 ModOpt is a series of Modular Optimisation tools for solving inverse problems. diff --git a/modopt/info.py b/modopt/info.py index 045764fe..688f79d4 100644 --- a/modopt/info.py +++ b/modopt/info.py @@ -6,12 +6,12 @@ :Author: Samuel Farrens -:Version: 1.2.0 +:Version: 1.3.0 """ # Package Version -version_info = (1, 2, 0) +version_info = (1, 3, 0) __version__ = '.'.join(str(c) for c in version_info) __about__ = ('ModOpt \n\n ' diff --git a/setup.cfg b/setup.cfg index 227c0b64..041dbd8c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,4 +9,6 @@ build-dir=docs/build description-file = README.rst [tool:pytest] -addopts = --verbose --cov=modopt/ modopt/tests/ +addopts = --verbose --codestyle --cov=modopt +testpaths = modopt +codestyle_ignore = diff --git a/setup.py b/setup.py index 9a262c20..f6f952d6 100644 --- a/setup.py +++ b/setup.py @@ -24,5 +24,5 @@ description='Modular Optimisation tools for soliving inverse problems.', long_description=release_info["__about__"], setup_requires=['pytest-runner', ], - tests_require=['pytest', 'pytest-cov', ], + tests_require=['pytest', 'pytest-cov', 'pytest-codestyle'], ) From 8e0847d002bae98672d33a6d6ce34d0573adec64 Mon Sep 17 00:00:00 2001 From: Samuel Farrens Date: Wed, 27 Mar 2019 14:30:53 +0100 Subject: [PATCH 2/5] fixed style errors --- modopt/opt/algorithms.py | 123 +++++++++++++---------------- modopt/opt/gradient.py | 2 +- modopt/opt/proximity.py | 1 - modopt/tests/test_opt.py | 1 + modopt/tests/test_signal.py | 150 ++++++++++++++++++------------------ 5 files changed, 130 insertions(+), 147 deletions(-) diff --git a/modopt/opt/algorithms.py b/modopt/opt/algorithms.py index 45602c43..8fddf48d 100644 --- a/modopt/opt/algorithms.py +++ b/modopt/opt/algorithms.py @@ -4,7 +4,8 @@ This module contains class implementations of various optimisation algoritms. -:Author: Samuel Farrens , Zaccharie Ramzi +:Author: Samuel Farrens , + Zaccharie Ramzi NOTES ----- @@ -260,58 +261,43 @@ class FISTA(object): None, # no restarting ] - def __init__( - self, - restart_strategy=None, - min_beta=None, - s_greedy=None, - xi_restart=None, - a_cd=None, - p_lazy=1, - q_lazy=1, - r_lazy=4, - ): + def __init__(self, restart_strategy=None, min_beta=None, s_greedy=None, + xi_restart=None, a_cd=None, p_lazy=1, q_lazy=1, r_lazy=4): + if isinstance(a_cd, type(None)): self.mode = 'regular' self.p_lazy = p_lazy self.q_lazy = q_lazy self.r_lazy = r_lazy + elif a_cd > 2: self.mode = 'CD' self.a_cd = a_cd self._n = 0 + else: - raise ValueError( - "a_cd must either be None (for regular mode) or a number > 2", - ) + raise ValueError('a_cd must either be None (for regular mode) or ' + 'a number > 2') + if restart_strategy in self.__class__.__restarting_strategies__: - self._check_restart_params( - restart_strategy, - min_beta, - s_greedy, - xi_restart, - ) + self._check_restart_params(restart_strategy, min_beta, s_greedy, + xi_restart) self.restart_strategy = restart_strategy self.min_beta = min_beta self.s_greedy = s_greedy self.xi_restart = xi_restart + else: - raise ValueError( - "Restarting strategy must be one of %s." % - ", ".join(self.__class__.__restarting_strategies__) - ) + raise ValueError('Restarting strategy must be one of {}.'.format( + ', '.join( + self.__class__.__restarting_strategies__))) self._t_now = 1.0 self._t_prev = 1.0 self._delta_0 = None self._safeguard = False - def _check_restart_params( - self, - restart_strategy, - min_beta, - s_greedy, - xi_restart, - ): + def _check_restart_params(self, restart_strategy, min_beta, s_greedy, + xi_restart): r""" Check restarting parameters This method checks that the restarting parameters are set and satisfy @@ -346,23 +332,24 @@ def _check_restart_params( When a parameter that should be set isn't or doesn't verify the correct assumptions. """ + if restart_strategy is None: return True + if self.mode != 'regular': - raise ValueError( - "Restarting strategies can only be used with regular mode." - ) - greedy_params_check = ( - min_beta is None or s_greedy is None or s_greedy <= 1 - ) + raise ValueError('Restarting strategies can only be used with ' + 'regular mode.') + + greedy_params_check = (min_beta is None or s_greedy is None or + s_greedy <= 1) + if restart_strategy == 'greedy' and greedy_params_check: - raise ValueError( - "You need a min_beta and an s_greedy > 1 for greedy restart." - ) + raise ValueError('You need a min_beta and an s_greedy > 1 for ' + 'greedy restart.') + if xi_restart is None or xi_restart >= 1: - raise ValueError( - "You need a xi_restart < 1 for restart." - ) + raise ValueError('You need a xi_restart < 1 for restart.') + return True def is_restart(self, z_old, x_new, x_old): @@ -393,18 +380,22 @@ def is_restart(self, z_old, x_new, x_old): """ if self.restart_strategy is None: return False + criterion = np.vdot(z_old - x_new, x_new - x_old) >= 0 + if criterion: if 'adaptive' in self.restart_strategy: self.r_lazy *= self.xi_restart if self.restart_strategy in ['adaptive-ii', 'adaptive-2']: self._t_now = 1 + if self.restart_strategy == 'greedy': cur_delta = np.linalg.norm(x_new - x_old) if self._delta_0 is None: self._delta_0 = self.s_greedy * cur_delta else: self._safeguard = cur_delta >= self._delta_0 + return criterion def update_beta(self, beta): @@ -422,9 +413,11 @@ def update_beta(self, beta): ------- float: the new value for the beta parameter """ + if self._safeguard: beta *= self.xi_restart beta = max(beta, self.min_beta) + return beta def update_lambda(self, *args, **kwargs): @@ -441,12 +434,17 @@ def update_lambda(self, *args, **kwargs): Implements steps 3 and 4 from algoritm 10.7 in [B2011]_ """ + if self.restart_strategy == 'greedy': return 2 + # Steps 3 and 4 from alg.10.7. self._t_prev = self._t_now + if self.mode == 'regular': - self._t_now = (self.p_lazy + np.sqrt(self.r_lazy * self._t_prev ** 2 + self.q_lazy)) * 0.5 + self._t_now = (self.p_lazy + np.sqrt(self.r_lazy * + self._t_prev ** 2 + self.q_lazy)) * 0.5 + elif self.mode == 'CD': self._t_now = (self._n + self.a_cd - 1) / self.a_cd self._n += 1 @@ -538,7 +536,7 @@ def __init__(self, x, grad, prox, cost='auto', beta_param=1.0, else: self._check_param_update(lambda_update) self._lambda_update = lambda_update - self._is_restart = lambda *args, **kwargs:False + self._is_restart = lambda *args, **kwargs: False # Automatically run the algorithm if auto_iterate: @@ -1070,6 +1068,7 @@ def retrieve_outputs(self): metrics[obs.name] = obs.retrieve_metrics() self.metrics = metrics + class POGM(SetUp): r"""Proximal Optimised Gradient Method @@ -1103,28 +1102,13 @@ class POGM(SetUp): Option to automatically begin iterations upon initialisation (default is 'True') """ - def __init__( - self, - u, - x, - y, - z, - grad, - prox, - cost='auto', - linear=None, - beta_param=1.0, - sigma_bar=1.0, - auto_iterate=True, - metric_call_period=5, - metrics={}, - ): + def __init__(self, u, x, y, z, grad, prox, cost='auto', linear=None, + beta_param=1.0, sigma_bar=1.0, auto_iterate=True, + metric_call_period=5, metrics={}): + # Set default algorithm properties - super(POGM, self).__init__( - metric_call_period=metric_call_period, - metrics=metrics, - linear=linear, - ) + super(POGM, self).__init__(metric_call_period=metric_call_period, + metrics=metrics, linear=linear) # set the initial variable values (self._check_input_data(data) for data in (u, x, y, z)) @@ -1145,7 +1129,7 @@ def __init__( # Set the algorithm parameters (self._check_param(param) for param in (beta_param, sigma_bar)) - if not (0 <= sigma_bar <=1): + if not (0 <= sigma_bar <= 1): raise ValueError('The sigma bar parameter needs to be in [0, 1]') self._beta = beta_param self._sigma_bar = sigma_bar @@ -1169,7 +1153,7 @@ def _update(self): """ # Step 4 from alg. 3 self._grad.get_grad(self._x_old) - self._u_new = self._x_old - self._beta * self._grad.grad + self._u_new = self._x_old - self._beta * self._grad.grad # Step 5 from alg. 3 self._t_new = 0.5 * (1 + np.sqrt(1 + 4 * self._t_old**2)) @@ -1218,7 +1202,6 @@ def _update(self): self.converge = self.any_convergence_flag() or \ self._cost_func.get_cost(self._x_new) - def iterate(self, max_iter=150): r"""Iterate diff --git a/modopt/opt/gradient.py b/modopt/opt/gradient.py index cf198053..13d42d2d 100644 --- a/modopt/opt/gradient.py +++ b/modopt/opt/gradient.py @@ -168,7 +168,7 @@ def cost(self, method): self._cost = check_callable(method) def trans_op_op(self, data): - """Transpose Operation of the Operator + r"""Transpose Operation of the Operator This method calculates the action of the transpose operator on the action of the operator on the data diff --git a/modopt/opt/proximity.py b/modopt/opt/proximity.py index c9d21fac..cc16b7be 100644 --- a/modopt/opt/proximity.py +++ b/modopt/opt/proximity.py @@ -292,7 +292,6 @@ def __init__(self, linear_op, prox_op): self.op = self._op_method self.cost = self._cost_method - def _op_method(self, data, extra_factor=1.0): r"""Operator method diff --git a/modopt/tests/test_opt.py b/modopt/tests/test_opt.py index 3b16d788..8cca152a 100644 --- a/modopt/tests/test_opt.py +++ b/modopt/tests/test_opt.py @@ -187,6 +187,7 @@ def test_pogm(self): err_msg='Incorrect POGM result.', ) + class CostTestCase(TestCase): def setUp(self): diff --git a/modopt/tests/test_signal.py b/modopt/tests/test_signal.py index 23ea821b..eb45c122 100644 --- a/modopt/tests/test_signal.py +++ b/modopt/tests/test_signal.py @@ -42,62 +42,62 @@ def test_mex_hat_dir(self): class NoiseTestCase(TestCase): - def setUp(self): + def setUp(self): - self.data1 = np.arange(9).reshape(3, 3).astype(float) - self.data2 = np.array([[0., 2., 2.], [4., 5., 10.], - [11., 15., 18.]]) - self.data3 = np.array([[1.62434536, 0.38824359, 1.47182825], - [1.92703138, 4.86540763, 2.6984613], - [7.74481176, 6.2387931, 8.3190391]]) - self.data4 = np.array([[0., 0., 0.], [0., 0., 5.], [6., 7., 8.]]) - self.data5 = np.array([[0., 0., 0.], [0., 0., 0.], - [1., 2., 3.]]) + self.data1 = np.arange(9).reshape(3, 3).astype(float) + self.data2 = np.array([[0., 2., 2.], [4., 5., 10.], + [11., 15., 18.]]) + self.data3 = np.array([[1.62434536, 0.38824359, 1.47182825], + [1.92703138, 4.86540763, 2.6984613], + [7.74481176, 6.2387931, 8.3190391]]) + self.data4 = np.array([[0., 0., 0.], [0., 0., 5.], [6., 7., 8.]]) + self.data5 = np.array([[0., 0., 0.], [0., 0., 0.], + [1., 2., 3.]]) - def tearDown(self): + def tearDown(self): - self.data1 = None - self.data2 = None - self.data3 = None - self.data4 = None - self.data5 = None + self.data1 = None + self.data2 = None + self.data3 = None + self.data4 = None + self.data5 = None - def test_add_noise_poisson(self): + def test_add_noise_poisson(self): - np.random.seed(1) - npt.assert_array_equal(noise.add_noise(self.data1, - noise_type='poisson'), self.data2, - err_msg='Incorrect noise: Poisson') + np.random.seed(1) + npt.assert_array_equal(noise.add_noise(self.data1, + noise_type='poisson'), self.data2, + err_msg='Incorrect noise: Poisson') - npt.assert_raises(ValueError, noise.add_noise, self.data1, - noise_type='bla') + npt.assert_raises(ValueError, noise.add_noise, self.data1, + noise_type='bla') - npt.assert_raises(ValueError, noise.add_noise, self.data1, (1, 1)) + npt.assert_raises(ValueError, noise.add_noise, self.data1, (1, 1)) - def test_add_noise_gaussian(self): + def test_add_noise_gaussian(self): - np.random.seed(1) - npt.assert_almost_equal(noise.add_noise(self.data1), self.data3, - err_msg='Incorrect noise: Gaussian') + np.random.seed(1) + npt.assert_almost_equal(noise.add_noise(self.data1), self.data3, + err_msg='Incorrect noise: Gaussian') - np.random.seed(1) - npt.assert_almost_equal(noise.add_noise(self.data1, - sigma=(1, 1, 1)), self.data3, - err_msg='Incorrect noise: Gaussian') + np.random.seed(1) + npt.assert_almost_equal(noise.add_noise(self.data1, + sigma=(1, 1, 1)), self.data3, + err_msg='Incorrect noise: Gaussian') - def test_thresh_hard(self): + def test_thresh_hard(self): - npt.assert_array_equal(noise.thresh(self.data1, 5), self.data4, - err_msg='Incorrect threshold: hard') + npt.assert_array_equal(noise.thresh(self.data1, 5), self.data4, + err_msg='Incorrect threshold: hard') - npt.assert_raises(ValueError, noise.thresh, self.data1, 5, - threshold_type='bla') + npt.assert_raises(ValueError, noise.thresh, self.data1, 5, + threshold_type='bla') - def test_thresh_soft(self): + def test_thresh_soft(self): - npt.assert_array_equal(noise.thresh(self.data1, 5, - threshold_type='soft'), self.data5, - err_msg='Incorrect threshold: soft') + npt.assert_array_equal(noise.thresh(self.data1, 5, + threshold_type='soft'), self.data5, + err_msg='Incorrect threshold: soft') class PositivityTestCase(TestCase): @@ -213,13 +213,13 @@ def test_svd_thresh(self): def test_svd_thresh_coef(self): - npt.assert_almost_equal(svd.svd_thresh_coef(self.data1, - lambda x: x, 0), - self.data1, - err_msg='Incorrect SVD coefficient ' - 'tresholding') + npt.assert_almost_equal(svd.svd_thresh_coef(self.data1, + lambda x: x, 0), + self.data1, + err_msg='Incorrect SVD coefficient ' + 'tresholding') - npt.assert_raises(TypeError, svd.svd_thresh_coef, self.data1, 0, 0) + npt.assert_raises(TypeError, svd.svd_thresh_coef, self.data1, 0, 0) class ValidationTestCase(TestCase): @@ -247,34 +247,34 @@ class WaveletTestCase(TestCase): def setUp(self): - self.data1 = np.arange(9).reshape(3, 3).astype(float) - self.data2 = np.arange(36).reshape(4, 3, 3).astype(float) - self.data3 = np.array([[[6., 20., 26.], - [36., 84., 84.], - [90., 164., 134.]], - [[78., 155., 134.], - [225., 408., 327.], - [270., 461., 350.]], - [[150., 290., 242.], - [414., 732., 570.], - [450., 758., 566.]], - [[222., 425., 350.], - [603., 1056., 813.], - [630., 1055., 782.]]]) - - self.data4 = np.array([[6496., 9796., 6544.], - [9924., 14910., 9924.], - [6544., 9796., 6496.]]) - - self.data5 = np.array([[[0., 1., 4.], - [3., 10., 13.], - [6., 19., 22.]], - [[3., 10., 13.], - [24., 46., 40.], - [45., 82., 67.]], - [[6., 19., 22.], - [45., 82., 67.], - [84., 145., 112.]]]) + self.data1 = np.arange(9).reshape(3, 3).astype(float) + self.data2 = np.arange(36).reshape(4, 3, 3).astype(float) + self.data3 = np.array([[[6., 20., 26.], + [36., 84., 84.], + [90., 164., 134.]], + [[78., 155., 134.], + [225., 408., 327.], + [270., 461., 350.]], + [[150., 290., 242.], + [414., 732., 570.], + [450., 758., 566.]], + [[222., 425., 350.], + [603., 1056., 813.], + [630., 1055., 782.]]]) + + self.data4 = np.array([[6496., 9796., 6544.], + [9924., 14910., 9924.], + [6544., 9796., 6496.]]) + + self.data5 = np.array([[[0., 1., 4.], + [3., 10., 13.], + [6., 19., 22.]], + [[3., 10., 13.], + [24., 46., 40.], + [45., 82., 67.]], + [[6., 19., 22.], + [45., 82., 67.], + [84., 145., 112.]]]) def tearDown(self): From 2470120af7c75734372c89410ef76e6fbfa7a452 Mon Sep 17 00:00:00 2001 From: Samuel Farrens Date: Wed, 27 Mar 2019 15:14:05 +0100 Subject: [PATCH 3/5] changed to pytest-pep8 --- setup.cfg | 3 +-- setup.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/setup.cfg b/setup.cfg index 041dbd8c..5cee9793 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,6 +9,5 @@ build-dir=docs/build description-file = README.rst [tool:pytest] -addopts = --verbose --codestyle --cov=modopt +addopts = --verbose --pep8 --cov=modopt testpaths = modopt -codestyle_ignore = diff --git a/setup.py b/setup.py index f6f952d6..932056b8 100644 --- a/setup.py +++ b/setup.py @@ -24,5 +24,5 @@ description='Modular Optimisation tools for soliving inverse problems.', long_description=release_info["__about__"], setup_requires=['pytest-runner', ], - tests_require=['pytest', 'pytest-cov', 'pytest-codestyle'], + tests_require=['pytest', 'pytest-cov', 'pytest-pep8'], ) From 691838fd503313df887a9ded3cddf0fdbc6708d9 Mon Sep 17 00:00:00 2001 From: Samuel Farrens Date: Wed, 27 Mar 2019 15:37:01 +0100 Subject: [PATCH 4/5] added E402 exception --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 5cee9793..0b01e8bd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -11,3 +11,4 @@ description-file = README.rst [tool:pytest] addopts = --verbose --pep8 --cov=modopt testpaths = modopt +pep8ignore = E402 From 7793ebe24ab0320c3677c3fcd0dd2105cab67966 Mon Sep 17 00:00:00 2001 From: Samuel Farrens Date: Wed, 27 Mar 2019 15:59:03 +0100 Subject: [PATCH 5/5] added exceptions for E121 E126 and fixed PEP8 errors --- modopt/opt/algorithms.py | 6 +++--- modopt/tests/test_base.py | 2 +- modopt/tests/test_signal.py | 6 +++--- setup.cfg | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modopt/opt/algorithms.py b/modopt/opt/algorithms.py index 8fddf48d..ea8026d8 100644 --- a/modopt/opt/algorithms.py +++ b/modopt/opt/algorithms.py @@ -686,8 +686,8 @@ def __init__(self, x, grad, prox_list, cost='auto', gamma_param=1.0, self._x_old = np.copy(x) # Set the algorithm operators - (self._check_operator(operator) for operator in [grad, cost] - + prox_list) + (self._check_operator(operator) for operator in [grad, cost] + + prox_list) self._grad = grad self._prox_list = np.array(prox_list) self._linear = linear @@ -908,7 +908,7 @@ class Condat(SetUp): """ def __init__(self, x, y, grad, prox, prox_dual, linear=None, cost='auto', - reweight=None, rho=0.5, sigma=1.0, tau=1.0, rho_update=None, + reweight=None, rho=0.5, sigma=1.0, tau=1.0, rho_update=None, sigma_update=None, tau_update=None, auto_iterate=True, max_iter=150, n_rewightings=1, metric_call_period=5, metrics={}): diff --git a/modopt/tests/test_base.py b/modopt/tests/test_base.py index ed49f062..25305062 100644 --- a/modopt/tests/test_base.py +++ b/modopt/tests/test_base.py @@ -42,7 +42,7 @@ def test_rotate_stack(self): npt.assert_array_equal(np_adjust.rotate_stack(self.data2), np.array([[[8, 7, 6], [5, 4, 3], [2, 1, 0]], [[17, 16, 15], [14, 13, 12], - [11, 10, 9]]]), + [11, 10, 9]]]), err_msg='Incorrect stack rotation') def test_pad2d(self): diff --git a/modopt/tests/test_signal.py b/modopt/tests/test_signal.py index eb45c122..2fa10125 100644 --- a/modopt/tests/test_signal.py +++ b/modopt/tests/test_signal.py @@ -143,9 +143,9 @@ def setUp(self): [-0.21816724, -0.28316221], [-0.28507434, -0.17275339], [-0.35198144, -0.06234457], - [-0.41888854, 0.04806424], - [-0.48579564, 0.15847306], - [-0.55270274, 0.26888188]]), + [-0.41888854, 0.04806424], + [-0.48579564, 0.15847306], + [-0.55270274, 0.26888188]]), np.array([42.23492742, 1.10041151]), np.array([[-0.67608034, -0.73682791], [0.73682791, -0.67608034]])) diff --git a/setup.cfg b/setup.cfg index 0b01e8bd..6284f904 100644 --- a/setup.cfg +++ b/setup.cfg @@ -11,4 +11,4 @@ description-file = README.rst [tool:pytest] addopts = --verbose --pep8 --cov=modopt testpaths = modopt -pep8ignore = E402 +pep8ignore = E121 E126 E402