Skip to content

Conversation

@palday
Copy link
Member

@palday palday commented Dec 3, 2020

Add optimized rank update for Diagonal L with dense A.

Note that we "found" this optimization via a missing method for a complicated nested type in LinearAlgebra.

Closes #445.

@palday palday requested a review from dmbates December 3, 2020 23:16
@palday
Copy link
Member Author

palday commented Dec 3, 2020

The failure on nightly is a known upstream issue.

@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #446 (030ca8e) into master (52651b7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   92.71%   92.74%   +0.02%     
==========================================
  Files          23       23              
  Lines        1716     1722       +6     
==========================================
+ Hits         1591     1597       +6     
  Misses        125      125              
Impacted Files Coverage Δ
src/linalg/rankUpdate.jl 95.83% <100.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52651b7...030ca8e. Read the comment docs.

Copy link
Collaborator

@dmbates dmbates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to decide why the second model for the pastes data didn't pick up this case. It is because the 2,1 block is sparse in that case.

julia> m1 = fit(MixedModel, @formula(strength ~ 1 + (1|batch/cask)), pastes)
Linear mixed model fit by maximum likelihood
 strength ~ 1 + (1 | batch) + (1 | batch & cask)
   logLik   -2 logLik     AIC       AICc        BIC    
  -123.9972   247.9945   255.9945   256.7217   264.3718

Variance components:
                Column   Variance Std.Dev. 
batch & cask (Intercept)  8.433617 2.904069
batch        (Intercept)  1.199180 1.095071
Residual                  0.678002 0.823409
 Number of obs: 60; levels of grouping factors: 30, 10

  Fixed-effects parameters:
─────────────────────────────────────────────────
               Coef.  Std. Error      z  Pr(>|z|)
─────────────────────────────────────────────────
(Intercept)  60.0533    0.642136  93.52    <1e-99
─────────────────────────────────────────────────

julia> BlockDescription(m1)
rows: batch & cask     batch         fixed     
  30:   Diagonal   
  10:    Sparse       Diagonal   
   1:    Dense         Dense         Dense     

Thanks for addressing this so quickly.

@palday
Copy link
Member Author

palday commented Dec 3, 2020

Ah, so this is then a really good method to have when we experiment with the densify threshold.

@palday palday merged commit f642f5d into master Dec 3, 2020
@palday palday deleted the pa/diagonalstride branch December 3, 2020 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested models

3 participants