Skip to content

RS-coarsening reference implementation#1985

Draft
gojakuch wants to merge 7 commits intodevelopfrom
feat/rs-coarsening
Draft

RS-coarsening reference implementation#1985
gojakuch wants to merge 7 commits intodevelopfrom
feat/rs-coarsening

Conversation

@gojakuch
Copy link
Copy Markdown
Collaborator

@gojakuch gojakuch commented Feb 24, 2026

This PR provides the reference implementation for RS-coarsening

@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 Feb 24, 2026
@upsj
Copy link
Copy Markdown
Member

upsj commented Mar 4, 2026

FYI, #1978 will likely require some changes in your kernel function signatures to match the changes there

@gojakuch gojakuch force-pushed the feat/rs-coarsening branch from ab79a98 to e5e08d3 Compare March 9, 2026 09:55
@ginkgo-project ginkgo-project deleted a comment from ginkgo-bot Mar 17, 2026
@gojakuch gojakuch force-pushed the feat/rs-coarsening branch from 33ac013 to 5010a34 Compare March 17, 2026 12:21
@gojakuch
Copy link
Copy Markdown
Collaborator Author

now that #1978 is merged, I know I need to adapt the kernels. I'll do that. for now, committing what I've got so far and rebasing

@gojakuch gojakuch changed the title RS-coarsening RS-coarsening reference implementation Mar 17, 2026
Copy link
Copy Markdown
Member

@pratikvn pratikvn left a comment

Choose a reason for hiding this comment

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

Nice work! For the most part, I think the sturucture looks good! I think some kernels can be merged.

Comment on lines +86 to +89
exec->run(rs::make_init_cf(cf_marker));
exec->run(rs::make_rs_coarsening(rs_op, is_strong.get_const_data(),
lambda.get_data(), cf_marker));
exec->run(rs::make_rs_cleanup(cf_marker));
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.

A general rule: If you are calling kernels consecutively, they can probably be merged into a single kernel.

As much as possible, you want to have as few kernel launches as possible.

Comment on lines +97 to +101
exec->run(rs::make_fill_coarse_rows(cf_marker, coarse_rows.get_data()));

array<IndexType> fine_to_coarse(exec, fine_dim);
exec->run(
rs::make_fill_fine_to_coarse(cf_marker, fine_to_coarse.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.

Same here. fine_dim is already known before-hand, so you can allocate the array before, and combine these kernels.

rs_op = rs_op_shared_ptr.get();
this->set_fine_op(rs_op_shared_ptr);
}

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.

I think it might be reasonable to add a validation check here that the matrix is an M-matrix, and if not throw. At a later point, we can hide it under a flag, or move it to the data_validation compoenent of the matrix.

Comment on lines +29 to +32
// * A =
// * [ 2 -1 0 ]
// * [ -1 2 -1 ]
// * [ 0 -1 2 ]
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.

I think this is a bit trivial. Can you choose a slightly larger matrix ? Maybe like 5x5 ?

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

Labels

1:ST:run-full-test 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