search

const-ae / glmGamPoi

Fit Gamma-Poisson Generalized Linear Models Reliably
103 stars 14 forks source link

overdispersion_shrinkage is not optional #51

Closed alexbarrera closed 1 year ago

alexbarrera commented 1 year ago

Hi team,

I was running some tests using different configurations of glm_nb and I wanted to quantify what the effect of shrinkage is on my data. It appears that test_de will error out in the lines below when turning off overdispersion shrinkage in glm_nb (overdispersion_shrinkage = F). Is this on propose? I understand its usage is strongly recommended, but this looks like a small bug.

https://github.com/const-ae/glmGamPoi/blob/15bdd099efc0926413a3594ed89626dc6d3bd0fd/R/test_de.R#L212-L214

Any thoughts or workarounds?
Thanks, Alex

const-ae commented 1 year ago

Hey Alex,

you are right. I did also run into this a few weeks ago and was scratching my head why I decided to implement it that way. I arrived at the same conclusion as you that this is a bug and I should fix it. I just haven't gotten around to that. I will put it on my to-do list and hope that I will soon get around to addressing the issue.

Best, Constantin

alexbarrera commented 1 year ago

Sounds good, thanks Constantin.

const-ae commented 1 year ago

Hey, I implemented a likelihood ratio test based on the Chi-squared distribution if you call test_de after setting overdispersion_shrinkage = FALSE (https://github.com/const-ae/glmGamPoi/commit/809678d55545300634e0a2299d861e5ae1494680).

In the process, I also realized why I originally had not implemented this test. The problem is in this case the test assumes that the overdispersion values were known. Ignoring the uncertainty about the overdispersion estimates can make the results overconfident.