Skip to content

Commit c5e2b2a

Browse files
authored
Stop considering that Geometric has a dispersion parameter (#633)
This is just incorrect. Fix tests to reflect correct degrees of freedom and therefore AIC, BIC and standard errors. Also avoid defining `dispersion_parameter` fallback for any distribution: it's clearer for users to get a `MethodError` so that they have to define the appropriate method themselves, than silently returning a wrong value. `NegativeBinomial` will be fixed separately as it requires a deeper refactoring of `negbin` (#624).
1 parent 8976b53 commit c5e2b2a

File tree

2 files changed

+26
-18
lines changed

2 files changed

+26
-18
lines changed

src/glmtools.jl

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -452,18 +452,20 @@ devresid(::Poisson, y, μ::Real) = 2 * (xlogy(y, y / μ) - (y - μ))
452452
453453
Does distribution `D` have a separate dispersion parameter, ϕ?
454454
455-
Returns `false` for the `Bernoulli`, `Binomial`, and `Poisson` distributions, `true` otherwise.
455+
Returns `true` for `Gamma`, `InverseGaussian`, `NegativeBinomial`
456+
and `Normal` distributions, and false for other known distributions.
456457
457458
# Examples
458459
```jldoctest; setup = :(using GLM)
459-
julia> show(GLM.dispersion_parameter(Normal()))
460+
julia> GLM.dispersion_parameter(Normal())
460461
true
461-
julia> show(GLM.dispersion_parameter(Bernoulli()))
462+
463+
julia> GLM.dispersion_parameter(Bernoulli())
462464
false
463465
```
464466
"""
465-
dispersion_parameter(D) = true
466-
dispersion_parameter(::Union{Bernoulli,Binomial,Poisson}) = false
467+
dispersion_parameter(::Union{Gamma,InverseGaussian,NegativeBinomial,Normal}) = true
468+
dispersion_parameter(::Union{Bernoulli,Binomial,Geometric,Poisson}) = false
467469

468470
"""
469471
_safe_int(x::T)

test/runtests.jl

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -952,18 +952,23 @@ end
952952
gm22 = glm(@formula(Days ~ Eth + Sex + Age + Lrn), quine, Geometric();
953953
method=dmethod)
954954
test_show(gm22)
955-
@test dof(gm22) == 8
955+
@test dof(gm22) == 7
956956
@test deviance(gm22) 137.8781581814965
957957
@test loglikelihood(gm22) -548.3711276642073
958-
@test aic(gm22) 1112.7422553284146
959-
@test aicc(gm22) 1113.7933502189255
960-
@test bic(gm22) 1136.6111083020812
958+
@test aic(gm22) 1110.742255328415
959+
@test aicc(gm22) 1111.5538495313135
960+
@test bic(gm22) 1131.6275016803734
961961
@test coef(gm22)[1:7] [2.8978546663153897, -0.5701067649409168, 0.08040181505082235,
962962
-0.4497584898742737, 0.08622664933901254, 0.3558996662512287,
963963
0.29016080736927813]
964-
@test stderror(gm22) [0.22754287093719366, 0.15274755092180423, 0.15928431669166637,
965-
0.23853372776980591, 0.2354231414867577, 0.24750780320597515,
966-
0.18553339017028742]
964+
# Values match R with
965+
# summary(glm(cbind(1, Days) ~ Eth + Sex + Age + Lrn, data=quine, family=binomial))
966+
# or summary(glm(..., family=negative.binomial(1)), dispersion=1)
967+
# (as by default summary.glm estimates the dispersion instead of fixing
968+
# it to 1 as it should)
969+
@test stderror(gm22) [0.2556768565484684, 0.17163365085581442, 0.17897863915251788,
970+
0.26802665117909047, 0.2645314640101982, 0.2781103043759507,
971+
0.20847321556654236]
967972
end
968973

969974
@testset "Geometric is a special case of NegativeBinomial with θ = 1 and $dmethod" for dmethod in
@@ -974,14 +979,15 @@ end
974979
gm24 = glm(@formula(Days ~ Eth + Sex + Age + Lrn), quine, NegativeBinomial(1),
975980
InverseLink(); method=dmethod)
976981
@test coef(gm23) coef(gm24)
977-
@test stderror(gm23) stderror(gm24)
978-
@test confint(gm23) confint(gm24)
979-
@test dof(gm23) dof(gm24)
982+
# This is broken as dispersion_parameter(::NegativeBinomial) should be false (#624)
983+
@test_broken stderror(gm23) stderror(gm24)
984+
@test_broken confint(gm23) confint(gm24)
985+
@test_broken dof(gm23) dof(gm24)
980986
@test deviance(gm23) deviance(gm24)
981987
@test loglikelihood(gm23) loglikelihood(gm24)
982-
@test aic(gm23) aic(gm24)
983-
@test aicc(gm23) aicc(gm24)
984-
@test bic(gm23) bic(gm24)
988+
@test_broken aic(gm23) aic(gm24)
989+
@test_broken aicc(gm23) aicc(gm24)
990+
@test_broken bic(gm23) bic(gm24)
985991
@test predict(gm23) predict(gm24)
986992
end
987993

0 commit comments

Comments
 (0)