Support (un)whiten and (inv)quad with static arrays
#183
Conversation
| # map(Base.Fix1(invquad, a), eachcol(x)) or similar alternatives | ||
| # do NOT return a `SVector` for inputs `x::SMatrix`. | ||
| return vec(sum(abs2.(x) .* a.diag; dims = 1)) |
There was a problem hiding this comment.
This is a bit unsatisfying - is there any way we could avoid unnecessary allocations but still make StaticArray return the expected types?
There was a problem hiding this comment.
I added a version with reduced allocations specialized for Array arguments.
| invquad(a::PDiagMat, x::AbstractVector) = invwsumsq(a.diag, x) | ||
| function quad(a::PDiagMat, x::AbstractVecOrMat) | ||
| @check_argdims a.dim == size(x, 1) | ||
| if x isa AbstractVector |
There was a problem hiding this comment.
Not sure if it is helpful/desirable here, but generally I tried to reduce the number of methods to lower the probability for method ambiguity errors. Maybe it's mostly useful for the generic code path in src/generics.jl.
| # map(Base.Fix1(quad, a), eachcol(x)) or similar alternatives | ||
| # do NOT return a `SVector` for inputs `x::SMatrix`. | ||
| wsq = let w = a.value | ||
| x -> w * abs2(x) | ||
| end | ||
| return vec(sum(wsq, x; dims=1)) |
There was a problem hiding this comment.
Any less ugly way to implement this?
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
devmotion
force-pushed
the
dw/better_staticarray
branch
from
edada9f
to
0aca2f3
Compare
| whiten(a::AbstractMatrix{<:Real}, x::AbstractVecOrMat) = whiten(AbstractPDMat(a), x) | ||
| unwhiten(a::AbstractMatrix{<:Real}, x::AbstractVecOrMat) = unwhiten(AbstractPDMat(a), x) |
There was a problem hiding this comment.
Why the restriction on the type parameter?
There was a problem hiding this comment.
Currently, AbstractPDMats are Real matrices:
Line 36 in 9572e79
Now necessary because of JuliaStats/PDMats.jl#183
* Add missing convert methods Now necessary because of JuliaStats/PDMats.jl#179 * Generalize (inv)quad and (un)whiten methods Now necessary because of JuliaStats/PDMats.jl#183 * Increment patch number with `-DEV` suffix * Bump required PDMats version * Add codecov token * Fix definition of whiten * Add definition of `whiten!` * Add more whiten/unwhiten tests * Add more conversion tests * Add tests for (inv)quad! * Fix variable names * Add missing tests * Add correct size tests
Based on #181, this PR also adds support for
whiten,unwhiten,quad, andinvquadwith static arrays.The PR grew a bit larger, hence I kept it separate from #181 for now.
Main points:
Array, see my comment in Support StaticArrays in X(t)_(inv)A_X(t) with ScalMat #181 - should we do this?)f(..., AbstractPDMat(x), ...)(as a nice side effect, this means that more efficient implementations are called automatically wheneverAbstractPDMat(x)is specialized; this is the case, e.g., forDiagonalmatrices).