cuDSS implementations of Cholesky solver and refactorization - #459
Conversation
|
Fixed the changelog "conflict". Reviewing and testing this now. Please pull before making new changes. |
| error_sum += cudssMatrixCreateDn(&rhs_descr, n, 1, n, rhs->getData(memory::DEVICE), CUDA_R_64F, CUDSS_LAYOUT_COL_MAJOR); | ||
| error_sum += cudssMatrixCreateDn(&x_descr, n, 1, n, x->getData(memory::DEVICE), CUDA_R_64F, CUDSS_LAYOUT_COL_MAJOR); |
There was a problem hiding this comment.
Nothing to fix here, but I noted that CUDA doesn't support row major. This could cause conversion issues with the code down the line.
There was a problem hiding this comment.
Please make a new example which uses cudss. Ensure that the old examples with cuSolver still work with no changes. Also, unless you have tested and shown that transposing on the host is faster, I would transpose on the device. I expect it to be faster for large matrices. No need to reimplement everything, just call our existing functions.
What are you referring to by transpose? |
Done. The new example |
| cudssMatrixDestroy(descr_b_); | ||
| cudssMatrixDestroy(descr_x_); | ||
| } | ||
| else |
There was a problem hiding this comment.
what is this else for? it seems within the nested if, but then the commands are outside the outer if
There was a problem hiding this comment.
If RESOLVE_USE_CUDSS is defined, there is an if/else that checks use_cudss_ and takes one branch. If RESOLVE_USE_CUDSS is not defined, there is no if/else, so the else is compiled out.
| #ifdef RESOLVE_USE_CUDSS | ||
| } | ||
| else if (refactorizationMethod_ == "cudssrf") | ||
| { | ||
| refactorizationSolver_ = new ReSolve::LinSolverDirectCuDssRf(); | ||
| #endif |
There was a problem hiding this comment.
I find this writing style very confusing, though I see you are copying it and see why it works. I really this brackets should be within #ifdef statements.
| #ifdef RESOLVE_USE_CUDSS | ||
| else if (refactorizationMethod_ == "cudssrf") | ||
| { | ||
| LinSolverDirectCuDssRf* Rf = dynamic_cast<LinSolverDirectCuDssRf*>(refactorizationSolver_); | ||
| Rf->setNumericalProperties(1e-14, 1e-1); | ||
|
|
||
| status += refactorizationSolver_->setup(A_, L_, U_, P_, Q_); | ||
|
|
||
| is_solve_on_device_ = false; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This way looks much better and is easier to follow.
Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com>
shakedregev
left a comment
There was a problem hiding this comment.
Great work. Checked the tests and examples
Description
Added cuDSS implementations of Cholesky solver and refactorization on CUDA. These are currently implemented in cuSolver, which is an older library. Updated unit tests for refactorization to test both the cuDSS and cuSolver implementations.
Proposed changes
Cholesky solver: rewrote with cuDSS, but retained cuSolver implementation. The user can pass the
use_cudssparameter in the constructor to decide which version to use. It is true by default. The cuDSS implementation is around 4x the speed of the cuSolver implementation.Refactorization: added
LinSolverDirectCuDssRfclass as an independent solver. It is very slightly slower than the cuSolver implementation. The interface is kept unchanged, but the following variables are actually unused:LandUare allocated internally by cuDSSQis unused, onlyP. cuDSS only takes one permutation vector and computes the othernboost/pivot_boosthave no equivalents in cuDSSOther changes:
SystemSolver.cppto acceptcudssrfas a factorization methodtestRefactor.cppandtestSysRefactor.cppto run tests on both implementationsChecklist
make testandmake test_installper testing instructions). Code tested on./examples/<your_example>.exe -hto get instructions how to run examples). Code tested on:-Wall -Wpedantic -Wconversion -Wextra.Further comments