Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEA] Lanczos solver v2 #2481

Merged
merged 38 commits into from
Nov 20, 2024
Merged

Conversation

lowener
Copy link
Contributor

@lowener lowener commented Nov 1, 2024

I unfortunately don't have permissions to push on @aamijar branch for the previous Lanczos solver PR (#2416) so I kept his commits and continued it here.

Lanczos Solver for Sparse Eigen Decomposition

We propose a new lanczos solver in raft that fixes the issues present in the previous solver raft::sparse::solver::detail::computeSmallestEigenvectors.

Specifically we address the following issues:

  1. Numerical Stability for both float32 and float64 datatypes
  2. Efficiency and Speed of Convergence

This new implementation is taken from the cupy library cupyx.scipy.sparse.linalg.eigsh where the thick-restart and full reorthogonalzation methods are used.

Additionally this PR exposes a python api for raft lanczos solver with an interface similar to scipy.sparse.linalg.eigsh and cupyx.scipy.sparse.linalg.eigsh.

from pylibraft.solver import eigsh

@lowener lowener requested review from a team as code owners November 1, 2024 21:28
@lowener
Copy link
Contributor Author

lowener commented Nov 5, 2024

/ok to test

@lowener
Copy link
Contributor Author

lowener commented Nov 14, 2024

/ok to test

@cjnolet
Copy link
Member

cjnolet commented Nov 16, 2024

/ok to test

@lowener
Copy link
Contributor Author

lowener commented Nov 18, 2024

/ok to test

@lowener
Copy link
Contributor Author

lowener commented Nov 19, 2024

/ok to test

@lowener lowener linked an issue Nov 20, 2024 that may be closed by this pull request
void lanczos_solver( \
const raft::resources& handle, \
raft::sparse::solver::lanczos_solver_config<ValueType> config, \
raft::device_vector_view<IndexType, uint32_t, raft::row_major> rows, \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too- please consolidate this list by using device_coo_matrix_view.

Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to get this PR in because code freeze is looming so I'm merging this now. @lowener can we please follow-up by exposing the device_coo_matrix_view in the public API insted of the individual arrays? It should be a 1-2 line change.

@cjnolet
Copy link
Member

cjnolet commented Nov 20, 2024

/merge

@rapids-bot rapids-bot bot merged commit 64c09d7 into rapidsai:branch-24.12 Nov 20, 2024
70 checks passed
@lowener lowener deleted the lanczos-solver-new branch November 21, 2024 11:03
@lowener
Copy link
Contributor Author

lowener commented Nov 21, 2024

@cjnolet I think that the name of the parameters (rows, cols, values) is subject to confusion, and should have been instead (indptr, indices, values). The Lanczos solver only supports CSR format, not COO. There is no conversion from COO to CSR happening in the API point I am adding.
The API that supports the arrays individually and not through the sparse data type was added because the bindings for this sparse data type is not yet present in pylibraft. I created #2504 for this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

[FEA] Use new Lanczos solver in raft::sparse::spectral
4 participants