Add a uniform coarse generation algorithm #1526

pratikvn · 2024-01-13T01:06:28Z

This PR is an updated version of #979 . The idea is to have a very cheap coarse grid generation algorithm that works well for matrices that are uniform, for example, matrices that arise from stencils on structured grids.

For example, for a 7 point stencil on a 3D grid, it is very competitive to PGM (with_deterministic=false) with much faster generation and faster apply for larger problems (on OMP executor with 8 cores):

sonarqubecloud · 2024-01-13T05:59:10Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

44 New issues
1 Security Hotspot
66.4% Coverage on New Code
2.0% Duplication on New Code

See analysis details on SonarCloud

codecov · 2024-01-13T06:04:31Z

Codecov Report

Attention: Patch coverage is 97.64310% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 89.78%. Comparing base (fb70ec0) to head (0537b1e).
Report is 232 commits behind head on develop.

❗ Current head 0537b1e differs from pull request most recent head 915e227. Consider uploading reports for the commit 915e227 to get more accurate results

Files	Patch %	Lines
...ence/test/multigrid/uniform_coarsening_kernels.cpp	96.32%	5 Missing ⚠️
core/device_hooks/common_kernels.inc.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1526      +/-   ##
===========================================
- Coverage    91.04%   89.78%   -1.27%     
===========================================
  Files          700      707       +7     
  Lines        56996    57487     +491     
===========================================
- Hits         51894    51612     -282     
- Misses        5102     5875     +773

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

upsj · 2024-01-19T09:43:08Z

Especially focusing on distributed, I think if we include this, we should try to support at least 2D and maybe 3D coarsening (i + n * j and i + n * j + k * n * m).

MarcelKoch · 2024-01-19T15:21:24Z

Maybe it makes sense to split the UniformCoarsening class up. The generated object essentially stores an array with coarse row indices, so it seems natural to allow users to pass in these indices. But I don't think we can add a parameter to the existing class, since then there would be two mutually exclusive parameters.
I think Tobias' approach for the direct solvers, i.e. having multiple factories that generate the same class makes sense here.

MarcelKoch

As I mentioned, I think the UniformCoarsening class should be split up. Other than that I have mostly smaller comments.

MarcelKoch · 2024-01-19T15:34:13Z

core/multigrid/uniform_coarsening.cpp

+        parameters_.coarse_skip, &coarse_rows_));
+
+    gko::dim<2>::dimension_type coarse_dim =
+        (coarse_rows_.get_size() + parameters_.coarse_skip - 1) /


MarcelKoch · 2024-01-19T15:48:30Z

examples/multigrid-preconditioned-solver/multigrid-preconditioned-solver.cpp

