Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sigma=None with absolute_sigma=True in scipy.optimize.curve_fit() unsenseful #12584

Open
juhuebner opened this issue Jul 19, 2020 · 1 comment

Comments

@juhuebner
Copy link

Leaving sigma=None and putting absolute_sigma=True in scipy.optimize.curve_fit() still gives an output, which potentially entails wrongly interpreted results because in this very case a) the weighting is defaulted to unity and b) the option absolute_sigma=True makes scipy.optimize.curve_fit() think the real errors are all ones. The correct output would be only given by curve_fit(func, x_data, y_data) with absolute_sigma=False which is the default already and the covariance matrix being scaled as already implemented.

Inserting the following query here https://github.com/scipy/scipy/blob/v1.5.1/scipy/optimize/minpack.py#L766 in scipy.optimize.curve_fit() would avoid a wrong unintend use:

    ...
    elif absolute_sigma:
        raise ValueError("sigma=None and absolute_sigma=True should not be used at the same time!")
    else:
        transform = None

and in the docstring:

        ...
        
    absolute_sigma : bool, optional
        If True, `sigma` is used in an absolute sense and the estimated parameter
        covariance `pcov` reflects these absolute values, i.e., `pcov` is not scaled by the reduced chi-squared.
        Can not be used together with `sigma=None`.
        
        ...

Reproducing code example:

import numpy as np
from pytest import raises as assert_raises
from scipy.optimize import curve_fit

y = np.asfarray([1, 2.5, 3, 4])
x = np.asfarray([1, 2, 3, 4])

def func(x, a, b):
    return a*x+b

with assert_raises(ValueError, match="sigma=None and absolute_sigma=True should not be used at the same time!"):
    curve_fit(func, x, y, absolute_sigma=True)

Error message:

Failed: DID NOT RAISE <class 'ValueError'>

Scipy/Numpy/Python version information:

1.4.1 1.18.1 sys.version_info(major=3, minor=7, micro=3, releaselevel='final', serial=0)
@RobertoFranceschini
Copy link

Yeah, I think this is a pitfall some people may fall for. So I would put at least a warning when absolute sigma is used. However, I must say that if you make it to the point to say that your errors are absolute, you should have at least added something for the error itself via the sigma argument. Otherwise why did you specify the errors were absolute?

The fact that absolute error are not the default, instead, is very far from my expectation, but this may vary from a field of application to another. Still it stinks for me...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants