Skip to content
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

Added opt. fname argument to sparseL and condVar #545

Merged
merged 4 commits into from
Aug 24, 2021
Merged

Added opt. fname argument to sparseL and condVar #545

merged 4 commits into from
Aug 24, 2021

Conversation

dmbates
Copy link
Collaborator

@dmbates dmbates commented Jul 22, 2021

  • sparseL gains an optional fname argument. When a grouping factor name is given for this argument the return value consists of the lower-right triangle starting at that the rows and columns for that factor.
  • condVar gains a method with a second argument fname. When given the conditional variance array for that factor only is returned.
  • the default condVar now just iterates over the grouping factor names. (The old code is retained for comparison but will never be executed - it should be removed after review of this PR.)
  • the main use of condVar is in MixedModelsMakie.caterpillar which can now be simplified somewhat.
  • this should provide considerable time savings for models like those fit by @kliegl to the FFGK21 dataset when only the caterpillar plot for the Schools is desired. This still needs to be checked out.
  • Needs specific tests for the newly introduced methods.

@dmbates dmbates marked this pull request as draft July 22, 2021 19:58
@dmbates dmbates requested a review from palday July 22, 2021 19:58
@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #545 (05e8311) into main (26a87d5) will increase coverage by 1.68%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
+ Coverage   94.44%   96.13%   +1.68%     
==========================================
  Files          27       27              
  Lines        2468     2485      +17     
==========================================
+ Hits         2331     2389      +58     
+ Misses        137       96      -41     
Impacted Files Coverage Δ
src/linearmixedmodel.jl 97.83% <100.00%> (+2.44%) ⬆️
src/generalizedlinearmixedmodel.jl 89.83% <0.00%> (-0.37%) ⬇️
src/remat.jl 96.70% <0.00%> (ø)
src/optsummary.jl 97.72% <0.00%> (+0.05%) ⬆️
src/Xymat.jl 90.00% <0.00%> (+2.50%) ⬆️
src/linalg/cholUnblocked.jl 100.00% <0.00%> (+9.09%) ⬆️
src/linalg.jl 98.03% <0.00%> (+35.21%) ⬆️

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 26a87d5...05e8311. Read the comment docs.

src/linearmixedmodel.jl Outdated Show resolved Hide resolved
src/linearmixedmodel.jl Outdated Show resolved Hide resolved
@@ -358,7 +350,7 @@ function condVartables(m::MixedModel{T}) where {T}
return NamedTuple{fnames(m)}((map(_cvtbl, condVar(m), m.reterms)...,))
end

function pushALblock!(A, L, blk)
function _pushALblock!(A, L, blk)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed the name of this function to indicate that it was an internal utility.

Co-authored-by: Phillip Alday <[email protected]>
@dmbates dmbates marked this pull request as ready for review August 24, 2021 17:26
Copy link
Member

@palday palday left a comment

Choose a reason for hiding this comment

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

awesome! ready to tag 4.1? 😄

@dmbates
Copy link
Collaborator Author

dmbates commented Aug 24, 2021

awesome! ready to tag 4.1? smile

Sounds good to me.

@dmbates dmbates merged commit 138a5b9 into main Aug 24, 2021
@dmbates dmbates deleted the db/ranefL branch August 24, 2021 17:59
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.

2 participants