Separate Algorithm and Driver - part III (Schur-Eig)#194
Separate Algorithm and Driver - part III (Schur-Eig)#194
Algorithm and Driver - part III (Schur-Eig)#194Conversation
|
After the build completes, the updated documentation will be available here |
|
The lists of available algorithms in the user interface page should also be updated, they still point to the LAPACK-specific algorithms through which should become |
| function qr_iteration_eig_full!( | ||
| driver::Driver, A, Dd, V; | ||
| fixgauge::Bool = default_fixgauge(), balanced::Bool = _has_geevx!(driver), kwargs... | ||
| ) | ||
| (balanced ? geevx! : geev!)(driver, A, Dd, V; kwargs...) | ||
| fixgauge && gaugefix!(eig_full!, V) | ||
| return Dd, V | ||
| end |
There was a problem hiding this comment.
So I looked at the LinearAlgebra.jl implementation again, and then also on our geevx! in YALAPACK. What I did not fully remember correctly, is that we were already having balancing, but the way this is expressed in the old version (and also in LinearAlgebra.eigen) is via two keywords permute and scale, both of which have default value true. Indeed, the LAPACK manual describes 4 (= 2 x 2) options for balancing, consisting of a permutation and a scaling step. Probably with both permute = scale = false, or balancing = 'N' in geevx!, it behaves the same as geev!, though this I should check more carefully. But from that perspective, I don't think it makes sense to have a balanced keyword.
Either we just always call geevx! and pass along kwargs..., which can contain scale and permute, and do not expose geev! at all (as it is probably equivalent), or we check explicitly for the case scale == permute == false and route that to geev! (at the 'cost' of not exposing one of the four balancing options of geevx!, namely the one where I suspect it is just geev!).
There was a problem hiding this comment.
Note also that this is only done for the eigenvalue decomposition, not for Schur. For Schur, the difference between ?gees and ?geesx lies elsewhere, namely in the ability to compute condition numbers.
There was a problem hiding this comment.
So basically, we should probably only support geevx (with the keywords), gees and not the others, all under the algorithm name QRIteration.
There was a problem hiding this comment.
While I'm definitely okay with that; the issue then is that the GPU drivers only expose the geev version. I have no real problem myself to just silently use these whenever geevx isn't available, which is why I also didn't add an error but only a warning in the current version, but that is definitely something to keep in mind
There was a problem hiding this comment.
Maybe we can write a GPU geevx! version that simply takes the keywords, check that they are both false, print a warning if not, and pass it on to geev! ?
There was a problem hiding this comment.
This is actually already what I did, so if you are ok with this I will implement it tomorrow.
There was a problem hiding this comment.
I looked a bit more into the ?geev implementation, and it actually does the full balancing (so scaling and permuting). It is just simpler in some of the other options that it allows. But if anything lowers to geev, it should be the scale == permute == true case. So the error should probably be something like "GPU does not allow to disable balancing (scaling and/or permuting)".
4 participants
Follow-up on #178 and #189, addressing #176.
Same treatment for
eig,eighandschurfunctions.