-
Notifications
You must be signed in to change notification settings - Fork 279
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
Model.fit() gives the wrong 'rsquared' when 'weights' is given #921
Comments
@ydy1206 Thanks -- yes, it looks like the R^2 value is definitely very wrong when weights are used! Line 1482 in c415990
should replace self.residual (which, as you say, is weighted) with dat-self.best_fit (probably moving the calculation of self.best_fit that is just a few lines below.
If you are willing and able to make a Pull Request, that would be great. If not, let us know and we'll fix this very soon. |
Thank you for reply, but I'm sorry I don't know how to make a Pull Request. Actually, I just register my Github account today, and I'm afraid I'll mess it up if I make a Pull Request. |
No worries, we'll do this. Thanks! |
I have a small comment to this fix. I have slightly misused lmfit in the sense that I made a Child Model class that supports geometric objects in 2D using shapely and fitting those models. Now to my suggestion, instead of using best_fit, could we use the _residual function evaluated without weights? |
@gyger Is this related to the value of r-squared when using weights? For sure, responding to an issue that was closed many months ago will drive the conversation to "how is this comment/suggestion/report related to the original question".
Well, if you want to use best_fit or your overwritten residual function to calculate some statistic, that seems like it shouldn't be that hard. OTOH, if you changed the residual function so that it uses only uses a limited number of data and model values (not sure why you would need to do that instead of just limiting the data used, but okay), wouldn't that be the more meaningful statistic? I think what we're going to focus on here is what the default value calculated by default after each fit should be. |
It does not directly have something to do with r-squared when using weights, but with the fix to this problem in #923.
This assumes that self.best_fit has points for all data points. This assumption is not always true when the fitting function is not parametrized with a dependent variable e.g. fitting a circle to XY data points with noise, does not necessarily have evaluated data for every data point so the fit function can no longer be called and just exits with a dimension error, because dat and self.best_fit do not have the same size. |
But you chose to restart a conversation in this thread (closed for nearly a year) anyway, instead of asking a question about whatever your having trouble with? That's sort of deliberately distracting. Yes, that section of code was changed, referencing this issue. I don't see any reason to re-open this issue.
Um, what? The fit is evaluated for all the data points, and The independent data might be a different size, and you can set up a model function to generate "current model" from the independent data any way you want. But that model array has to match the input data array. And, if weights are provided, those also much match. The Model fit minimizes If you have a question about lmfit, I strongly recommend asking a question with Discussions or the mailing list, and including real code describing what you are doing. |
Thanks for replying. I mostly indented to put the question in context where the code change was motivated that created a change in the way lmfit behaved. I understand that this can be non-wanted functionality. Happy to open a discussions instead. |
First Time Issue Code
Yes, I read the instructions and I am sure this is a GitHub Issue.
Description
The
ModelResult
instance gained fromModel.fit()
has the wrongrsquared
attribute. Floating pointR^2
statistic, defined for datay
and best-fit modelf
asR^2=1-\sum{(y_i-f_i)^2/(y_i-y_mean)^2}
. In model.py, line1482, the code to calculatersquared
attribute isself.rsquared = 1.0 - (self.residual**2).sum()/max(tiny, sstot)
, but in your code,residual
is notbest_fit-data
but the return value of the objective function when using the best-fit values of the parameters, which is(best_fit-data)*weights
. ifweights=None
inModel.fit()
, the two are same, otherwise they are different. This is a small bug, but I don't know if there's other part where you useresidual
asbest_fit-data
. Hope you check your code carefully. Thank you for all your contributions.A Minimal, Complete, and Verifiable example
Fit report:
Error message:
There's no error message, but as you see,
ModelResult
gives the wrongrsquared
.Version information
Python: 3.12.0 (tags/v3.12.0:0fb18b0, Oct 2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)]
lmfit: 1.2.2, scipy: 1.11.3, numpy: 1.26.0,asteval: 0.9.31, uncertainties: 3.1.7
The text was updated successfully, but these errors were encountered: