Implement the HyKKT Cholesky module#350
Merged
shakedregev merged 28 commits intohykkt-devfrom Sep 2, 2025
Merged
Conversation
adhamsi
force-pushed
the
adham/hykkt-cholesky
branch
from
339fb5c to
b5f357d
Compare
shakedregev
force-pushed
the
adham/hykkt-cholesky
branch
from
dcceadf to
9c471ea
Compare
shakedregev
reviewed
Aug 25, 2025
| if(RESOLVE_USE_KLU) | ||
| add_subdirectory(hykkt) | ||
| endif() | ||
| #list(APPEND ReSolve_Targets_List resolve_hykkt) |
Collaborator
There was a problem hiding this comment.
Why are we removing this from the list of targets?
Collaborator
Author
There was a problem hiding this comment.
resolve_hykkt is the target for only the Permutation class. This is a holdover from your original commit of the Permutation class. The name hasn't been changed and needs to be. The other hykkt modules are named resolve_hykkt_ruiz, etc. But it appears adding it to the list of targets is redundant if the subdirectory is added.
shakedregev
reviewed
Aug 25, 2025
shakedregev
reviewed
Aug 25, 2025
shakedregev
reviewed
Aug 25, 2025
shakedregev
reviewed
Aug 25, 2025
Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com>
shakedregev
reviewed
Aug 27, 2025
shakedregev
approved these changes
Aug 28, 2025
Collaborator
shakedregev
left a comment
shakedregev
left a comment
There was a problem hiding this comment.
CUDA and CPU tests pass. Approved, pending documentation change requests.
HIP is not working because as far as we can tell their Cholesky solver does not work (verified with @kswirydo from AMD). We will need to add a separate test for HIP once we figure out what is going on with their Cholesky solver.
Collaborator
|
We are investigating the issue on AMD GPUs. I got the same failure for identity matrix too ... |
shakedregev
added a commit
that referenced
this pull request
Nov 13, 2025
* initial solver and cuda header * cuda implementation * minimal test on cuda passes * cpu implementation * tell cmake to find cholmod * add template hip implementation * dependencies * cpu: memory freeing and unused fields * Apply pre-commmit fixes * error handling and correct memory in cuda * randomized testing: different sparsity patterns * Apply pre-commmit fixes * fix for cuda: resetting workspace * Apply pre-commmit fixes * test reusing sparsity pattern * Apply pre-commmit fixes * Apply pre-commmit fixes * draft: rocsolver * Apply pre-commmit fixes * fix build issue * documentation * Apply pre-commmit fixes * error message when no gpu support Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com> * address minor review comments * Apply pre-commmit fixes * remove commented code * more detail on documentation * Apply pre-commmit fixes --------- Co-authored-by: adhamsi <adhamsi@users.noreply.github.com> Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com>
pelesh
added a commit
that referenced
this pull request
Nov 20, 2025
* Added the permutation class from hykkt, along with tests. * Implement HyKKT Ruiz Scaling (#317) * Implement the HyKKT Cholesky module (#350) * SpGEMM for HyKKT (#366) --------- Co-authored-by: superwhiskers <whiskerdev@protonmail.com> Co-authored-by: Nicholson Koukpaizan <72402802+nkoukpaizan@users.noreply.github.com> Co-authored-by: Adham Ibrahim <adham.ibrahim@princeton.edu> Co-authored-by: kswirydo <kasia.swirydowicz@gmail.com> Co-authored-by: shakedregev <shakedregev@users.noreply.github.com> Co-authored-by: Slaven Peles <peless@ornl.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This implements the Cholesky factorization module, one of the five HyKKT modules. The module supports computing the symbolic and numeric factorization of a sparse positive definite matrix (the H_gamma matrix from the HyKKT paper), and updating the nonzeros of the matrix and reusing the symbolic factorization. The module then provides a
solve(vector::Vector* x, vector::Vector* b)function.Closes #311
Proposed changes
cusolver.cholmod_sparsetype, which is an inideal solution, but acceptable given that the CPU implementation is primarily for correctness and comparison to GPU implementations.rocsolverlibrary refactorization and triangular solve functions.Checklist
-Wall -Wpedantic -Wconversion -Wextra.