Stop considering that Negative Binomial has a dispersion parameter #624

nalimilan · 2025-12-24T21:25:47Z

This is just incorrect. Fix tests to reflect correct degrees of freedom and therefore AIC, BIC and standard errors.

This requires adapting how negbin works as we need to count one additional degree of freedom due to the shape parameter (distinct from the dispersion, which is 1) being estimated from the data. Do this by wrapping the shape parameter in an Estimated type that we use to detect that degrees of freedom need adjusting.

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.

Fixes #564, #617.

I have tentatively implemented the first of three possible approaches I see to the negbin issue to illustrate how it looks:

Wrap the shape parameter of NegativeBinomial in an Estimated type: I find this is somewhat elegant. The disadvantage is that users can't really work with that parameter if they extract it. We could define more methods on this type but that would be weird anyway. We'd better provide an accessor which would unwrap it, which is needed anyway.
Add a field to GeneralizedLinearModel (or one of its component) indicating that one parameter was estimated. This has the advantage of being super simple. The drawback is that its a bit ad-hoc but in practice it probably doesn't matter.
Make negbin return a NegativeBinomialModel <: GLMLikeModel <: LinPredModel. This has the advantage of clarity (in R also glm.nb returns an object of a different class) since these are not really GLMs. In terms of implementation I'm not sure whether it would be easy (just replace AbstractGLM with GLMLikeModel for most methods) or not.

Opinions on the best approach?

Note that R's glm seems broken with negative.binomial as it estimates a dispersion parameter even if it's 1 by theory. You have to do summary(m, dispersion=1) to get the correct standard errors. Yet ?negative.binomial claims it's meant do be used with glm with a fixed shape.

codecov · 2025-12-24T21:33:55Z

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.95%. Comparing base (c5e2b2a) to head (6af4790).

Files with missing lines	Patch %	Lines
src/glmtools.jl	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #624   +/-   ##
=======================================
  Coverage   96.94%   96.95%           
=======================================
  Files           8        8           
  Lines        1213     1216    +3     
=======================================
+ Hits         1176     1179    +3     
  Misses         37       37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/glmtools.jl

andreasnoack · 2026-01-02T16:46:12Z

The two cases here are different, and I think it is best to handle them in separate PRs. Geometric only has a single parameter while NegativeBinomial has two parameters where the second isn't a dispersion parameter in the GLM sense. I agree that we should not have a fallback but only define dispersion_parameter for the cases we want to support. I think the better way to handle the NegativeBinomial case is to introduce a new type just for NegativeBinomial, somewhat like what MASS does, and I think it should just be a direct subtype of LinPredModel, i.e. no need for a new abstract type.

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 (#624).

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).

nalimilan · 2026-01-03T22:32:10Z

It's hard to fix the two cases completely separately as we have tests checking that geometric and negative binomial give the same results, but we can leave some tests as broken temporarily. Let's start by fixing Geometric, which is the simplest: #633.

This is incorrect when the shape parameter has been fixed manually and the model estimated via `glm`. Fix tests to reflect correct degrees of freedom and therefore AIC, BIC and standard errors. This requires adapting how `negbin` works as we need to count one additional degree of freedom due to the shape parameter (distinct from the dispersion, which is 1) being estimated from the data. Do this by returning a `NegativeBinomialModel` instead of a standard GLM.

nalimilan · 2026-01-04T15:50:55Z

I've updated the PR so that it sits on top of #633 and introduced a NegativeBinomial <: AbstractGLM model. Changes are relatively limited, though some functions had to be changed from taking GeneralizedLinearModel to AbstractGLM (this was probably just an oversight).

(CI doesn't run for some reason, but maybe that will work once I base PR on master.)

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).

andreasnoack · 2026-01-05T21:02:51Z

src/glmfit.jl

+For generalized linear models, consumed degrees of freedom correspond
+to the number of estimated coefficients, plus one for the estimated
+dispersion parameter if the distribution is `Gamma`, `InverseGaussian` or `Normal`
+(see [GLM.dispersion_parameter](@ref)).


I think we should just define this as the number of estimated parameters in the model.

Are you sure it is useful to loosen the signature from GeneralizedLinearModel to AbstractGLM? Isn't the only type that is an AbstractGLM but not a GeneralizedLinearModel the new NegativeBinomialModel for which there is a separate model?

I think we should just define this as the number of estimated parameters in the model.

That's actually the definition in the generic docstring in StatsModels: "Return the number of degrees of freedom consumed in the model, including when applicable the intercept and the distribution's dispersion parameter." Here I was trying to be a bit more specific about what this means for different distributions.