+                                        .with_criteria(iter_stop, tol_stop)
+                                        .on(exec));
+    if (coarse_type == std::string("uniform")) {
+        std::cout << "Using Uniform coarsening" << std::endl;


I don't think the output is necessary, since that information is already part of the program arguments.

MarcelKoch · 2024-01-19T15:49:28Z

examples/multigrid-preconditioned-solver/multigrid-preconditioned-solver.cpp

+                                        .on(exec));
+    if (coarse_type == std::string("uniform")) {
+        std::cout << "Using Uniform coarsening" << std::endl;
+        multigrid_gen = gko::share(mg::build()


it shouldn't generate the MG twice.

This just builds the factories, no actual generation is done.

doesn't really matter, it's unnecessary duplication.

MarcelKoch · 2024-01-19T15:51:16Z

examples/multigrid-preconditioned-solver/multigrid-preconditioned-solver.cpp

+    tol_stop->add_logger(logger);
+
+    // Create smoother factory (ir with bj)
+    auto inner_solver_gen =


is it necessary to introduce these intermediate factories again. I refactored this example to be as minimal as possible.

MarcelKoch · 2024-01-19T15:54:09Z

reference/multigrid/uniform_coarsening_kernels.cpp

+namespace kernels {
+namespace reference {
+/**
+ * @brief The UNIFORM_COARSENING solver namespace.


I don't think these comments are useful.

MarcelKoch · 2024-01-19T15:58:55Z

reference/test/multigrid/uniform_coarsening_kernels.cpp

+        }
+    }
+
+    static void assert_same_coarse_rows(const index_type* m1,


Same but for array.

MarcelKoch · 2024-01-19T15:59:37Z

reference/test/multigrid/uniform_coarsening_kernels.cpp

+                 PairTypenameNameGenerator);
+
+
+TYPED_TEST(UniformCoarsening, CanBeCopied)


Aren't copy/move/clone/clear generally core tests?

MarcelKoch · 2024-01-19T16:00:05Z

reference/test/multigrid/uniform_coarsening_kernels.cpp

+TYPED_TEST(UniformCoarsening, CanBeCopied)
+{
+    using Mtx = typename TestFixture::Mtx;
+    using MgLevel = typename TestFixture::MgLevel;


unused (and in the other tests)

MarcelKoch · 2024-01-19T16:04:52Z

include/ginkgo/core/multigrid/uniform_coarsening.hpp

+ * @ingroup LinOp
+ */
+template <typename ValueType = default_precision, typename IndexType = int32>
+class UniformCoarsening


Should this define copy/move assignment/constructors? The default ones are not completely inline with our normal understanding of those.

MarcelKoch · 2024-01-19T16:06:25Z

reference/test/multigrid/uniform_coarsening_kernels.cpp

+    gko::kernels::reference::uniform_coarsening::fill_incremental_indices(
+        this->exec, 2, &c_rows);
+    GKO_ASSERT_ARRAY_EQ(c_rows, c2_rows);


range-based for loop?

sonarqubecloud · 2024-04-27T08:36:14Z

Quality Gate passed

Issues
44 New issues
0 Accepted issues

Measures
1 Security Hotspot
66.4% Coverage on New Code
2.0% Duplication on New Code

See analysis details on SonarCloud

pratikvn · 2024-07-04T12:00:43Z

@upsj, After a bit of thought, I think this class should not represent Geometric/Structured coarsening. Uniform coarsening is just a cheap coarsening method that just uniformly discards rows and columns to construct the coarse matrix, and hence is not suitable for Geometric coarsening, which needs to create connections between the coarse nodes.

@MarcelKoch , we already provide the users with a way to specify custom indices for coarsening with the FixedCoarsening class

upsj · 2024-07-04T12:04:47Z

@pratikvn If we discard variables for restriction, we still need to interpolate new values for prolongation, so I don't yet see how this makes multi-dimensional coarsening impossible if 1D coarsening is possible?

pratikvn · 2024-07-04T12:10:32Z

In a similar fashion to PGM, the prolongation matrix will be a transpose of the restriction matrix. I think maybe I have given a wrong impression that this would be a 1D coarsening. The fine level vectors will not have extrapolated values, but just "mapped" values.

upsj · 2024-07-04T12:12:44Z

Doesn't PGM do the same thing? And I believe we are mixing two things here - coarsening and interpolation. One is structural, one concerns the actual numerical values.

pratikvn · 2024-07-17T14:30:26Z

Just adding more performance results on a 7pt stencil and some chosen matrices from Suitesparse, to show that this type of coarsening can help in certain cases.

Apply speedup for cage15 (v/s PGM) matrix and bcsstm39 matrix (v/s no preconditioning)

speedup_app_ss_bicgstab_cskip.pdf

Apply speedup for 7pt stencil (v/s PGM)

speedup_app_7pt_cg_cskip.pdf

Generate speedup for 7pt stencil (v/s PGM)

speedup_gen_7pt_cg_cskip.pdf

upsj · 2024-07-17T15:24:08Z

I don't think these results matter, since even for simple stencils, the matrix loses all connectivity:

Take the poisson-solver example and plug in the following code:

    auto gen = gko::multigrid::UniformCoarsening<ValueType, IndexType>::build().on(app_exec)->generate(matrix->clone());
    gko::write(std::cout, matrix);
    gko::write(std::cout, gko::as<mtx>(gen->get_composition()->get_operators()[1]));

The fine matrix is tridiagonal, but the coarse matrix is just a diagonal matrix.

upsj · 2024-10-18T12:50:17Z

I don't see this PR being useful in its current state. IMO we should either turn it into a regular coarsening algorithm, or abandon it.

pratikvn added is:new-feature A request or implementation of a feature that does not exist yet. 1:ST:ready-for-review This PR is ready for review type:multigrid This is related to multigrid labels Jan 13, 2024

pratikvn requested review from a team January 13, 2024 01:06

pratikvn self-assigned this Jan 13, 2024

pratikvn force-pushed the uniform-coarsening branch from 0537b1e to 2db6399 Compare January 19, 2024 09:13

MarcelKoch self-requested a review January 19, 2024 15:13

MarcelKoch requested changes Jan 19, 2024

View reviewed changes

upsj requested review from upsj and greole February 7, 2024 09:46

MarcelKoch added this to the Ginkgo 1.8.0 milestone Apr 5, 2024

pratikvn force-pushed the uniform-coarsening branch from 903a8a5 to 915e227 Compare April 26, 2024 13:01

pratikvn removed this from the Ginkgo 1.8.0 milestone May 3, 2024

tcojean added this to the Ginkgo 1.9.0 milestone May 3, 2024

pratikvn force-pushed the uniform-coarsening branch from 915e227 to 2f5525a Compare June 25, 2024 11:38

pratikvn removed the 1:ST:ready-for-review This PR is ready for review label Jun 26, 2024

pratikvn added the 1:ST:WIP This PR is a work in progress. Not ready for review. label Jun 26, 2024

pratikvn force-pushed the uniform-coarsening branch 2 times, most recently from c6939ee to e77eef8 Compare July 4, 2024 10:44

pratikvn added 10 commits July 4, 2024 13:35

Add uniform coarsening class and core

54d5463

Add core tests

c12c0b3

Add reference kernels and tests

19aa73b

Add device common kernels and test

44e48f0

Update multigrid-precond example

9da0014

Rename to coarse_skip

1e9c452

minor doc and code fixes

8f4bd38

format files

b43431f

use GKO_DEVICE_NAMESPACE

4c21ff2

review updates

185f551

pratikvn force-pushed the uniform-coarsening branch from e77eef8 to 185f551 Compare July 4, 2024 11:55

MarcelKoch modified the milestones: Ginkgo 1.9.0, Ginkgo 1.10.0 Dec 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a uniform coarse generation algorithm #1526

Add a uniform coarse generation algorithm #1526

pratikvn commented Jan 13, 2024 •

edited

Loading

sonarqubecloud bot commented Jan 13, 2024

codecov bot commented Jan 13, 2024 •

edited

Loading

upsj commented Jan 19, 2024 •

edited

Loading

MarcelKoch commented Jan 19, 2024

MarcelKoch left a comment

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

pratikvn Jan 24, 2024

MarcelKoch Jan 24, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

MarcelKoch Jan 19, 2024

sonarqubecloud bot commented Apr 27, 2024

pratikvn commented Jul 4, 2024

upsj commented Jul 4, 2024

pratikvn commented Jul 4, 2024

upsj commented Jul 4, 2024

pratikvn commented Jul 17, 2024

upsj commented Jul 17, 2024

upsj commented Oct 18, 2024

		PairTypenameNameGenerator);


		TYPED_TEST(UniformCoarsening, CanBeCopied)

Add a uniform coarse generation algorithm #1526

Are you sure you want to change the base?

Add a uniform coarse generation algorithm #1526

Conversation

pratikvn commented Jan 13, 2024 • edited Loading

sonarqubecloud bot commented Jan 13, 2024

Quality Gate passed

codecov bot commented Jan 13, 2024 • edited Loading

Codecov Report

upsj commented Jan 19, 2024 • edited Loading

MarcelKoch commented Jan 19, 2024

MarcelKoch 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 27, 2024

Quality Gate passed

pratikvn commented Jul 4, 2024

upsj commented Jul 4, 2024

pratikvn commented Jul 4, 2024

upsj commented Jul 4, 2024

pratikvn commented Jul 17, 2024

Apply speedup for cage15 (v/s PGM) matrix and bcsstm39 matrix (v/s no preconditioning)

Apply speedup for 7pt stencil (v/s PGM)

Generate speedup for 7pt stencil (v/s PGM)

upsj commented Jul 17, 2024

upsj commented Oct 18, 2024

pratikvn commented Jan 13, 2024 •

edited

Loading

codecov bot commented Jan 13, 2024 •

edited

Loading

upsj commented Jan 19, 2024 •

edited

Loading