parametrize Tridiagonal on the wrapped vector type #23154
Conversation
fredrikekre
force-pushed
the
fe/tridiagonal
branch
from
a54fa4e
to
627f751
Compare
fredrikekre
added
kind:breaking
|
Meh, had to fix something so lets redo this |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan |
NEWS.md
Outdated
| wrapped vectors, allowing `Diagonal`, `Bidiagonal` and `SymTridiagonal` matrices with | ||
| arbitrary `AbstractVector`s ([#22718], [#22925], [#23035]). | ||
| * `Diagonal`, `Bidiagonal`, `Tridiagonal` and `SymTridiagonal` are now parameterized on the | ||
| type of the wrapped vectors, allowing `Diagonal`, `Bidiagonal` and `SymTridiagonal` |
| A.uplo == 'U' ? Tridiagonal(z, convert(Vector{T},A.dv), convert(Vector{T},A.ev)) : Tridiagonal(convert(Vector{T},A.ev), convert(Vector{T},A.dv), z) | ||
| dv = convert(AbstractVector{T}, A.dv) | ||
| ev = convert(AbstractVector{T}, A.ev) | ||
| z = fill!(similar(ev), zero(T)) |
There was a problem hiding this comment.
To check my understanding, converting a Bidiagonal over Ranges to a Tridiagonal should fail?
There was a problem hiding this comment.
Right..., Yea I guess that’s a consequence of this series of PR's? 😄
There was a problem hiding this comment.
If we're converting from Diagonal or Bidiagonal or SymTridiagonal that have fewer diagonal vectors and they already have to be a consistent type within those structs, can't we use that same parameter to know which new diagonal vector type to fill in when converting to Tridiagonal?
There was a problem hiding this comment.
In the case of a Range: You can not create a Range of a given length with only zeros, right? That is the only failure case here I think...
Using Ranges results in other problems as well, so I guess you will just have to convert to a vector format that supports a larger set of functions.
fredrikekre
force-pushed
the
fe/tridiagonal
branch
2 times, most recently
from
b805157
to
ff20c2d
Compare
|
Any more comments? |
fredrikekre
force-pushed
the
fe/tridiagonal
branch
from
ff20c2d
to
3cf2634
Compare
|
Rebased to run through CI once more before merging since its been over a week. |
|
InterruptException on macos, restarted. Also, there are 3200 lines of depwarns in the logs from Documenter and friends, any chance we could upgrade those dependencies? :) |
|
sure, if there are new versions to upgrade to |
Last PR in this series. This is definitely not as straight forward as #22718, #22925 and #23035 so please review carefully. It took some thinking to do this in a nice way, hence I saved it for last. I am quite pleased with the current implementation.
The internal field
.du2was only used inlufact!, and for all other operations it should have been considered hidden. Since we never initialize that field otherwise, other operations onTridiagonalshould allocate less after this change:@nanosoldier
runbenchmarks("linalg", vs = ":master")The other good thing with not initializing that field always, is that we don't need to use similar, to construct it, since that is not applicable for all
AbstractVectors (e.g.Tridiagonal(1:2, 1:3, 1:2)would not work, how to construct the.du2field?). Now it is only needed inlufact!.