Are you sure it is useful to loosen the signature from GeneralizedLinearModel to AbstractGLM? Isn't the only type that is an AbstractGLM but not a GeneralizedLinearModel the new NegativeBinomialModel for which there is a separate model?

It's not really required for this method, but I figured it would be better as it's consistent with what we do for all other methods. I don't have a strong opinion.

andreasnoack · 2026-01-05T21:27:41Z

src/glmfit.jl

 generally written σ for linear models and ϕ for generalized linear models.
-It is, by definition, equal to 1 for the Bernoulli, Binomial, and Poisson families.
+
+It is, by definition, equal to 1 for the Bernoulli, Binomial, Geometric, Negative


I don't think it is correct to say that it is one for the negative binomial. As I understand McCullagh and Nelder, the proper dispersion parameter is only defined in the case when the variance can be written
$$\mathrm{Var}(Y) = \frac{\phi}{m}V(\mu)$$
in which case $\phi$ is called the dispersion parameter. For the negative binomial, the variance is
$$\mathrm{Var}(Y) = \mu + \mu^2/k$$
so $k$ isn't a dispersion parameter according to their definition, but I wouldn't say it is one either. It happens to be the case the Fisher information is $\phi(X^T W X)^{-1}$ for the proper GLMs and for negative binomial, it is just $(X^T W X)^{-1}$ so you can reuse the code for the GLMs if you pass $\phi=1$ but that is just a computational convenience and not because the negative binomial fits into the framework.

Hm. For fixed $k$, I guess it is correct to say that the dispersion is one, so maybe "fixed" should be mentioned in the docstring.

So say "Negative Binomial (with fixed θ)"?

andreasnoack · 2026-01-05T21:30:55Z

src/negbinfit.jl

@@ -1,3 +1,25 @@
+mutable struct NegativeBinomialModel{G<:GlmResp,L<:LinPred} <: AbstractGLM


Maybe just wrap a GeneralizedLinearModel instead of repeating the definition?

With a getproperty override to delegate field accesses? Doesn't really seem simpler in the end?

andreasnoack · 2026-01-05T21:32:51Z

src/lm.jl

 end

+"""
+    dof(model::GeneralizedLinearModel)


Suggested change

dof(model::GeneralizedLinearModel)

dof(model::LinearModel)

nalimilan requested review from andreasnoack, devmotion and palday December 24, 2025 21:25

nalimilan mentioned this pull request Dec 24, 2025

dispersion_parameter(::Geometric) incorrectly returns true #564

Closed

nalimilan added this to the Release 2.0 milestone Dec 24, 2025

nalimilan mentioned this pull request Dec 24, 2025

dof() incorrect for NegativeBinomial() and Geometric() #617

Open

devmotion reviewed Dec 24, 2025

View reviewed changes

src/glmtools.jl Outdated Show resolved Hide resolved

nalimilan mentioned this pull request Dec 26, 2025

Print model information above coefficient table in show #625

Open

nalimilan mentioned this pull request Jan 3, 2026

Stop considering that Geometric has a dispersion parameter #633

Merged

nalimilan force-pushed the nl/dispersion branch from 2bc8e3a to 3693b3c Compare January 4, 2026 15:10

nalimilan changed the base branch from master to nl/geometric January 4, 2026 15:11

nalimilan changed the title ~~Stop considering that Geometric and Negative Binomial have a dispersion parameter~~ Stop considering that Negative Binomial has a dispersion parameter Jan 4, 2026

Fix CI

85b9674

nalimilan closed this Jan 4, 2026

nalimilan reopened this Jan 4, 2026

Try triggering GitHub Actions

6640a7b

Base automatically changed from nl/geometric to master January 4, 2026 15:54

nalimilan added 2 commits January 4, 2026 16:59

Merge branch 'master' into nl/dispersion

5157688

Fix merge

6af4790

andreasnoack reviewed Jan 5, 2026

View reviewed changes

		@@ -1,3 +1,25 @@
		mutable struct NegativeBinomialModel{G<:GlmResp,L<:LinPred} <: AbstractGLM

Stop considering that Negative Binomial has a dispersion parameter #624

Are you sure you want to change the base?

Stop considering that Negative Binomial has a dispersion parameter #624

Uh oh!

Conversation

nalimilan commented Dec 24, 2025

Uh oh!

codecov bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

andreasnoack commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nalimilan commented Jan 3, 2026

Uh oh!

nalimilan commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreasnoack Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 24, 2025 •

edited

Loading

andreasnoack commented Jan 2, 2026 •

edited

Loading

nalimilan commented Jan 4, 2026 •

edited

Loading

andreasnoack Jan 5, 2026 •

edited

Loading