diff --git a/src/easyscience/fitting/minimizers/minimizer_lmfit.py b/src/easyscience/fitting/minimizers/minimizer_lmfit.py index 30664708..b3dd9cbb 100644 --- a/src/easyscience/fitting/minimizers/minimizer_lmfit.py +++ b/src/easyscience/fitting/minimizers/minimizer_lmfit.py @@ -242,10 +242,14 @@ def _get_fit_kws( ) -> dict[str:str]: if minimizer_kwargs is None: minimizer_kwargs = {} + # `method` is usually None, because `Fitter.fit` + # does not pass one; the minimizer's own method is what actually runs, + # so it decides which tolerance keyword the backend accepts. + effective_method = method if method is not None else self._method if tolerance is not None: - if method in [None, 'least_squares', 'leastsq']: + if effective_method in ['least_squares', 'leastsq']: minimizer_kwargs['ftol'] = tolerance - if method in ['differential_evolution', 'powell', 'cobyla']: + if effective_method in ['differential_evolution', 'powell', 'cobyla']: minimizer_kwargs['tol'] = tolerance return minimizer_kwargs @@ -371,7 +375,11 @@ def _set_parameter_fit_result(self, fit_result: ModelResult, stack_status: bool) if fit_result.errorbars: pars[name].error = fit_result.params[MINIMIZER_PARAMETER_PREFIX + str(name)].stderr else: - pars[name].error = 0.0 + # No covariance available (gradient-free method, aborted fit, or a + # parameter at a bound). None keeps that distinguishable from a + # genuine zero uncertainty and clears any stale error from a + # previous fit. + pars[name].error = None if stack_status: global_object.stack.endMacro() diff --git a/tests/integration/fitting/test_fitter.py b/tests/integration/fitting/test_fitter.py index 5224da73..1b4be9c4 100644 --- a/tests/integration/fitting/test_fitter.py +++ b/tests/integration/fitting/test_fitter.py @@ -78,7 +78,7 @@ def __call__(self, x: np.ndarray) -> np.ndarray: return self.slope.value * x + self.intercept.value -def check_fit_results(result, sp_sin, ref_sin, x, **kwargs): +def check_fit_results(result, sp_sin, ref_sin, x, expect_error=True, **kwargs): assert result.n_pars == len(sp_sin.get_fit_parameters()) assert result.chi2 == pytest.approx(0, abs=1.5e-3 * (len(result.x) - result.n_pars)) assert result.reduced_chi2 == pytest.approx(0, abs=1.5e-3) @@ -91,8 +91,13 @@ def check_fit_results(result, sp_sin, ref_sin, x, **kwargs): assert result.p0[key] == pytest.approx(value) # Bumps does something strange here assert np.all(result.x == x) for item1, item2 in zip(sp_sin._kwargs.values(), ref_sin._kwargs.values()): - # assert item.error > 0 % This does not work as some methods don't calculate error - assert item1.error == pytest.approx(0, abs=2.1e-1) + # Gradient-free methods (e.g. lmfit's powell/cobyla) have no covariance + # matrix, so they report error as None rather than a fake 0.0. Every + # other method must still produce a real uncertainty. + if expect_error: + assert item1.error == pytest.approx(0, abs=2.1e-1) + else: + assert item1.error is None assert item1.value == pytest.approx(item2.value, abs=5e-3) y_calc_ref = ref_sin(x) assert result.y_calc == pytest.approx(y_calc_ref, abs=1e-2) @@ -330,7 +335,9 @@ def test_lmfit_methods(fit_method): f = Fitter(sp_sin, sp_sin) assert fit_method in f._minimizer.supported_methods() result = f.fit(x, y, weights=weights, method=fit_method) - check_fit_results(result, sp_sin, ref_sin, x) + check_fit_results( + result, sp_sin, ref_sin, x, expect_error=fit_method not in ('powell', 'cobyla') + ) # @pytest.mark.xfail(reason="known bumps issue") @@ -409,7 +416,12 @@ def test_dependent_parameter(fit_engine): pytest.skip(reason=f'{fit_engine} is not installed') result = f.fit(x, y, weights=weights) - check_fit_results(result, sp_sin, ref_sin, x) + # With `offset` dependent on `phase` only one parameter varies against + # noiseless data, so lmfit's covariance is exactly [[0.]] and it reports + # `errorbars=False` -> no uncertainties. + check_fit_results( + result, sp_sin, ref_sin, x, expect_error=fit_engine is not AvailableMinimizers.LMFit + ) @pytest.mark.fast diff --git a/tests/unit/fitting/minimizers/test_minimizer_lmfit.py b/tests/unit/fitting/minimizers/test_minimizer_lmfit.py index 781893d9..0141fc56 100644 --- a/tests/unit/fitting/minimizers/test_minimizer_lmfit.py +++ b/tests/unit/fitting/minimizers/test_minimizer_lmfit.py @@ -212,6 +212,48 @@ def test_fit_kwargs(self, minimizer: LMFit) -> None: ) assert callable(mock_model.fit.call_args.kwargs['iter_cb']) + @pytest.mark.parametrize( + 'minimizer_method, passed_method, expected', + [ + ('leastsq', None, {'ftol': 0.1}), + ('least_squares', None, {'ftol': 0.1}), + ('powell', None, {'tol': 0.1}), + ('differential_evolution', None, {'tol': 0.1}), + ('cobyla', None, {'tol': 0.1}), + ('leastsq', 'powell', {'tol': 0.1}), + ('powell', 'leastsq', {'ftol': 0.1}), + ('nelder', None, {}), + ], + ids=[ + 'leastsq', + 'least_squares', + 'powell', + 'differential_evolution', + 'cobyla', + 'explicit_method_overrides', + 'explicit_leastsq_overrides', + 'unmapped_method', + ], + ) + def test_get_fit_kws_tolerance( + self, minimizer: LMFit, minimizer_method, passed_method, expected + ) -> None: + # When + minimizer._method = minimizer_method + + # Then + fit_kws = minimizer._get_fit_kws(passed_method, 0.1, None) + + # Expect + assert fit_kws == expected + + def test_get_fit_kws_no_tolerance(self, minimizer: LMFit) -> None: + # When Then + fit_kws = minimizer._get_fit_kws(None, None, {'existing': 'kwarg'}) + + # Expect + assert fit_kws == {'existing': 'kwarg'} + def test_fit_progress_callback(self, minimizer: LMFit) -> None: # When progress_callback = MagicMock(return_value=True) @@ -517,9 +559,9 @@ def test_set_parameter_fit_result_no_stack_status_no_error(self, minimizer: LMFi # Expect assert minimizer._cached_pars['a'].value == 1.0 - assert minimizer._cached_pars['a'].error == 0.0 + assert minimizer._cached_pars['a'].error is None assert minimizer._cached_pars['b'].value == 2.0 - assert minimizer._cached_pars['b'].error == 0.0 + assert minimizer._cached_pars['b'].error is None def test_gen_fit_results(self, minimizer: LMFit, monkeypatch) -> None: # When