tBuLi / symfit

Symbolic Fitting; fitting as it should be.
http://symfit.readthedocs.org
MIT License
233 stars 17 forks source link

FitResults should show more goodness of fit qualifiers besides R^2 #85

Closed tBuLi closed 5 years ago

tBuLi commented 7 years ago

More goodness of fit qualifiers should be added to FitResults besides R^2. Depending on the fitting type, we also directly know chi_squared or log_likelihood. These should therefore be available from FitResults.

So at the very least chi_squared, degrees_of_freedom (or dof) and chi_squared_reduced, likelihood and log_likelihood can be added. In case one of these is not present because it is not sensible to ask, it should return None.

likelihood should be a property, log_likelihood an attribute. likelihood can then be calculated from log_likelihood.

chi_squared and degrees_of_freedom should be an attribute, chi_squared_reduced is given by dividing chi_squared by degrees_of_freedom.

Jhsmit commented 7 years ago

Are GoF qualifiers available in FitResults yet? I'm interested in the chi_squared, which I currently get by:

@property
def chi_squared(self):
    return np.sum(
        self.fit.model.numerical_chi_squared(
            *self.fit.fit.data.values(), **self.result.params
        )
   )

Where I've added the second fit in fit.fit.data when updating to symfit 0.3.5. Is this still the correct/best way to get the chi_squared?

tBuLi commented 7 years ago

Yeah, unfortunately you have to create a workaround like yours. This is a priority item to fix with the next release.

So you created this property on an object you use? Personally I would have done the same as you, but it should also be possible to use

 np.sum(result.infodict['fvec']**2)

result.infodict['fvec'] contains the residuals evaluated at the solution as it is returned by scipy.optimize.leastsq. As I am typing this I realize there might be a bug here: I think fvec might be a vector of the residuals when returned under the hood by NumericalLeastSquares and the scalar chi^2 when ConstrainedNumericalLeastSquares was used. So please check this against your method before implementing this :P.

Jhsmit commented 7 years ago

Yes I should have mentioned that this a property in one of my own objects. And your suggestion indeed gives the same result as my property. I'm not using constraints at the moment so I should be fine.

tBuLi commented 7 years ago

Glad to hear it. Could you briefly try to use a ConstrainedNumericalLeastSquares object instead of the Fit object? If that is not the same it is definitely a bug.