diff --git a/src/easyscience/fitting/minimizers/minimizer_bumps.py b/src/easyscience/fitting/minimizers/minimizer_bumps.py index 37f8873e..68098ddc 100644 --- a/src/easyscience/fitting/minimizers/minimizer_bumps.py +++ b/src/easyscience/fitting/minimizers/minimizer_bumps.py @@ -2,6 +2,8 @@ # SPDX-License-Identifier: BSD-3-Clause import copy +import functools +import inspect from typing import Callable from typing import List from typing import Optional @@ -28,6 +30,19 @@ FIT_AVAILABLE_IDS_FILTERED.remove('pt') +class _EvalCounter: + def __init__(self, fn: Callable): + self._fn = fn + self.count = 0 + self.__name__ = getattr(fn, '__name__', self.__class__.__name__) + self.__signature__ = inspect.signature(fn) + functools.update_wrapper(self, fn) + + def __call__(self, *args, **kwargs): + self.count += 1 + return self._fn(*args, **kwargs) + + class Bumps(MinimizerBase): """ This is a wrapper to Bumps: https://bumps.readthedocs.io/ @@ -54,6 +69,7 @@ def __init__( """ super().__init__(obj=obj, fit_function=fit_function, minimizer_enum=minimizer_enum) self._p_0 = {} + self._eval_counter: Optional[_EvalCounter] = None @staticmethod def all_methods() -> List[str]: @@ -148,7 +164,7 @@ def fit( try: model_results = bumps_fit(problem, **method_dict, **minimizer_kwargs, **kwargs) self._set_parameter_fit_result(model_results, stack_status, problem._parameters) - results = self._gen_fit_results(model_results) + results = self._gen_fit_results(model_results, max_evaluations=max_evaluations) except Exception as e: for key in self._cached_pars.keys(): self._cached_pars[key].value = self._cached_pars_vals[key][0] @@ -200,7 +216,8 @@ def _make_model(self, parameters: Optional[List[BumpsParameter]] = None) -> Call :return: Callable to make a bumps Curve model :rtype: Callable """ - fit_func = self._generate_fit_function() + fit_func = _EvalCounter(self._generate_fit_function()) + self._eval_counter = fit_func def _outer(obj): def _make_func(x, y, weights): @@ -249,7 +266,12 @@ def _set_parameter_fit_result( if stack_status: global_object.stack.endMacro() - def _gen_fit_results(self, fit_results, **kwargs) -> FitResults: + def _gen_fit_results( + self, + fit_results, + max_evaluations: Optional[int] = None, + **kwargs, + ) -> FitResults: """Convert fit results into the unified `FitResults` format. :param fit_result: Fit object which contains info on the fit @@ -261,7 +283,10 @@ def _gen_fit_results(self, fit_results, **kwargs) -> FitResults: for name, value in kwargs.items(): if getattr(results, name, False): setattr(results, name, value) - results.success = fit_results.success + nit = getattr(fit_results, 'nit', 0) + stopped_on_budget = max_evaluations is not None and nit >= max_evaluations - 1 + + results.success = fit_results.success and not stopped_on_budget pars = self._cached_pars item = {} for index, name in enumerate(self._cached_model.pars.keys()): @@ -275,6 +300,12 @@ def _gen_fit_results(self, fit_results, **kwargs) -> FitResults: results.y_obs = self._cached_model.y results.y_calc = self.evaluate(results.x, minimizer_parameters=results.p) results.y_err = self._cached_model.dy + results.n_evaluations = None if self._eval_counter is None else self._eval_counter.count + results.message = ( + f'Fit stopped: reached maximum evaluations ({max_evaluations})' + if stopped_on_budget + else '' + ) # results.residual = results.y_obs - results.y_calc # results.goodness_of_fit = np.sum(results.residual**2) results.minimizer_engine = self.__class__ diff --git a/src/easyscience/fitting/minimizers/minimizer_dfo.py b/src/easyscience/fitting/minimizers/minimizer_dfo.py index a480c823..74c2d8f9 100644 --- a/src/easyscience/fitting/minimizers/minimizer_dfo.py +++ b/src/easyscience/fitting/minimizers/minimizer_dfo.py @@ -122,6 +122,10 @@ def fit( model_results = self._dfo_fit(self._cached_pars, model, **kwargs) self._set_parameter_fit_result(model_results, stack_status) results = self._gen_fit_results(model_results, weights) + except FitError: + for key in self._cached_pars.keys(): + self._cached_pars[key].value = self._cached_pars_vals[key][0] + raise except Exception as e: for key in self._cached_pars.keys(): self._cached_pars[key].value = self._cached_pars_vals[key][0] @@ -208,7 +212,7 @@ def _gen_fit_results(self, fit_results, weights, **kwargs) -> FitResults: for name, value in kwargs.items(): if getattr(results, name, False): setattr(results, name, value) - results.success = not bool(fit_results.flag) + results.success = fit_results.flag == fit_results.EXIT_SUCCESS pars = {} for p_name, par in self._cached_pars.items(): @@ -220,11 +224,14 @@ def _gen_fit_results(self, fit_results, weights, **kwargs) -> FitResults: results.y_obs = self._cached_model.y results.y_calc = self.evaluate(results.x, minimizer_parameters=results.p) results.y_err = weights + results.n_evaluations = int(fit_results.nf) + results.message = str(fit_results.msg) # results.residual = results.y_obs - results.y_calc # results.goodness_of_fit = fit_results.f results.minimizer_engine = self.__class__ results.fit_args = None + results.engine_result = fit_results # results.check_sanity() return results @@ -258,10 +265,10 @@ def _dfo_fit( results = dfols.solve(model, pars_values, bounds=bounds, **kwargs) - if 'Success' not in results.msg: - raise FitError(f'Fit failed with message: {results.msg}') + if results.flag in {results.EXIT_SUCCESS, results.EXIT_MAXFUN_WARNING}: + return results - return results + raise FitError(f'Fit failed with message: {results.msg}') @staticmethod def _prepare_kwargs( diff --git a/src/easyscience/fitting/minimizers/minimizer_lmfit.py b/src/easyscience/fitting/minimizers/minimizer_lmfit.py index 4a8104b2..237ee00a 100644 --- a/src/easyscience/fitting/minimizers/minimizer_lmfit.py +++ b/src/easyscience/fitting/minimizers/minimizer_lmfit.py @@ -298,6 +298,8 @@ def _gen_fit_results(self, fit_results: ModelResult, **kwargs) -> FitResults: # results.goodness_of_fit = fit_results.chisqr results.y_calc = fit_results.best_fit results.y_err = 1 / fit_results.weights + results.n_evaluations = fit_results.nfev + results.message = fit_results.message results.minimizer_engine = self.__class__ results.fit_args = None diff --git a/src/easyscience/fitting/minimizers/utils.py b/src/easyscience/fitting/minimizers/utils.py index 76449a17..e3633365 100644 --- a/src/easyscience/fitting/minimizers/utils.py +++ b/src/easyscience/fitting/minimizers/utils.py @@ -20,6 +20,8 @@ class FitResults: 'y_obs', 'y_calc', 'y_err', + 'n_evaluations', + 'message', 'engine_result', 'total_results', ] @@ -35,6 +37,8 @@ def __init__(self): self.y_obs = np.ndarray([]) self.y_calc = np.ndarray([]) self.y_err = np.ndarray([]) + self.n_evaluations = None + self.message = '' self.engine_result = None self.total_results = None diff --git a/src/easyscience/fitting/multi_fitter.py b/src/easyscience/fitting/multi_fitter.py index 94e715c6..6f3b9938 100644 --- a/src/easyscience/fitting/multi_fitter.py +++ b/src/easyscience/fitting/multi_fitter.py @@ -127,6 +127,8 @@ def _post_compute_reshaping( current_results.minimizer_engine = fit_result_obj.minimizer_engine current_results.p = fit_result_obj.p current_results.p0 = fit_result_obj.p0 + current_results.n_evaluations = fit_result_obj.n_evaluations + current_results.message = fit_result_obj.message current_results.x = this_x current_results.y_obs = y[idx] current_results.y_calc = np.reshape( diff --git a/tests/integration/fitting/test_fitter.py b/tests/integration/fitting/test_fitter.py index c6d130fd..9b956cb1 100644 --- a/tests/integration/fitting/test_fitter.py +++ b/tests/integration/fitting/test_fitter.py @@ -207,14 +207,48 @@ def test_basic_max_evaluations(fit_engine): except AttributeError: pytest.skip(msg=f'{fit_engine} is not installed') f.max_evaluations = 3 - try: - result = f.fit(x=x, y=y, weights=weights) - # Result should not be the same as the reference - assert sp_sin.phase.value != pytest.approx(ref_sin.phase.value, rel=1e-3) - assert sp_sin.offset.value != pytest.approx(ref_sin.offset.value, rel=1e-3) - except FitError as e: - # DFO throws a different error - assert 'Objective has been called MAXFUN times' in str(e) + result = f.fit(x=x, y=y, weights=weights) + # Result should not be the same as the reference + assert sp_sin.phase.value != pytest.approx(ref_sin.phase.value, rel=1e-3) + assert sp_sin.offset.value != pytest.approx(ref_sin.offset.value, rel=1e-3) + + +@pytest.mark.fast +@pytest.mark.parametrize( + 'fit_engine', + [ + None, + AvailableMinimizers.LMFit, + AvailableMinimizers.Bumps, + AvailableMinimizers.DFO, + ], +) +def test_max_evaluations_populates_fit_result_fields(fit_engine): + """With a tight budget every engine must return success=False, n_evaluations>0, non-empty message.""" + ref_sin = AbsSin(0.2, np.pi) + sp_sin = AbsSin(0.354, 3.05) + + x = np.linspace(0, 5, 200) + weights = np.ones_like(x) + y = ref_sin(x) + + sp_sin.offset.fixed = False + sp_sin.phase.fixed = False + + f = Fitter(sp_sin, sp_sin) + if fit_engine is not None: + try: + f.switch_minimizer(fit_engine) + except AttributeError: + pytest.skip(msg=f'{fit_engine} is not installed') + f.max_evaluations = 3 + result = f.fit(x=x, y=y, weights=weights) + + assert result.success is False + assert result.n_evaluations is not None + assert result.n_evaluations > 0 + assert isinstance(result.message, str) + assert len(result.message) > 0 @pytest.mark.fast diff --git a/tests/integration/fitting/test_multi_fitter.py b/tests/integration/fitting/test_multi_fitter.py index 1cc5b395..25cc9adf 100644 --- a/tests/integration/fitting/test_multi_fitter.py +++ b/tests/integration/fitting/test_multi_fitter.py @@ -103,6 +103,46 @@ def test_multi_fit(fit_engine): assert result.residual == pytest.approx(F_real[idx](X[idx]) - F_ref[idx](X[idx]), abs=1e-2) +@pytest.mark.parametrize('fit_engine', [None, 'LMFit', 'Bumps', 'DFO']) +def test_multi_fit_propagates_n_evaluations_and_message(fit_engine): + """Verify that n_evaluations and message are copied into each per-dataset result.""" + ref_sin_1 = AbsSin(0.2, np.pi) + sp_sin_1 = AbsSin(0.354, 3.05) + ref_sin_2 = AbsSin(np.pi * 0.45, 0.45 * np.pi * 0.5) + sp_sin_2 = AbsSin(1, 0.5) + + ref_sin_2.offset.make_dependent_on( + dependency_expression='ref_sin1', dependency_map={'ref_sin1': ref_sin_1.offset} + ) + sp_sin_2.offset.make_dependent_on( + dependency_expression='sp_sin1', dependency_map={'sp_sin1': sp_sin_1.offset} + ) + + x1 = np.linspace(0, 5, 200) + y1 = ref_sin_1(x1) + x2 = np.copy(x1) + y2 = ref_sin_2(x2) + weights = np.ones_like(x1) + + sp_sin_1.offset.fixed = False + sp_sin_1.phase.fixed = False + sp_sin_2.phase.fixed = False + + f = MultiFitter([sp_sin_1, sp_sin_2], [sp_sin_1, sp_sin_2]) + if fit_engine is not None: + try: + f.switch_minimizer(fit_engine) + except AttributeError: + pytest.skip(msg=f'{fit_engine} is not installed') + + results = f.fit(x=[x1, x2], y=[y1, y2], weights=[weights, weights]) + for result in results: + assert result.n_evaluations is not None + assert isinstance(result.n_evaluations, int) + assert result.n_evaluations > 0 + assert isinstance(result.message, str) + + @pytest.mark.parametrize('fit_engine', [None, 'LMFit', 'Bumps', 'DFO']) def test_multi_fit2(fit_engine): ref_sin_1 = AbsSin(0.2, np.pi) diff --git a/tests/unit/fitting/minimizers/test_minimizer_bumps.py b/tests/unit/fitting/minimizers/test_minimizer_bumps.py index ba86b4d0..3d01fddc 100644 --- a/tests/unit/fitting/minimizers/test_minimizer_bumps.py +++ b/tests/unit/fitting/minimizers/test_minimizer_bumps.py @@ -89,7 +89,7 @@ def fake_set_parameter_fit_result(fit_result, stack_status, par_list): assert result == 'gen_fit_results' mock_bumps_fit.assert_called_once_with(mock_FitProblem_instance, method='amoeba') minimizer._make_model.assert_called_once_with(parameters=None) - minimizer._gen_fit_results.assert_called_once_with('fit') + minimizer._gen_fit_results.assert_called_once_with('fit', max_evaluations=None) mock_model_function.assert_called_once_with(1.0, 2.0, 1) mock_FitProblem.assert_called_once_with(mock_model) @@ -127,10 +127,13 @@ def test_make_model(self, minimizer: Bumps, monkeypatch) -> None: curve_for_model = model( x=np.array([1, 2]), y=np.array([10, 20]), weights=np.array([100, 200]) ) + wrapped_fit_function = mock_Curve.call_args[0][0] + wrapped_fit_function(np.array([1, 2]), pmock_parm_1=3) # Expect minimizer._generate_fit_function.assert_called_once_with() - assert mock_Curve.call_args[0][0] == mock_fit_function + assert minimizer._eval_counter is wrapped_fit_function + assert minimizer._eval_counter.count == 1 assert all(mock_Curve.call_args[0][1] == np.array([1, 2])) assert all(mock_Curve.call_args[0][2] == np.array([10, 20])) assert curve_for_model == 'curve' @@ -178,6 +181,7 @@ def test_gen_fit_results(self, minimizer: Bumps, monkeypatch): mock_fit_result = MagicMock() mock_fit_result.success = True + mock_fit_result.nit = 2 # nit >= max_evaluations - 1 → budget exhausted mock_cached_model = MagicMock() mock_cached_model.x = 'x' @@ -193,28 +197,34 @@ def test_gen_fit_results(self, minimizer: Bumps, monkeypatch): minimizer._cached_pars = {'par_1': mock_cached_par_1, 'par_2': mock_cached_par_2} minimizer._p_0 = 'p_0' + minimizer._eval_counter = MagicMock(count=7) minimizer.evaluate = MagicMock(return_value='evaluate') # Then domain_fit_results = minimizer._gen_fit_results( - mock_fit_result, **{'kwargs_set_key': 'kwargs_set_val'} + mock_fit_result, + max_evaluations=3, + **{'kwargs_set_key': 'kwargs_set_val'}, ) # Expect assert domain_fit_results == mock_domain_fit_results assert domain_fit_results.kwargs_set_key == 'kwargs_set_val' - assert domain_fit_results.success == True + assert domain_fit_results.success == False assert domain_fit_results.y_obs == 'y' assert domain_fit_results.x == 'x' assert domain_fit_results.p == {'ppar_1': 'par_value_1', 'ppar_2': 'par_value_2'} assert domain_fit_results.p0 == 'p_0' assert domain_fit_results.y_calc == 'evaluate' assert domain_fit_results.y_err == 'dy' + assert domain_fit_results.n_evaluations == 7 + assert domain_fit_results.message == 'Fit stopped: reached maximum evaluations (3)' assert ( str(domain_fit_results.minimizer_engine) == "" ) assert domain_fit_results.fit_args is None + assert domain_fit_results.engine_result == mock_fit_result minimizer.evaluate.assert_called_once_with( 'x', minimizer_parameters={'ppar_1': 'par_value_1', 'ppar_2': 'par_value_2'} ) diff --git a/tests/unit/fitting/minimizers/test_minimizer_dfo.py b/tests/unit/fitting/minimizers/test_minimizer_dfo.py index e1d5eeef..34f98c4f 100644 --- a/tests/unit/fitting/minimizers/test_minimizer_dfo.py +++ b/tests/unit/fitting/minimizers/test_minimizer_dfo.py @@ -177,7 +177,10 @@ def test_gen_fit_results(self, minimizer: DFO, monkeypatch): ) mock_fit_result = MagicMock() - mock_fit_result.flag = False + mock_fit_result.EXIT_SUCCESS = 0 + mock_fit_result.flag = 0 + mock_fit_result.nf = 12 + mock_fit_result.msg = 'Maximum function evaluations reached' mock_cached_model = MagicMock() mock_cached_model.x = 'x' @@ -214,15 +217,80 @@ def test_gen_fit_results(self, minimizer: DFO, monkeypatch): assert domain_fit_results.p0 == 'p_0' assert domain_fit_results.y_calc == 'evaluate' assert domain_fit_results.y_err == 'weights' + assert domain_fit_results.n_evaluations == 12 + assert domain_fit_results.message == 'Maximum function evaluations reached' + assert domain_fit_results.engine_result == mock_fit_result assert ( str(domain_fit_results.minimizer_engine) == "" ) - assert domain_fit_results.fit_args is None - minimizer.evaluate.assert_called_once_with( - 'x', minimizer_parameters={'ppar_1': 'par_value_1', 'ppar_2': 'par_value_2'} + + def test_gen_fit_results_maxfun_warning_sets_success_false(self, minimizer: DFO, monkeypatch): + """When DFO returns EXIT_MAXFUN_WARNING, _gen_fit_results must set success=False.""" + mock_domain_fit_results = MagicMock() + mock_FitResults = MagicMock(return_value=mock_domain_fit_results) + monkeypatch.setattr( + easyscience.fitting.minimizers.minimizer_dfo, 'FitResults', mock_FitResults + ) + + mock_fit_result = MagicMock() + mock_fit_result.EXIT_SUCCESS = 0 + mock_fit_result.EXIT_MAXFUN_WARNING = 1 + mock_fit_result.flag = 1 # MAXFUN_WARNING + mock_fit_result.nf = 50 + mock_fit_result.msg = 'Objective has been called MAXFUN times' + + mock_cached_model = MagicMock() + mock_cached_model.x = 'x' + mock_cached_model.y = 'y' + minimizer._cached_model = mock_cached_model + + mock_cached_par_1 = MagicMock() + mock_cached_par_1.value = 'v1' + minimizer._cached_pars = {'par_1': mock_cached_par_1} + minimizer._p_0 = 'p_0' + minimizer.evaluate = MagicMock(return_value='evaluate') + + domain_fit_results = minimizer._gen_fit_results(mock_fit_result, 'weights') + + assert domain_fit_results.success == False + assert domain_fit_results.n_evaluations == 50 + assert domain_fit_results.message == 'Objective has been called MAXFUN times' + + def test_dfo_fit_allows_maxfun_warning(self, minimizer: DFO, monkeypatch) -> None: + mock_result = MagicMock() + mock_result.EXIT_SUCCESS = 0 + mock_result.EXIT_MAXFUN_WARNING = 1 + mock_result.flag = 1 + + mock_solve = MagicMock(return_value=mock_result) + monkeypatch.setattr( + easyscience.fitting.minimizers.minimizer_dfo.dfols, 'solve', mock_solve ) + parameter = MagicMock(min=0.0, max=1.0, value=0.5) + + result = minimizer._dfo_fit({'par': parameter}, MagicMock()) + + assert result == mock_result + + def test_dfo_fit_raises_for_non_maxfun_failure(self, minimizer: DFO, monkeypatch) -> None: + mock_result = MagicMock() + mock_result.EXIT_SUCCESS = 0 + mock_result.EXIT_MAXFUN_WARNING = 1 + mock_result.flag = 4 + mock_result.msg = 'linear algebra error' + + mock_solve = MagicMock(return_value=mock_result) + monkeypatch.setattr( + easyscience.fitting.minimizers.minimizer_dfo.dfols, 'solve', mock_solve + ) + + parameter = MagicMock(min=0.0, max=1.0, value=0.5) + + with pytest.raises(FitError, match='linear algebra error'): + minimizer._dfo_fit({'par': parameter}, MagicMock()) + def test_dfo_fit(self, minimizer: DFO, monkeypatch): # When mock_parm_1 = MagicMock(Parameter) @@ -239,6 +307,9 @@ def test_dfo_fit(self, minimizer: DFO, monkeypatch): mock_dfols = MagicMock() mock_results = MagicMock() + mock_results.EXIT_SUCCESS = 0 + mock_results.EXIT_MAXFUN_WARNING = 1 + mock_results.flag = 0 mock_results.msg = 'Success' mock_dfols.solve = MagicMock(return_value=mock_results) @@ -272,6 +343,9 @@ def test_dfo_fit_no_scaling(self, minimizer: DFO, monkeypatch): mock_dfols = MagicMock() mock_results = MagicMock() + mock_results.EXIT_SUCCESS = 0 + mock_results.EXIT_MAXFUN_WARNING = 1 + mock_results.flag = 0 mock_results.msg = 'Success' mock_dfols.solve = MagicMock(return_value=mock_results) @@ -290,6 +364,33 @@ def test_dfo_fit_no_scaling(self, minimizer: DFO, monkeypatch): assert 'kwargs_set_key' in list(mock_dfols.solve.call_args[1].keys()) assert mock_dfols.solve.call_args[1]['kwargs_set_key'] == 'kwargs_set_val' + def test_fit_generic_exception_resets_parameters_and_raises_fit_error( + self, minimizer: DFO + ) -> None: + """When _dfo_fit raises a non-FitError exception, fit() must reset + parameter values to cached originals and re-raise as FitError.""" + from easyscience import global_object + + global_object.stack.enabled = False + + mock_model = MagicMock() + mock_model_function = MagicMock(return_value=mock_model) + minimizer._make_model = MagicMock(return_value=mock_model_function) + minimizer._dfo_fit = MagicMock(side_effect=RuntimeError('solver crashed')) + + cached_par_1 = MagicMock() + cached_par_1.value = 5.0 + cached_par_2 = MagicMock() + cached_par_2.value = 10.0 + minimizer._cached_pars = {'a': cached_par_1, 'b': cached_par_2} + minimizer._cached_pars_vals = {'a': (1.0, 0.1), 'b': (2.0, 0.2)} + + with pytest.raises(FitError): + minimizer.fit(x=np.array([1.0]), y=np.array([1.0]), weights=np.array([1.0])) + + assert cached_par_1.value == 1.0 + assert cached_par_2.value == 2.0 + def test_dfo_fit_exception(self, minimizer: DFO, monkeypatch): # When pars = {1: MagicMock(Parameter)} @@ -297,6 +398,9 @@ def test_dfo_fit_exception(self, minimizer: DFO, monkeypatch): mock_dfols = MagicMock() mock_results = MagicMock() + mock_results.EXIT_SUCCESS = 0 + mock_results.EXIT_MAXFUN_WARNING = 1 + mock_results.flag = 3 mock_results.msg = 'Failed' mock_dfols.solve = MagicMock(return_value=mock_results) diff --git a/tests/unit/fitting/minimizers/test_minimizer_lmfit.py b/tests/unit/fitting/minimizers/test_minimizer_lmfit.py index ac280873..0b2065a1 100644 --- a/tests/unit/fitting/minimizers/test_minimizer_lmfit.py +++ b/tests/unit/fitting/minimizers/test_minimizer_lmfit.py @@ -209,6 +209,25 @@ def test_fit_exception(self, minimizer: LMFit) -> None: with pytest.raises(FitError): minimizer.fit(x=1.0, y=2.0, weights=1) + def test_gen_fit_results_populates_evaluation_metadata(self, minimizer: LMFit) -> None: + fit_results = MagicMock() + fit_results.success = False + fit_results.data = 'data' + fit_results.userkws = {'x': 'x'} + fit_results.values = {'p1': 1.0} + fit_results.init_values = {'p1': 0.5} + fit_results.best_fit = 'best_fit' + fit_results.weights = 2 + fit_results.nfev = 9 + fit_results.message = 'max evaluations reached' + + result = minimizer._gen_fit_results(fit_results) + + assert result.success is False + assert result.n_evaluations == 9 + assert result.message == 'max evaluations reached' + assert result.engine_result == fit_results + def test_convert_to_pars_obj(self, minimizer: LMFit, monkeypatch) -> None: # When minimizer._object = MagicMock()