-
Notifications
You must be signed in to change notification settings - Fork 48
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
Better loglikelihood for GLMM #419
Conversation
c1bd575
to
a4f4ddc
Compare
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
==========================================
+ Coverage 94.31% 94.39% +0.07%
==========================================
Files 23 23
Lines 1637 1642 +5
==========================================
+ Hits 1544 1550 +6
+ Misses 93 92 -1
Continue to review full report at Codecov.
|
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.
Sorry not to review this previously. I just noticed now that you asked for my review.
At some point we should revisit the decision to use NaN
for the value of sdest
when there is no dispersion parameter. It seems that in modern Julia we should consider missing
so we can extract a value of type T
with coalesce(sdest(m), one(T))
.
Thanks for straightening out the use of ϕ in loglikelihood
.
…lday/glmmloglikelood
So we had actually revisited that decision in a previous PR (#418, on GLMM parametric bootstrap) -- I've now merged those changes back into this branch and am re-running the tests. |
Amongst other things, this makes it so that the
show(::GeneralizedLinearMixedModel)
method works for models with a dispersion parameter.(
If it's not obvious from the commit history,this is me stripping off part of #291 so that at least some of those improvements make it in sooner rather than later.)