Remove code which has been deprecated since v24.3 or earlier (#2309)

* Remove deprecated code

* fix test failures

* handle max iterations

* Fix progress test

* Fix some GD issues

* Fix callbacks fail

* Fix issue with CGLS iterating after fully converged on exact solution

* Add to Change log

* Margaret's first pass, fixing failing test and deprecating a couple more things

* White space changes

* Added rosen with fixed stepsize test back

* Update documentation

---------

Signed-off-by: Laura Murgatroyd <60604372+lauramurgatroyd@users.noreply.github.com>
Co-authored-by: Margaret Duff <margaret.duff@stfc.ac.uk>
Co-authored-by: Gemma Fardell <47746591+gfardell@users.noreply.github.com>
Co-authored-by: gfardell <gemma.fardell@stfc.ac.uk>

Author: 3 people

CHANGELOG.md

@@ -22,15 +22,27 @@
     - Update to ASTRA-TOOLBOX version v2.4 from the `astra-toolbox` channel (#2330)
     - Update to TIGRE v3.1.3 (#2317)
     - Added support for numpy 2 (#2218)
-      - Update to CCPi-Regularisation toolkit 26.0.0
-      - Added support for python 3.13
+      - Added utililty method `dtype_like` to convert an input value to the same type as a reference array
+      - Added documentation to developer guide on scalar types with numpy>=2.
+    - Update to CCPi-Regularisation toolkit v26.0.0 (#2218)
+    - Update to Tomophantom v3.1.4 (#2218)
+    - Added support for python 3.13 (#2218)
   - Documentation:
     - Render the user showcase notebooks in the documentation (#2189)
     - Update on build instructions in README and developer guide for all OS (#2286)
   - Enhancements:
     - Add prefix argument to TIFFStackReader to load a subset of TIFF files in
     a folder (#2239)
     - Update ASTRA interface to `direct_FP3D/BP3D` removing copies for GPU `ProjectionOperator` calls (#2134)
+  - Removes the following code which had been deprecated since v24.3.0 or earlier:
+    - Removes `max_iteration` and `log_file` input parameters to `Algorithm`s. 
+    - Removes `max_iteration_stop_criterion`, `objective_to_string`, `verbose_output` and `verbose_header` methods from `Algorithm`s.
+    - Removes `print_interval` and `callback` kwargs from `Algorithm.run` (note: `callbacks` kwarg remains)
+    - Removes `tolerance` input parameter to `CGLS`.
+    - Removes `should_stop` and `flag` methods from `CGLS`.
+    - Removes `alpha`, `beta`, `rtol` and `atol` kwargs from `GD`.
+    - Removes `should_stop` and `objective_function` methods from `GD`.
+    - Removes `norms` and `prob` kwargs from `SPDHG`.
   - Testing:
     - migrate from `conda build` to `rattler-build`
 

Wrappers/Python/cil/optimisation/algorithms/APGD.py

@@ -34,9 +34,9 @@ class ScalarMomentumCoefficient(ABC):
     The call method of the ScalarMomentumCoefficient returns a scalar value. Given access to the algorithm object, the momentum coefficient can be a function of the algorithm state.
 
     The `apply_momentum_in_APGD` function,  updates the solution in the APGD algorithm as 
-    
+
     .. math:: y_{k+1}=x_{k+1}+M(x_{k+1}-x_{k}),
-    
+
     where :math:`M` is the calculated scalar momentum value. 
 
 
@@ -61,7 +61,7 @@ def __call__(self, algorithm):
 
     def apply_momentum_in_APGD(self, algorithm, out=None):
         r'''Calculates the momentum cofficient, applies a scalar momentum update in the APGD algorithm and returns the next iterate.
-        
+
         Uses the calculation, :math:`y_{k+1}=x_{k+1}+M(x_{k+1}-x_{k})`, where :math:`M` is the calculated scalar momentum value. 
 
         Parameters
@@ -98,7 +98,7 @@ class NesterovMomentum(ScalarMomentumCoefficient):
     Starting with :math:`t=1`, the Nesterov algorithm updates with each iteration:
 
     .. math:: t_{k+1}=\dfrac{1}{2}(1+\sqrt{1+4t_{k}^2})
-    
+
     The momentum coefficient is then returned as :math:`\dfrac{t_{k}-1}{t_{k}}`.
     '''
 
@@ -126,7 +126,7 @@ class APGD(Algorithm):
     where :math:`\alpha` is the :code:`step_size`.
 
     Then, :math:`y_{k+1}`, is then calculated from :math:`x_{k+1}`, based on a momentum rule. Note that :math:`y_0=x_0`. Users have flexibility to do this however they wish by passing to `momentum` a class that has an `apply_montemum_in_APGD` function which takes an intialised algorithm and returns the next iterate. 
-    
+
 
     Currently, we have implemented options for a scalar momentum coefficient (see :class:`cil.optimisation.algorithms.APGD.ScalarMomentumCoefficient` class.). In this case, the momentum term is added as follows:
 
@@ -233,9 +233,9 @@ def update(self):
         r"""Performs a single iteration of APGD. For :math:`k\geq 1`:
 
         .. math::
-        
+
                 x_{k+1} = \mathrm{prox}_{\alpha g}(y_{k} - \alpha\nabla f(y_{k}))\\
-            
+
 
         where :math:`\alpha` is the step size. From :math:`x_{k+1}` (and any other information available in the algorithm class) the momentum function then calculates :math:`y_{k+1}`. 
         """
@@ -293,14 +293,15 @@ def step_size(self):
         '''
         Returns the most recently used step size. Note, if the step-size is set by a non-constant step size rule, you must use the algorithm run or update method before this getter will return the most recently used step size. 
         '''
-        
+
         if isinstance(self.step_size_rule, ConstantStepSize):
             return self.step_size_rule.step_size
         else:
-            try: 
+            try:
                 return self._step_size
             except AttributeError:
-                raise NotImplementedError("Note the step-size is set by a step-size rule and could change with each iteration. Call the algorithm run or update method first and then this function will give the most recently used step size.")
+                raise NotImplementedError(
+                    "Note the step-size is set by a step-size rule and could change with each iteration. Call the algorithm run or update method first and then this function will give the most recently used step size.")
 
     @property
     def momentum(self):

Wrappers/Python/cil/optimisation/algorithms/Algorithm.py

@@ -22,65 +22,43 @@
 
 import numpy as np
 
-from cil.optimisation.utilities.callbacks import Callback, LogfileCallback, _OldCallback, ProgressCallback
+from cil.optimisation.utilities.callbacks import Callback, ProgressCallback
 
 
 class Algorithm:
     r"""Base class providing minimal infrastructure for iterative algorithms.
 
    An iterative algorithm is designed to solve an optimization problem by repeatedly refining a solution. In CIL, we use iterative algorithms to minimize an objective function, often referred to as a loss. The process begins with an initial guess, and with each iteration, the algorithm updates the current solution based on the results of previous iterations (previous iterates). Iterative algorithms typically continue until a stopping criterion is met, indicating that an optimal or sufficiently good solution has been found. In CIL, stopping criteria can be implemented using a callback function (`cil.optimisation.utilities.callbacks`).
-     
+
     The user is required to implement the :code:`set_up`, :code:`__init__`, :code:`update` and :code:`update_objective` methods.
 
     The method :code:`run` is available to run :code:`n` iterations. The method accepts :code:`callbacks`: a list of callables, each of which receive the current Algorithm object (which in turn contains the iteration number and the actual objective value) and can be used to trigger print to screens and other user interactions. The :code:`run` method will stop when the stopping criterion is met or `StopIteration` is raised.
-    
+
     Parameters
     ----------
     update_objective_interval: int, optional, default 1
         The objective (or loss) is calculated and saved every `update_objective_interval`.  1 means every iteration, 2 every 2 iterations and so forth. This is by default 1 and should be increased when evaluating the objective is computationally expensive.
     """
 
-    def __init__(self, update_objective_interval=1, max_iteration=None, log_file=None):
+    def __init__(self, update_objective_interval=1):
 
         self.iteration = -1
-        self.__max_iteration = 1
-        if max_iteration is not None:
-            warn("use `Algorithm.run(iterations)` instead of `Algorithm(max_iteration)`", DeprecationWarning, stacklevel=2)
-            self.__max_iteration = max_iteration
+        self._total_iterations = 1
         self.__loss = []
         self.memopt = False
         self.configured = False
         self._iteration = []
         self.update_objective_interval = update_objective_interval
-        # self.x = None
         self.iter_string = 'Iter'
-        if log_file is not None:
-            warn("use `run(callbacks=[LogfileCallback(log_file)])` instead of `log_file`",
-                 DeprecationWarning, stacklevel=2)
-            self.__log_file = log_file
 
     def set_up(self, *args, **kwargs):
         '''Set up the algorithm'''
         raise NotImplementedError
+
     def update(self):
         '''A single iteration of the algorithm'''
         raise NotImplementedError
 
-    def should_stop(self):
-        '''default stopping criterion: number of iterations
-
-        The user can change this in concrete implementation of iterative algorithms.'''
-        return self.iteration > self.max_iteration
-
-    def __set_up_logger(self, *_, **__):
-        """Do not use: this is being deprecated"""
-        warn("use `run(callbacks=[LogfileCallback(log_file)])` instead", DeprecationWarning, stacklevel=2)
-
-    def max_iteration_stop_criterion(self):
-        """Do not use: this is being deprecated"""
-        warn("use `should_stop()` instead of `max_iteration_stop_criterion()`", DeprecationWarning, stacklevel=2)
-        return self.iteration > self.max_iteration
-
     def __iter__(self):
         '''Algorithm is an iterable'''
         return self
@@ -90,22 +68,21 @@ def __next__(self):
 
         This method triggers :code:`update()` and :code:`update_objective()`
         '''
-        if self.should_stop():
-            raise StopIteration
         if self.iteration == -1 and self.update_objective_interval > 0:
             self._iteration.append(self.iteration)
             self.update_objective()
             self.iteration += 1
             return self.iteration
         if not self.configured:
-            raise ValueError('Algorithm not configured correctly. Please run set_up.')
+            raise ValueError(
+                'Algorithm not configured correctly. Please run set_up.')
         self.update()
         self.iteration += 1
 
         self._update_previous_solution()
 
         if self.iteration >= 0 and self.update_objective_interval > 0 and\
-            self.iteration % self.update_objective_interval == 0:
+                self.iteration % self.update_objective_interval == 0:
 
             self._iteration.append(self.iteration)
             self.update_objective()
@@ -127,32 +104,33 @@ def _update_previous_solution(self):
 
     def get_output(self):
         r""" Returns the current solution. 
-        
+
         Returns
         -------
         DataContainer
             The current solution 
-             
+
         """
         return self.x
 
     def _provable_convergence_condition(self):
         r""" Checks if the algorithm set-up (e.g. chosen step-sizes or other parameters) meets a mathematical convergence criterion.
-        
+
         Returns
         -------
         bool: Outcome of the convergence check  
         """
-        raise NotImplementedError(" Convergence criterion is not implemented for this algorithm. ")
+        raise NotImplementedError(
+            " Convergence criterion is not implemented for this algorithm. ")
 
     def is_provably_convergent(self):
         r""" Check if the algorithm is convergent based on the provable convergence criterion.
-        
+
         Returns
         -------
         Boolean
             Outcome of the convergence check  
-        
+
         """
         return self._provable_convergence_condition()
 
@@ -163,17 +141,17 @@ def solution(self):
 
     def get_last_loss(self, return_all=False):
         r'''Returns the last stored value of the loss function. "Loss" is an alias for "objective value". If `update_objective_interval` is 1 it is the value of the objective at the current iteration. If update_objective_interval > 1 it is the last stored value.
-        
+
         Parameters
         ----------
         return_all: Boolean, default is False
             If True, returns all the stored loss functions 
-        
+
         Returns
         -------
         Float
             Last stored value of the loss function 
-        
+
         '''
         try:
             objective = self.__loss[-1]
@@ -183,7 +161,7 @@ def get_last_loss(self, return_all=False):
             return objective if return_all else objective[0]
         return [objective, np.nan, np.nan] if return_all else objective
 
-    get_last_objective = get_last_loss # alias
+    get_last_objective = get_last_loss  # alias
 
     def update_objective(self):
         '''calculates the objective with the current solution'''
@@ -193,7 +171,7 @@ def update_objective(self):
     def iterations(self):
         '''returns the iterations at which the objective has been evaluated'''
         return self._iteration
-    
+
     @property
     def loss(self):
         '''returns a list of the values of the objective (alias of loss) during the iteration
@@ -202,18 +180,7 @@ def loss(self):
         '''
         return self.__loss
 
-    objective = loss # alias
-
-    @property
-    def max_iteration(self):
-        '''gets the maximum number of iterations'''
-        return self.__max_iteration
-
-    @max_iteration.setter
-    def max_iteration(self, value):
-        '''sets the maximum number of iterations'''
-        assert isinstance(value, Integral) or np.isposinf(value)
-        self.__max_iteration = value
+    objective = loss  # alias
 
     @property
     def update_objective_interval(self):
@@ -227,9 +194,9 @@ def update_objective_interval(self, value):
             raise ValueError('interval must be an integer >= 0')
         self.__update_objective_interval = value
 
-    def run(self, iterations=None, callbacks: Optional[List[Callback]]=None, verbose=1, **kwargs):
+    def run(self, iterations=None, callbacks: Optional[List[Callback]] = None, verbose=1):
         r"""run upto :code:`iterations` with callbacks/logging.
-        
+
         For a demonstration of callbacks see https://github.com/TomographicImaging/CIL-Demos/blob/main/misc/callback_demonstration.ipynb
 
         Parameters
@@ -241,41 +208,29 @@ def run(self, iterations=None, callbacks: Optional[List[Callback]]=None, verbose
         verbose: 0=quiet, 1=info, 2=debug
             Passed to the default callback to determine the verbosity of the printed output. 
         """
-        
+
         if iterations is None:
             raise ValueError("`run()` missing number of `iterations`")
-        
-        if 'print_interval' in kwargs:
-            warn("use `TextProgressCallback(miniters)` instead of `run(print_interval)`",
-                 DeprecationWarning, stacklevel=2)
+
         if np.isposinf(iterations):
             if callbacks is None:
-                raise ValueError("Infinite iterations require a callback with a stopping criterion that raises `StopIteration`")
+                raise ValueError(
+                    "Infinite iterations require a callback with a stopping criterion that raises `StopIteration`")
             else:
-                warn("Infinite iterations require a callback with a stopping criterion that raises `StopIteration`", UserWarning, stacklevel=2)
-        
+                warn("Infinite iterations require a callback with a stopping criterion that raises `StopIteration`",
+                     UserWarning, stacklevel=2)
+
         if callbacks is None:
             callbacks = [ProgressCallback(verbose=verbose)]
-            
-        
-        # transform old-style callbacks into new
-        callback = kwargs.get('callback', None)
-        if callback is not None:
-            callbacks.append(_OldCallback(callback, verbose=verbose))
-        if hasattr(self, '__log_file'):
-            callbacks.append(LogfileCallback(self.__log_file, verbose=verbose))
-
-        if self.should_stop():
-            print("Stop criterion has been reached.")
-        
-
-        if self.iteration == -1 and self.update_objective_interval>0:
-            iterations+=1
-
-        # call `__next__` upto `iterations` times or until `StopIteration` is raised
-        self.max_iteration = self.iteration + iterations
-        iters = (count(self.iteration) if np.isposinf(self.max_iteration)
-                 else range(self.iteration, self.max_iteration))
+
+        if self.iteration == -1 and self.update_objective_interval > 0:
+            iterations += 1
+
+        self._total_iterations = self.iteration + iterations
+
+        # call `__next__` up to `iterations` times or until `StopIteration` is raised
+        iters = (count(self.iteration) if np.isposinf(self._total_iterations)
+                 else range(self.iteration, self._total_iterations))
         for _ in iters:
             try:
                 self.__next__()
@@ -292,16 +247,3 @@ def objective_to_dict(self, verbose=False):
                 return {'primal': obj[0], 'dual': obj[1], 'primal_dual': obj[2]}
             obj = obj[0]
         return {'objective': obj}
-
-    def objective_to_string(self, verbose=False):
-        """Do not use: this is being deprecated"""
-        warn("consider using `run(callbacks=[LogfileCallback(log_file)])` instead", DeprecationWarning, stacklevel=2)
-        return str(self.objective_to_dict(verbose=verbose))
-
-    def verbose_output(self, *_, **__):
-        """Do not use: this is being deprecated"""
-        warn("use `run(callbacks=[ProgressCallback()])` instead", DeprecationWarning, stacklevel=2)
-
-    def verbose_header(self, *_, **__):
-        """Do not use: this is being deprecated"""
-        warn("consider using `run(callbacks=[LogfileCallback(log_file)])` instead", DeprecationWarning, stacklevel=2)