Add LDLT Cholesky decomposition #4130
Conversation
| * | ||
| * @param A The matrix whose LDLT cholesky decomposition is to be | ||
| * computed | ||
| * @param L The matrix that saves tht upper or lower triangular LDLT |
There was a problem hiding this comment.
also remove "upper or lower", just say "triangular LDLT (default: lower)"
| const SGMatrix<T>& A, SGMatrix<T>& L, SGVector<T>& d, | ||
| SGVector<index_t>& p, const bool lower = true) | ||
| { | ||
| REQUIRE(A.num_rows == A.num_cols, "Matrix A is not square\n"); |
There was a problem hiding this comment.
Should technically write: "Matrix dimensions (%dx%d) are not square.\n"
| * @return @see LinalgBackendBase pointer | ||
| */ | ||
| template <typename T, template <typename> class Container> | ||
| LinalgBackendBase* infer_backend( |
There was a problem hiding this comment.
Can't we make this a bit nicer here, so that any number of things that have the .on_gpu(), .on_cpu() method implemented is checked?
@lisitsyn can you do some magic ?
| * Solve the linear equations \f$Ax=b\f$, given the LDLT Cholesky | ||
| * factorization of A, | ||
| * where \f$A\f$ is a positive semidefinite or negative semidefinite | ||
| * Hermitan matrix |
There was a problem hiding this comment.
We could add a @see factor (or however it is called) here
| @@ -406,6 +406,44 @@ TEST(LinalgBackendEigen, SGMatrix_cholesky_solver) | |||
| EXPECT_EQ(x_ref.size(), x_cal.size()); | |||
| } | |||
|
|
|||
| TEST(LinalgBackendEigen, SGMatrix_ldlt_solver) | |||
There was a problem hiding this comment.
i wonder why there is "SGMatrix" in the test name?
There was a problem hiding this comment.
Just followed other test cases like SGMatrix_cholesky_solver
vinx13
force-pushed
the
feature/linalg_ldlt
branch
from
6178b40
to
2006141
Compare
| const SGMatrix<T>& A, SGMatrix<T>& L, SGVector<T>& d, | ||
| SGVector<index_t>& p, const bool lower = true) | ||
| { | ||
| REQUIRE( |
There was a problem hiding this comment.
@lisitsyn we need some systematic and compact way to do these checks. The amount of lines of code for such error messages is crazy
| b[2] = 11.0; | ||
|
|
||
| SGVector<float64_t> x_ref(size), x(size); | ||
| x_ref[0] = 0.0; |
There was a problem hiding this comment.
I would prefer a test that is more self explaining.
Like you do
x = A*b
then you solve b2= A^-1 * x
and then you check that b==b2
But this one is OK as well
|
I restarted the CI, after which we can merge this, thanks! @OXPHOS all ok with this? |
|
LGTM! I'll merge it. |
* Add LDLT decomposition * Add LDLT Cholesky solver * Polish error message * Add ref to ldlt_factor
Add LDLT cholesky decomposition and solver for positive semidefinite or negative semidefinite Hermitan matrixes.
See discussion in #4085