Lanczos Solver #2416

aamijar · 2024-08-20T23:22:27Z

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:

Numerical Stability for both float32 and float64 datatypes
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

copy-pr-bot · 2024-08-20T23:22:30Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

cjnolet

I think this is coming along great, @aamijar. Mostly it needs some cleanup and polishing, but otherwise should be ready to merge once my comments are resolved.

cpp/include/raft/sparse/solver/detail/lanczos.cuh

cpp/src/raft_runtime/solver/lanczos_solver.cuh

cpp/include/raft_runtime/solver/lanczos.hpp

cpp/include/raft/sparse/solver/lanczos.cuh

cpp/include/raft_runtime/solver/lanczos.hpp

cpp/include/raft/sparse/solver/lanczos.cuh

python/pylibraft/pylibraft/solver/__init__.py

cpp/include/raft/sparse/solver/detail/lanczos.cuh

lowener · 2024-08-22T14:54:44Z

cpp/include/raft/sparse/solver/lanczos.cuh

+template <typename IndexTypeT, typename ValueTypeT>
+auto lanczos_compute_smallest_eigenvectors(
+  raft::resources const& handle,
+  raft::spectral::matrix::sparse_matrix_t<IndexTypeT, ValueTypeT> const& A,


Should the newer raft::device_csr_matrix API be used for this new public function?

Use the device_csr_matrix_view in the public API, not in the detail namespace

Suggested change

raft::spectral::matrix::sparse_matrix_t<IndexTypeT, ValueTypeT> const& A,

raft::device_csr_matrix_view<ValueTypeT, IndexTypeT, IndexTypeT, IndexTypeT> A,

cpp/test/sparse/solver/lanczos.cu

cpp/include/raft/linalg/detail/norm.cuh

lowener

Can a test for the python API of Lanczos also be added?

lowener · 2024-09-10T10:43:26Z

cpp/include/raft/sparse/solver/lanczos_types.hpp

+
+namespace raft::sparse::solver {
+
+template <typename IndexTypeT, typename ValueTypeT>


IndexTypeT template is unused in this structure so it should be removed.

lowener · 2024-09-25T13:06:05Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+}
+
+/**
+ *  @brief Find the smallest eigenpairs using lanczos solver


The docstring should be on the top-level function which will be called directly, not in the detail namespace

lowener · 2024-09-25T13:08:28Z

cpp/include/raft/sparse/solver/lanczos.cuh

+template <typename IndexTypeT, typename ValueTypeT>
+auto lanczos_compute_smallest_eigenvectors(
+  raft::resources const& handle,
+  raft::spectral::matrix::sparse_matrix_t<IndexTypeT, ValueTypeT> const& A,


Use the device_csr_matrix_view in the public API, not in the detail namespace

Suggested change

raft::spectral::matrix::sparse_matrix_t<IndexTypeT, ValueTypeT> const& A,

raft::device_csr_matrix_view<ValueTypeT, IndexTypeT, IndexTypeT, IndexTypeT> A,

cpp/include/raft/sparse/solver/lanczos.cuh

lowener · 2024-09-25T13:24:47Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

@@ -1396,4 +1438,658 @@ int computeLargestEigenvectors(
  return status;
 }

+template <typename T>
+RAFT_KERNEL kernel_subtract_and_scale(T* u, T* vec, T* scalar, int n)


Yes it is possible: https://github.com/rapidsai/raft/blob/branch-24.10/cpp/include/raft/matrix/detail/math.cuh#L129

lowener · 2024-09-25T13:48:22Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+    kernel_clamp_down_vector<<<numBlocks, blockSize, 0, stream>>>(
+      u.data_handle(), static_cast<ValueTypeT>(1e-7), n);
+
+    kernel_clamp_down<<<1, 1, 0, stream>>>(&beta(0, i), static_cast<ValueTypeT>(1e-6));


Don't dereference a device memory address on host side

lowener · 2024-09-25T13:53:56Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+    handle,
+    v0_vector_const,
+    V_0_view,
+    [device_scalar = v0nrm_scalar.data_handle()] __device__(auto y) { return y / *device_scalar; });


Can v0nrm and it's copy operations be skipped this way?

Suggested change

[device_scalar = v0nrm_scalar.data_handle()] __device__(auto y) { return y / *device_scalar; });

[device_scalar = output1.data_handle()] __device__(auto y) { return y / *device_scalar; });

lowener · 2024-09-25T13:55:47Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+
+  raft::device_vector<ValueTypeT, uint32_t> output1 =
+    raft::make_device_vector<ValueTypeT, uint32_t>(handle, 1);
+  raft::device_matrix_view<const ValueTypeT> input1 =


What's the difference with v0_view? Can't it be used here?

lowener · 2024-09-25T14:09:44Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+    raft::linalg::unary_op(handle,
+                           u_vector_const,
+                           V_0_view,
+                           [device_scalar = unrm_scalar.data_handle()] __device__(auto y) {


unrm can be skipped

Suggested change

[device_scalar = unrm_scalar.data_handle()] __device__(auto y) {

[device_scalar = output.data_handle()] __device__(auto y) {

lowener · 2024-09-25T14:42:08Z

cpp/include/raft/sparse/solver/detail/lanczos.cuh

+                       raft::sqrt_op());
+    raft::copy(&res, output2.data_handle(), 1, stream);
+
+    RAFT_LOG_TRACE("Iteration %f: residual (tolerance) %d", iter, res);


To use a copied value on host, the stream would need to be synchronized before. Since that sync can slow this function down it would be better to check the log level, and only copy and sync if necessary.

init

2c68742

github-actions bot added the cpp label Aug 20, 2024

benchmarking lanczos working

0f5bdcb

aamijar force-pushed the lanczos-solver-new branch from 7ec5f7f to 0f5bdcb Compare August 20, 2024 23:45

aamijar mentioned this pull request Aug 20, 2024

Lanczos solver #2410

Closed

aamijar self-assigned this Aug 21, 2024

aamijar added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 21, 2024

aamijar added 2 commits August 21, 2024 00:45

format style

5ee60cb

eigsh pylibraft api

fa222c1

github-actions bot added CMake python labels Aug 21, 2024

aamijar mentioned this pull request Aug 21, 2024

Lanczos Solver API rapidsai/cuml#6034

Closed

aamijar added 2 commits August 21, 2024 20:47

gtest

7240937

clean up code

a4cdc7a

aamijar marked this pull request as ready for review August 21, 2024 23:24

aamijar requested review from a team as code owners August 21, 2024 23:24

cjnolet requested changes Aug 22, 2024

View reviewed changes

aamijar added 2 commits August 22, 2024 02:13

update gtest rng seed

2be393a

update gtest edge case

4f37b5c

aamijar force-pushed the lanczos-solver-new branch from 81475a6 to 2be393a Compare August 22, 2024 02:15

update gtest clean

79ce3f1

lowener requested changes Aug 22, 2024

View reviewed changes

aamijar added 4 commits August 23, 2024 19:26

resolving pr comments

ceb1d7a

resolving pr comments

14b7266

resolving pr comments

c564352

resolving pr comments

aba9c4e

aamijar added 2 commits August 24, 2024 03:28

resolving pr comments

ba16aca

resolving pr comments

74908f2

aamijar mentioned this pull request Aug 24, 2024

[FEA] Add new lanczos solver to UMAP for spectral initialization rapidsai/cuml#6045

Open

aamijar force-pushed the lanczos-solver-new branch from 379de5a to bd26c79 Compare August 28, 2024 16:09

resolving pr comments

7b31108

aamijar force-pushed the lanczos-solver-new branch from bd26c79 to 7b31108 Compare August 28, 2024 16:13

aamijar and others added 3 commits August 28, 2024 17:02

resolving pr comments

2cdcc66

Merge branch 'branch-24.10' into lanczos-solver-new

5aa1df2

Merge branch 'branch-24.10' into lanczos-solver-new

e4afa2d

cjnolet reviewed Sep 6, 2024

View reviewed changes

cpp/include/raft/linalg/detail/norm.cuh Outdated Show resolved Hide resolved

aamijar added 6 commits September 9, 2024 04:13

resolving pr comments

1160722

resolving pr comments

e473728

resolving pr comments

cc22a39

resolving pr comments

8767c6a

resolving pr comments

a3809eb

resolving pr comments

d4b4955

aamijar requested a review from lowener September 9, 2024 18:21

lowener requested changes Sep 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lanczos Solver #2416

Lanczos Solver #2416

aamijar commented Aug 20, 2024 •

edited

Loading

copy-pr-bot bot commented Aug 20, 2024

cjnolet left a comment

lowener Aug 22, 2024

lowener Sep 25, 2024

lowener left a comment

lowener Sep 10, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

lowener Sep 25, 2024

	raft::spectral::matrix::sparse_matrix_t<IndexTypeT, ValueTypeT> const& A,
	raft::device_csr_matrix_view<ValueTypeT, IndexTypeT, IndexTypeT, IndexTypeT> A,


		namespace raft::sparse::solver {

		template <typename IndexTypeT, typename ValueTypeT>

	[device_scalar = v0nrm_scalar.data_handle()] __device__(auto y) { return y / *device_scalar; });
	[device_scalar = output1.data_handle()] __device__(auto y) { return y / *device_scalar; });

	[device_scalar = unrm_scalar.data_handle()] __device__(auto y) {
	[device_scalar = output.data_handle()] __device__(auto y) {

Lanczos Solver #2416

Are you sure you want to change the base?

Lanczos Solver #2416

Conversation

aamijar commented Aug 20, 2024 • edited Loading

Lanczos Solver for Sparse Eigen Decomposition

copy-pr-bot bot commented Aug 20, 2024

cjnolet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowener left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aamijar commented Aug 20, 2024 •

edited

Loading