-
Notifications
You must be signed in to change notification settings - Fork 49
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
Combine updateL! operations on diagonal blocks #648
Conversation
Codecov ReportBase: 94.24% // Head: 94.23% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #648 +/- ##
==========================================
- Coverage 94.24% 94.23% -0.02%
==========================================
Files 29 29
Lines 2797 2792 -5
==========================================
- Hits 2636 2631 -5
Misses 161 161
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@inbounds for i in diagind(Ljj) | ||
Ljj[i] *= lambsq | ||
Ljj[i] += one(T) | ||
@inbounds for (i, a) in enumerate(Ajj.diag) |
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.
since we only ever create Diagonal{T, Vector{T}}
this shouldn't cause trouble, but strictly speaking, there could be Diagonal{T, StarWarsVector{T}}
which would cause undefined behavior with @inbounds
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.
Anyone who chooses to use a StarWarsVector
in real life deserves undefined behavior. 😄
@dmbates don't forget to bump the patch |
coverage suggests you could delete this method: function Base.copyto!(dest::UniformBlockDiagonal{T}, src::UniformBlockDiagonal{T}) where {T}
sdat = src.data
ddat = dest.data
size(ddat) == size(sdat) || throw(DimensionMismatch(""))
copyto!(ddat, sdat)
return dest
end |
closes #643
updateL!
was to copy blocks ofA
to the corresponding block ofL
then iterate over the random effects terms scaling the diagonal block and inflating the diagonal.copyto!
andscaleinflate!
bycopyscaleinflate!
.L
.Did behavior change? Did you add need features? If so, please update NEWS.md
docs/NEWS-update.jl
to update the cross-references.Should we release your changes right away? If so, bump the version: