-
Notifications
You must be signed in to change notification settings - Fork 49
Use 5-argument mul!
instead of mulαβ!
#231
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
Conversation
* Require julia v1.3.0 or later * Remove badge for coveralls * Move `fit` methods from `mixed.jl` to other files * Use 5-argument `mul!` methods * Drop 2-stage optimization for GLMM fit with `fast=false`
Codecov Report
@@ Coverage Diff @@
## master #231 +/- ##
==========================================
+ Coverage 92.89% 94.65% +1.75%
==========================================
Files 18 17 -1
Lines 1295 1234 -61
==========================================
- Hits 1203 1168 -35
+ Misses 92 66 -26
Continue to review full report at Codecov.
|
@@ -38,14 +38,36 @@ mutable struct OptSummary{T <: AbstractFloat} | |||
feval::Int | |||
optimizer::Symbol | |||
returnvalue::Symbol | |||
nAGQ::Integer # doesn't really belong here but I needed some place to store it | |||
nAGQ::Integer # don't really belong here but I needed a place to store them |
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.
I would leave the singular verb and pronoun here.
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.
I meant to indicate that the two last values in the structure are not related to the optimization per se but are tucked away in here for lack of a better place to put them.
I think we're still missing tests for one-stage |
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.
There are tests with fast=true
in the "contra" and "grouseticks" test sets in test/pirls.jl
. Did you want something in addition to those?
Something that would be problematic under the old two-stage method perhaps (I have some things from |
I've got the models for such a test sketched out in my fork and can move them over if you think testing the old boundary cases makes sense. Right now, MixedModels.jl and lme4 deliver somewhat different answers for the Bernoulli model and the correct answer for the Gamma model remains elusive. |
Starting in Julia v1.3.0 methods for
LinearAlgebra.mul!
can be defined for 5 arguments, the last two being scalar multiples ofA*B
and ofC
. These are used in the blocked Cholesky factorization in theupdateL!
method for this package, which is the computational workhorse. Previous versions of this package defined a generic calledmulαβ!
for this but now that functionality can be expressed as methods formul!
.Formatter
packagefast=false
(the default) no longer use a 2-stage optimization (fixes NLopt error for Bernoulli models with random slopes when NOT using fast=true #220, closes Force non-zero step size for general optimization after boundary fit in fast=true #201, closes Watch for boundary cases from fast=true in glmm fits #95)