-
Notifications
You must be signed in to change notification settings - Fork 115
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
backport of QR to 1.x #537
base: v1
Are you sure you want to change the base?
Changes from all commits
cd7b2b1
2d91725
97cab57
ae88551
173d3b7
5269e7a
23321a8
92ec3fa
cd187fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -283,7 +283,8 @@ function nulldeviance(m::GeneralizedLinearModel) | |
X = fill(1.0, length(y), hasint ? 1 : 0) | ||
nullm = fit(GeneralizedLinearModel, | ||
X, y, d, r.link; wts=wts, offset=offset, | ||
dropcollinear=isa(m.pp.chol, CholeskyPivoted), | ||
dropcollinear=ispivoted(m.pp), | ||
method=decomposition_method(m.pp), | ||
maxiter=m.maxiter, minstepfac=m.minstepfac, | ||
atol=m.atol, rtol=m.rtol) | ||
dev = deviance(nullm) | ||
|
@@ -338,7 +339,8 @@ function nullloglikelihood(m::GeneralizedLinearModel) | |
X = fill(1.0, length(y), hasint ? 1 : 0) | ||
nullm = fit(GeneralizedLinearModel, | ||
X, y, d, r.link; wts=wts, offset=offset, | ||
dropcollinear=isa(m.pp.chol, CholeskyPivoted), | ||
dropcollinear=ispivoted(m.pp), | ||
method=decomposition_method(m.pp), | ||
maxiter=m.maxiter, minstepfac=m.minstepfac, | ||
atol=m.atol, rtol=m.rtol) | ||
ll = loglikelihood(nullm) | ||
|
@@ -569,6 +571,7 @@ function fit(::Type{M}, | |
d::UnivariateDistribution, | ||
l::Link = canonicallink(d); | ||
dropcollinear::Bool = true, | ||
method::Symbol = :cholesky, | ||
dofit::Bool = true, | ||
wts::AbstractVector{<:Real} = similar(y, 0), | ||
offset::AbstractVector{<:Real} = similar(y, 0), | ||
|
@@ -580,7 +583,15 @@ function fit(::Type{M}, | |
end | ||
|
||
rr = GlmResp(y, d, l, offset, wts) | ||
res = M(rr, cholpred(X, dropcollinear), false) | ||
|
||
if method === :cholesky | ||
res = M(rr, cholpred(X, dropcollinear), false) | ||
elseif method === :qr | ||
res = M(rr, qrpred(X, dropcollinear), false) | ||
else | ||
throw(ArgumentError("The only supported values for keyword argument `method` are `:cholesky` and `:qr`.")) | ||
end | ||
|
||
return dofit ? fit!(res; fitargs...) : res | ||
end | ||
|
||
|
@@ -603,8 +614,9 @@ $FIT_GLM_DOC | |
""" | ||
glm(X, y, args...; kwargs...) = fit(GeneralizedLinearModel, X, y, args...; kwargs...) | ||
|
||
GLM.Link(r::GlmResp) = r.link | ||
GLM.Link(m::GeneralizedLinearModel) = Link(m.rr) | ||
Link(r::GlmResp) = r.link | ||
Link(m::GeneralizedLinearModel) = Link(m.rr) | ||
Link(m::StatsModels.TableRegressionModel{<:GeneralizedLinearModel}) = Link(m.model) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit surprised that you need to add this, as usually method delegations were only done in StatsModels, for functions from StatsAPI. For example, below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They're called in the tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah but I wonder why we never needed this kind of definition before. I think the code was designed so that we never called functions on objects which could either be Maybe this doesn't matter much as 1.x is a dead-end anyway. |
||
|
||
Distributions.Distribution(r::GlmResp{T,D,L}) where {T,D,L} = D | ||
Distributions.Distribution(m::GeneralizedLinearModel) = Distribution(m.rr) | ||
|
@@ -631,6 +643,8 @@ function dispersion(m::AbstractGLM, sqr::Bool=false) | |
end | ||
end | ||
|
||
dispersion(m::StatsModels.TableRegressionModel{<:AbstractGLM}, args...; kwargs...) = dispersion(m.model, args...; kwargs...) | ||
|
||
""" | ||
predict(mm::AbstractGLM, newX::AbstractMatrix; offset::FPVector=eltype(newX)[], | ||
interval::Union{Symbol,Nothing}=nothing, level::Real = 0.95, | ||
|
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.
Is this intentional?
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.
This is what
doctest
updated things to.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.
Weird... :-/