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

Add a uniform coarsening algorithm for coarse grid generation. #979

Closed
wants to merge 10 commits into from

Conversation

pratikvn
Copy link
Member

@pratikvn pratikvn commented Mar 4, 2022

This PR adds a new class to generate coarse matrices which can be useful for multigrid and other multi-level methods in general.

Two main methods to generate coarse matrices are provided:

  1. A naive one with user providing a constant jump between the rows.
  2. A user specified coarse row array. (Separate class and separate PR)

TODO:

Due to missing device kernels for creation of submatrix with IndexSet, the creation of the submatrix from the index set will be in a separate PR.

@pratikvn pratikvn added is:new-feature A request or implementation of a feature that does not exist yet. 1:ST:WIP This PR is a work in progress. Not ready for review. type:multigrid This is related to multigrid labels Mar 4, 2022
@pratikvn pratikvn self-assigned this Mar 4, 2022
@ginkgo-bot ginkgo-bot added mod:all This touches all Ginkgo modules. reg:build This is related to the build system. reg:example This is related to the examples. reg:testing This is related to testing. type:matrix-format This is related to the Matrix formats labels Mar 4, 2022
@upsj
Copy link
Member

upsj commented Mar 4, 2022

I mentioned this in the meeting before: When I hear the description of this functionality, I think "uniform". In my view, selection usually refers to finding a single element, or a small number of elements. The operations you are doing is a coarsening, so how about UniformCoarsening? This can probably also be used to implement a simple 1D Geometric Multigrid, right?

@pratikvn
Copy link
Member Author

pratikvn commented Mar 4, 2022

Naming it Coarsening is a good idea. How about SimpleCoarsening ? Because you have the ability to specify an array for the coarsening indices, Uniform might not be the best name.

Yes, this should allow us to implement a simple 1D Geometric Multigrid.

@upsj
Copy link
Member

upsj commented Mar 4, 2022

These two might be different enough that we could put them into different types? FixedCoarsening and UniformCoarsening?
Do we have any generic strategies for describing or picking the averaging operation? That could be just picking one of the fine nodes, (un-)weighted average, ...

@pratikvn
Copy link
Member Author

pratikvn commented Mar 4, 2022

Yes, we could technically put them in two separate classes. I guess at some point we will have a lot of separate classes. But I guess that might be okay if they have different algorithms.

Currently, this is just picking some nodes, but we could technically have different averaging strategies. This could be as simple as having a sum-normalized per row for the restriction and prolongation matrices to more involved averaging strategies based on diagonal and off-diagonal fine matrix element weights

@upsj
Copy link
Member

upsj commented Mar 4, 2022

I was thinking that our AMGX implementation does two things: a heavy edge matching to compute the fine-to-coarse mapping, and a uniform averaging scheme. Maybe it might make sense to look at what hypre is doing here? I think they have some averaging schemes specific to certain coarsening algorithms, but also generic ones? Anyways, this is only long-term thinking, not directly related to this PR, I am fine with the approach :)

@pratikvn
Copy link
Member Author

pratikvn commented Mar 4, 2022

Yes, I think generation of the restriction and prolongation matrices can be de-coupled from the coarsening strategy itself in some cases. I don't have a clear solution for that now (interface wise), but it might be something we should look at in the future.

@yhmtsai
Copy link
Member

yhmtsai commented Mar 4, 2022

the multigridLevel is the general thing. I do not put it as the coarsening method and the interpolator methods directly. The coarsening method might generate different information such that not all interpolator can rely on it. The MultigridLevel Factory can has option for intepolator but it should be decided by the coarsening method.
For example, Agg method might give the aggregation group in some sense (the agg group def might be different in different alg), the coarsening method gives the C-F. the information is not direct. Leave the intepolator decided by the the coarsening method to give full flexibilty for it.
The interpolator might not fit in the LinOpFactory directly. in Pgm, the intepolator uses the agg information and the size only.

@pratikvn pratikvn force-pushed the multigrid-selection branch from 3cf7648 to b77685c Compare March 4, 2022 20:42
@codecov
Copy link

