-
Notifications
You must be signed in to change notification settings - Fork 117
Residuals in GLM #540
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
base: pa/resid
Are you sure you want to change the base?
Residuals in GLM #540
Conversation
|
This PR seems to be picking up some older commits and needs cleaning up. |
|
Thanks, Viral. |
* [G]VIF * add reference value source * more tests * glm tests
|
The PR is now cleaned up and merged with the latest master branch. |
| import Base: (\), convert, show, size | ||
| import LinearAlgebra: cholesky, cholesky! | ||
| import Statistics: cor | ||
| using StatsAPI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be in using part
| residuals(obj::LinPredModel) = residuals(obj.rr) | ||
|
|
||
| function formula(obj::LinPredModel) | ||
| function residuals(model::LinPredModel; type=:deviance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know the details of the implementation, but is the residuals method properly documented (as it is not documented here). I.e. that it supports type kwarg and what is the behavior depending on the value of the kwarg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is documented on line 847 of glmfit.jl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That docstring is attached only to the method for GeneralizedLinearModel. It should be attached to a method for a more general type (probably LinPredModel).
BTW, I don't think this method is correct as LinPredModel includes GeneralizedLinearModel. It should be restricted to LinearModel, and moved to lm.jl.
|
what's the status of this PR? Can it get a bit of love? |
| function formula(obj::LinPredModel) | ||
| function residuals(model::LinPredModel; type=:deviance) | ||
| type in _RESIDUAL_TYPES || | ||
| throw(ArgumentError("Unsupported type `$(type)``; supported types are" * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| throw(ArgumentError("Unsupported type `$(type)``; supported types are" * | |
| throw(ArgumentError("Unsupported type `$(type)`; supported types are " * |
(Could be fixed in the GLM method too BTW.)
| residuals(obj::LinPredModel) = residuals(obj.rr) | ||
|
|
||
| function formula(obj::LinPredModel) | ||
| function residuals(model::LinPredModel; type=:deviance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That docstring is attached only to the method for GeneralizedLinearModel. It should be attached to a method for a more general type (probably LinPredModel).
BTW, I don't think this method is correct as LinPredModel includes GeneralizedLinearModel. It should be restricted to LinearModel, and moved to lm.jl.
I have added most of the test cases as discussed in #499.
Also, I have changed the residuals function for
lm. Now the residuals function forlmis similar to residuals function in 'glm'.Although, I am not sure about applicability of
:workingtype residuals inlm.Rsupports:workingtype residuals inlmand which is same as:responsetype.