Updated bumps and lmfit controllers to be more robust against paramet…

…er names (#1270) * Make params unique and valid python vars in bumps controller * Add missing enumerate * updated the lmfit_controller * added tests --------- Co-authored-by: RabiyaF <47083562+RabiyaF@users.noreply.github.com>
fitbenchmarking · Apr 29, 2024 · 138dfa5 · 138dfa5
1 parent 10d5107
commit 138dfa5
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 26 deletions.
diff --git a/fitbenchmarking/controllers/bumps_controller.py b/fitbenchmarking/controllers/bumps_controller.py
@@ -2,11 +2,10 @@
 Implements a controller for the Bumps fitting software.
 """
 
+import numpy as np
 from bumps.fitters import fit as bumpsFit
 from bumps.names import Curve, FitProblem, PoissonCurve
 
-import numpy as np
-
 from fitbenchmarking.controllers.base_controller import Controller
 from fitbenchmarking.cost_func.cost_func_factory import create_cost_func
 from fitbenchmarking.utils.exceptions import MaxRuntimeError
@@ -21,24 +20,24 @@ class BumpsController(Controller):
     """
 
     algorithm_check = {
-            'all': ['amoeba',
-                    'lm-bumps',
-                    'newton',
-                    'de',
-                    'scipy-leastsq',
-                    'dream'],
-            'ls': ['lm-bumps', 'scipy-leastsq'],
-            'deriv_free': ['amoeba', 'de'],
-            'general': ['amoeba', 'newton', 'de'],
-            'simplex': ['amoeba'],
-            'trust_region': ['lm-bumps', 'scipy-leastsq'],
-            'levenberg-marquardt': ['lm-bumps', 'scipy-leastsq'],
-            'gauss_newton': [],
-            'bfgs': ['newton'],
-            'conjugate_gradient': [],
-            'steepest_descent': [],
-            'global_optimization': ['de'],
-            'MCMC': ['dream']}
+        'all': ['amoeba',
+                'lm-bumps',
+                'newton',
+                'de',
+                'scipy-leastsq',
+                'dream'],
+        'ls': ['lm-bumps', 'scipy-leastsq'],
+        'deriv_free': ['amoeba', 'de'],
+        'general': ['amoeba', 'newton', 'de'],
+        'simplex': ['amoeba'],
+        'trust_region': ['lm-bumps', 'scipy-leastsq'],
+        'levenberg-marquardt': ['lm-bumps', 'scipy-leastsq'],
+        'gauss_newton': [],
+        'bfgs': ['newton'],
+        'conjugate_gradient': [],
+        'steepest_descent': [],
+        'global_optimization': ['de'],
+        'MCMC': ['dream']}
 
     def __init__(self, cost_func):
         """
@@ -49,9 +48,9 @@ def __init__(self, cost_func):
                 :class:`~fitbenchmarking.cost_func.base_cost_func.CostFunc`
         """
         super().__init__(cost_func)
-
-        self._param_names = [name.replace('.', '_')
-                             for name in self.problem.param_names]
+        # Need unique strings that are valid python vars
+        self._param_names = [
+            f'p{i}' for (i, _) in enumerate(self.problem.param_names)]
         self.support_for_bounds = True
         self._func_wrapper = None
         self._fit_problem = None

diff --git a/fitbenchmarking/controllers/lmfit_controller.py b/fitbenchmarking/controllers/lmfit_controller.py
@@ -105,21 +105,23 @@ def __init__(self, cost_func):
         self.bound_minimizers = ['dual_annealing', 'differential_evolution']
         self.lmfit_out = None
         self.lmfit_params = Parameters()
+        self._param_names = [
+            f'p{i}' for (i, _) in enumerate(self.problem.param_names)]
 
     def lmfit_resdiuals(self, params):
         """
         lmfit resdiuals
         """
         return self.cost_func.eval_r(list(map(lambda name: params[name].value,
-                                     self.problem.param_names)))
+                                     self._param_names)))
 
     def lmfit_jacobians(self, params):
         """
         lmfit jacobians
         """
         return self.cost_func.jac_cost(list(map(lambda name:
                                        params[name].value,
-                                       self.problem.param_names)))
+                                       self._param_names)))
 
     def setup(self):
         """
@@ -132,7 +134,7 @@ def setup(self):
                     f"{self.minimizer} requires finite bounds on all"
                     " parameters")
 
-        for i, name in enumerate(self.problem.param_names):
+        for i, name in enumerate(self._param_names):
             kwargs = {"name": name,
                       "value": self.initial_params[i]}
             if self.value_ranges is not None:

diff --git a/fitbenchmarking/controllers/tests/test_controllers.py b/fitbenchmarking/controllers/tests/test_controllers.py
@@ -523,6 +523,16 @@ def test_lmfit(self):
         controller.lmfit_out.success = False
         self.shared_tests.check_diverged(controller)
 
+    def test_variable_names_corrected_in_controllers(self):
+        """
+        Test if variable names are corrected properly
+        within the LmfitController and BumpsController
+        """
+        for control in ([LmfitController, BumpsController]):
+            self.cost_func.param_names = ['b.1', 'b@2', 'b-3', 'b_4']
+            controller = control(self.cost_func)
+            assert controller._param_names == ['p0', 'p1', 'p2', 'p3']
+
 
 @run_for_test_types(TEST_TYPE, 'all')
 class ControllerBoundsTests(TestCase):