add spectra routine for minPosLinearEigenvalue_EigenSymSolver #303
Conversation
| Spectra::DenseCholesky<NT> Bop(B); | ||
|
|
||
| //construct generalized eigen solver object, requesting the smallest eigenvalue | ||
| Spectra::SymGEigsSolver<NT, Spectra::SMALLEST_ALGE, Spectra::DenseSymMatProd<NT>, Spectra::DenseCholesky<NT>, Spectra::GEIGS_CHOLESKY> |
There was a problem hiding this comment.
You need the largest eigenvalue here, i.e., Spectra::LARGEST_ALGE.
| eigvec = geigs.eigenvectors().col(0); | ||
| } | ||
|
|
||
| lambdaMinPositive = evalues(0); |
There was a problem hiding this comment.
Since we compute the largest eigenvalue here we need lambdaMinPositive = NT(1) / evalues(0); to obtain the smallest positive eigenvalue.
| Spectra::DenseSymMatProd<NT> op(A); | ||
| Spectra::DenseCholesky<NT> Bop(B); |
There was a problem hiding this comment.
The matrix A is the correlation matrix (the positive definite matrix). So, I guess the correct operations are:
Spectra::DenseSymMatProd op(B);
Spectra::DenseCholesky Bop(A);
We initially want to compute the smallest positive eigenvalue of the GEP, A = tB. So, here we need to compute the largest eigenvalue of the GEP, tA = B and invert it. That's why the operations has to be as above.
|
|
||
| //check if all the diagonal elements are ones | ||
| for(int i=0 ; i<matrix.rows() ; i++){ | ||
| if(matrix(i,i) != 1.0){ |
There was a problem hiding this comment.
Can you do an epsilon-equality check here?
For example if (std::abs(matrix(i,i) - 1.0) > tol)
where const tol = 1e-08.
There was a problem hiding this comment.
Yes, I can, thanks for this! I will make the changes.
| if(eigen_solver.info() != Eigen::Success) return false; | ||
|
|
||
| //the matrix is positive definite if all eigenvalues are positive | ||
| return eigen_solver.eigenvalues().minCoeff() > 0; |
| if(eigen_solver.info() != Eigen::Success) return false; | ||
|
|
||
| //the matrix is positive definite if all eigenvalues are positive | ||
| return eigen_solver.eigenvalues().minCoeff() > tol; |
There was a problem hiding this comment.
I think it's better to check return eigen_solver.eigenvalues().minCoeff() > -tol;
There was a problem hiding this comment.
Okay yes, that sounds better! I'll incorporate this change
| write_to_file<PointMT>(walkname + "_matrices_MT.txt", randPoints); | ||
|
|
||
| //write_to_file<PointMT>(walkname + "_matrices_MT.txt", randPoints); |
There was a problem hiding this comment.
Yes, we should delete this line
| @@ -12,7 +12,7 @@ | |||
| #define RANDOM_WALKS_UNIFORM_BILLIARD_WALK_HPP | |||
|
|
|||
| #include <Eigen/Eigen> | |||
|
|
|||
| #include <iostream> | |||
There was a problem hiding this comment.
Did you maybe forget to delete it?
There was a problem hiding this comment.
Yes I'm sorry for this- I deleted it just now
header not required.
|
Sorry for reverting this, but I would like to ask for one more review before merging. |
I added the spectra routine for minPosLinearEigenvalue_EigenSymSolver in volesti/include/matrix_operations/EigenvaluesProblems.h