codecov bot commented Mar 5, 2022

Codecov Report

Merging #979 (0e30d47) into develop (0622251) will increase coverage by 0.51%.
The diff coverage is 97.47%.

❗ Current head 0e30d47 differs from pull request most recent head c2ac3f8. Consider uploading reports for the commit c2ac3f8 to get more accurate results

@@             Coverage Diff             @@
##           develop     #979      +/-   ##
===========================================
+ Coverage    91.77%   92.28%   +0.51%     
===========================================
  Files          499      486      -13     
  Lines        42972    40358    -2614     
===========================================
- Hits         39439    37246    -2193     
+ Misses        3533     3112     -421     
Impacted Files Coverage Δ
core/device_hooks/common_kernels.inc.cpp 0.00% <0.00%> (ø)
include/ginkgo/core/matrix/csr.hpp 45.53% <ø> (+2.17%) ⬆️
omp/base/index_set_kernels.cpp 94.11% <ø> (ø)
reference/base/index_set_kernels.cpp 94.11% <83.33%> (-0.09%) ⬇️
core/base/index_set.cpp 96.29% <92.30%> (-1.44%) ⬇️
...ence/test/multigrid/uniform_coarsening_kernels.cpp 96.03% <96.03%> (ø)
...n/unified/multigrid/uniform_coarsening_kernels.cpp 100.00% <100.00%> (ø)
core/matrix/csr.cpp 94.86% <100.00%> (+1.45%) ⬆️
core/multigrid/amgx_pgm.cpp 100.00% <100.00%> (ø)
core/multigrid/uniform_coarsening.cpp 100.00% <100.00%> (ø)
... and 182 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb224a5...c2ac3f8. Read the comment docs.

@pratikvn pratikvn changed the title Add a selection algorithm for coarse grid generation. Add a uniform coarsening algorithm for coarse grid generation. Mar 8, 2022
@pratikvn pratikvn force-pushed the multigrid-selection branch 2 times, most recently from c81f200 to 0e30d47 Compare March 10, 2022 13:03
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 83 Code Smells

70.8% 70.8% Coverage
14.6% 14.6% Duplication

@pratikvn pratikvn added 1:ST:ready-for-review This PR is ready for review 1:ST:WIP This PR is a work in progress. Not ready for review. 1:ST:do-not-merge Please do not merge PR this yet. and removed 1:ST:WIP This PR is a work in progress. Not ready for review. 1:ST:ready-for-review This PR is ready for review labels Mar 11, 2022

auto prolong_op = gko::as<csr_type>(share(restrict_op->transpose()));

// TODO: Can be done with submatrix index_set.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's true in the general case - the operation you need is more akin to a row_gather with arbitrary permutation than submatrix. That might also relate to your TODO on ordering?

@pratikvn pratikvn force-pushed the multigrid-selection branch from 0e30d47 to 911740a Compare June 21, 2022 07:18
@ginkgo-bot
Copy link
Member

Error: The following files need to be formatted:

test/multigrid/uniform_coarsening_kernels.cpp

You can find a formatting patch under Artifacts here or run format! if you have write access to Ginkgo

@ginkgo-bot
Copy link
Member

Note: This PR changes the Ginkgo ABI:

Functions changes summary: 0 Removed, 0 Changed, 968 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

For details check the full ABI diff under Artifacts here

@pratikvn
Copy link
Member Author

Closing stale PR

@pratikvn pratikvn closed this Jan 12, 2024
@pratikvn pratikvn deleted the multigrid-selection branch January 12, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1:ST:do-not-merge Please do not merge PR this yet. 1:ST:WIP This PR is a work in progress. Not ready for review. is:new-feature A request or implementation of a feature that does not exist yet. mod:all This touches all Ginkgo modules. reg:build This is related to the build system. reg:example This is related to the examples. reg:testing This is related to testing. type:matrix-format This is related to the Matrix formats type:multigrid This is related to multigrid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants