WIP: Add TSVD #184
WIP: Add TSVD #184
Conversation
tests/lapack_like/TSVD.cpp
Outdated
| const Real& A = Sk.Get(i,0); | ||
| const Real& B = S.Get(i,0); | ||
| Output( "Sk = ", A, "SVD: S_k", B); | ||
| auto eigenvalue_error = A - B; |
There was a problem hiding this comment.
I presume that this should be Abs(A-B) instead of A-B since you are testing its value being greater than a threshold later. (Also, the line is inconsistently indented and using snake_case for a variable. Also, why use A and B for scalars; that is a bit confusing when most of the library uses Householder notation for matrices and vectors.)
tests/lapack_like/TSVD.cpp
Outdated
| auto eigenvalue_error = A - B; | ||
| auto left_svec_error = Norm(Abs(Uk( ALL, IR(i))) - Abs(U( ALL, IR(i)))); | ||
| auto right_svec_error = Norm(Abs(Vk( ALL, IR(i))) - Abs(V( ALL, IR(i)))); | ||
| if( eigenvalue_error > limits<Real>::Epsilon()){ |
There was a problem hiding this comment.
I think you mean El::limits::Epsilon<Real>(); std::numeric_limits<Real> does not support many of Elemental's datatypes (e.g., BigFloat). The syntax is slightly different and this is the first problem breaking your Travis build.
It would also be a good idea to use a more principled failure condition, such as max(m,n) || A ||_F eps
If this option is not set, the communication will land on the Hydrogen-default stream. This _could_ be a separate stream from the data compute stream, but in the LBANN use case, it generally will be the compute stream.
I'm issuing this pull request to track progress on adding a Lanczos TSVD implementation to Elemental.
This may be slightly premature, so I am submitting a PR to track any work that is needed on this.
Added @andreasnoack and @jiahao. If you guys don't mind reviewing it would be much appreciated.