Skip to content

Add unified RS-coarsening kernels - #2034

Open
gojakuch wants to merge 5 commits into
developfrom
feat/rs-coarsening-kernels
Open

Add unified RS-coarsening kernels#2034
gojakuch wants to merge 5 commits into
developfrom
feat/rs-coarsening-kernels

Conversation

@gojakuch

Copy link
Copy Markdown
Collaborator

No description provided.

@ginkgo-bot ginkgo-bot added reg:build This is related to the build system. reg:testing This is related to testing. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:reference This is related to the reference module. mod:hip This is related to the HIP module. type:multigrid This is related to multigrid labels Jun 15, 2026
Base automatically changed from feat/rs-coarsening to develop June 15, 2026 21:31
Comment thread common/unified/multigrid/rs_kernels.cpp Outdated
exec, [] GKO_KERNEL(auto i, auto cf) { cf[i] = 0; },
cf_marker.get_size(), cf);

/// 4. RS-COARSENING (inherently sequential algorithm?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is lambda_vals and cf shared with all runner?
The unified kernel won't be able to control the visibility of memory, so the other threads may not see the updated value at all.
If it is the sequential part, you can still use the host to solve it

@gojakuch
gojakuch force-pushed the feat/rs-coarsening-kernels branch from f568022 to cc9e07b Compare June 17, 2026 08:48
@gojakuch
gojakuch marked this pull request as ready for review June 23, 2026 08:08
@gojakuch
gojakuch requested review from pratikvn and yhmtsai June 23, 2026 08:08

@pratikvn pratikvn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice work! Mostly minor comments

Comment thread common/unified/multigrid/rs_kernels.cpp Outdated
}

// Copy results back to device
lambda = array<IndexType>(exec, h_lambda);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lambda is unused.

Comment thread common/unified/multigrid/rs_kernels.cpp Outdated
Comment on lines +134 to +135
array<IndexType> h_row_ptrs(host_exec, a_row_ptrs, a_row_ptrs + n + 1);
array<IndexType> h_col_idxs(host_exec, a_col_idxs, a_col_idxs + nnz);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe check if host_exec == exec ? For OMP this is the case, and there is an unnecessary copy

Comment on lines +36 to 66
run_kernel_reduction(
exec,
[] GKO_KERNEL(auto row, auto row_ptrs, auto col_idxs, auto values) {
bool has_diag = false;
bool valid = true;
for (auto nz = row_ptrs[row]; nz < row_ptrs[row + 1]; ++nz) {
const auto col = col_idxs[nz];
const auto val = real(values[nz]);
using real_t = decltype(val);

if (row == col) {
has_diag = true;
if (val <= zero<real_t>()) valid = false;
} else {
if (val > zero<real_t>()) valid = false;
}
}
return (valid && has_diag) ? IndexType{1} : IndexType{0};
},
GKO_KERNEL_REDUCE_SUM(IndexType), d_result.get_data(), num_rows,
row_ptrs, col_idxs, values);

const IndexType count = get_element(d_result, 0);

run_kernel(
exec,
[count, num_rows] GKO_KERNEL(auto tidx, auto is_m_matrix) {
is_m_matrix[0] = (count == static_cast<IndexType>(num_rows));
},
1, is_m_matrix_array.get_data());
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be simplified ?

run_kernel_reduction(
      exec,
      [] GKO_KERNEL(auto row, auto row_ptrs, auto col_idxs, auto values) {
          bool has_diag = false, valid = true;
          for (auto nz = row_ptrs[row]; nz < row_ptrs[row + 1]; ++nz) { // same  }
          return valid && has_diag;          // bool per row
      },
      [] GKO_KERNEL(auto a, auto b) { return a && b; },   // AND
      [] GKO_KERNEL(auto a) { return a; },
      true, is_m_matrix_array.get_data(), num_rows,
      row_ptrs, col_idxs, values);

Comment thread test/multigrid/rs_kernels.cpp Outdated
Comment on lines +14 to +16
#include <ginkgo/core/matrix/diagonal.hpp>
#include <ginkgo/core/matrix/row_gatherer.hpp>
#include <ginkgo/core/matrix/sparsity_csr.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

unnecessary includes. Also check for others

@gojakuch
gojakuch force-pushed the feat/rs-coarsening-kernels branch 2 times, most recently from eda193f to 997ed1b Compare July 10, 2026 20:25
@gojakuch
gojakuch requested a review from pratikvn July 20, 2026 12:05
@gojakuch
gojakuch force-pushed the feat/rs-coarsening-kernels branch from 997ed1b to 9e2e4e2 Compare August 3, 2026 14:21
@gojakuch

gojakuch commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:hip This is related to the HIP module. mod:reference This is related to the reference module. reg:build This is related to the build system. reg:testing This is related to testing. type:multigrid This is related to multigrid

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants