Substitute COO_Mumps_Solver in ExtrapolatedSmoother by julianlitz · Pull Request #206 · SciCompMod/GMGPolar

julianlitz · 2026-04-03T21:29:55Z

Merge Request - GuideLine Checklist

Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.

Checks by code author:

Always to be checked:

There is at least one issue associated with the pull request.
New code adheres with the coding guidelines
No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

If functions were changed or functionality was added:

Tests for new functionality has been added
A local test was succesful

If new functionality was added:

There is appropriate documentation of your work. (use doxygen style comments)

If new third party software is used:

Did you pay attention to its license? Please remember to add it to the wiki after successful merging.

If new mathematical methods or epidemiological terms are used:

Are new methods referenced? Did you provide further documentation?

Checks by code reviewer(s):

Is the code clean of development artifacts e.g., unnecessary comments, prints, ...
The ticket goals for each associated issue are reached or problems are clearly addressed (i.e., a new issue was introduced).
There are appropriate unit tests and they pass.
The git history is clean and linearized for the merge request. All reviewers should squash commits and write a simple and meaningful commit message.
Coverage report for new code is acceptable.
No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

julianlitz · 2026-04-03T21:47:44Z

@EmilyBourne

Improve Readability of Give parallelization
Remove unnecessary Stencils in DirectSolverCSR
Use COO Mumps Solver class in Extrapolated Smoother
Improve the 'symmetric matrix' comment for the interior boundary matrix.

codecov · 2026-04-03T21:56:58Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.64%. Comparing base (ae2044c) to head (ce1ab16).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
- Coverage   90.69%   90.64%   -0.06%     
==========================================
  Files          86       85       -1     
  Lines        9433     9295     -138     
==========================================
- Hits         8555     8425     -130     
+ Misses        878      870       -8

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

EmilyBourne

Leaving the first part of my review as I see the same patterns repeating so it will be easier to finish the review when I have the reply to the first set of questions

EmilyBourne · 2026-04-21T09:54:35Z

+    if (additional_radial_tasks > 0) {
+        const int i_theta = 0;
+        buildSolverMatrixRadialSection(i_theta, solver_matrix);
+    }
+
+    if (additional_radial_tasks > 1) {
+        const int i_theta = 1;
+        buildSolverMatrixRadialSection(i_theta, solver_matrix);
+    }

+#pragma omp parallel num_threads(num_omp_threads)
+    {
 #pragma omp for
-            for (int radial_task = 0; radial_task < num_radial_tasks; radial_task += 3) {
-                if (radial_task > 0) {
-                    int i_theta = radial_task + additional_radial_tasks;
-                    buildSolverMatrixRadialSection(i_theta, solver_matrix);
-                }
-                else {
-                    if (additional_radial_tasks == 0) {
-                        buildSolverMatrixRadialSection(0, solver_matrix);
-                    }
-                    else if (additional_radial_tasks >= 1) {
-                        buildSolverMatrixRadialSection(0, solver_matrix);
-                        buildSolverMatrixRadialSection(1, solver_matrix);
-                    }
-                }
-            }
+        for (int radial_task = 0; radial_task < num_radial_tasks; radial_task += 3) {
+            const int i_theta = radial_task + additional_radial_tasks;
+            buildSolverMatrixRadialSection(i_theta, solver_matrix);
+        } /* Implicit barrier */
 #pragma omp for
-            for (int radial_task = 1; radial_task < num_radial_tasks; radial_task += 3) {
-                if (radial_task > 1) {
-                    int i_theta = radial_task + additional_radial_tasks;
-                    buildSolverMatrixRadialSection(i_theta, solver_matrix);
-                }
-                else {
-                    if (additional_radial_tasks == 0) {
-                        buildSolverMatrixRadialSection(1, solver_matrix);
-                    }
-                    else if (additional_radial_tasks == 1) {
-                        buildSolverMatrixRadialSection(2, solver_matrix);
-                    }
-                    else if (additional_radial_tasks == 2) {
-                        buildSolverMatrixRadialSection(2, solver_matrix);
-                        buildSolverMatrixRadialSection(3, solver_matrix);
-                    }
-                }
-            }


This doesn't quite line up. Before you had:

if (additional_radial_tasks == 0) { buildSolverMatrixRadialSection(1, solver_matrix); } else if (additional_radial_tasks == 1) { buildSolverMatrixRadialSection(2, solver_matrix); } else if (additional_radial_tasks == 2) { buildSolverMatrixRadialSection(2, solver_matrix); buildSolverMatrixRadialSection(3, solver_matrix); } for (int radial_task = 4; radial_task < num_radial_tasks; radial_task += 3) { int i_theta = radial_task + additional_radial_tasks; buildSolverMatrixRadialSection(i_theta, solver_matrix); }

but now you have:

if (additional_radial_tasks > 0) { const int i_theta = 0; buildSolverMatrixRadialSection(i_theta, solver_matrix); } if (additional_radial_tasks > 1) { const int i_theta = 1; buildSolverMatrixRadialSection(i_theta, solver_matrix); } for (int radial_task = 0; radial_task < num_radial_tasks; radial_task += 3) { const int i_theta = radial_task + additional_radial_tasks; buildSolverMatrixRadialSection(i_theta, solver_matrix); } /* Implicit barrier */

So for additional_radial_tasks == 0 you have i_theta = {0, 3, ...} whereas before you had i_theta = {1, 4, ...}

Example: additional tasks = 2
Keep a separation of three lines to avoid race conditions (note periodicity)

Before:

Step 1: {{0,1},{5},{8},...

BARRIER

Step 2: {{2,3},{6},{9},...

BARRIER

Step 3: {{4},{7},{10},...

Now:

additional tasks = 2

Step 1: {0}

BARRIER

Step 2: {1}

BARRIER

Step 1: {{2},{5},{8},...

BARRIER

Step 2: {{3},{6},{9},...

BARRIER

Step 3: {{4},{7},{10},...

@EmilyBourne

I think the new version is much nicer and easier to port to Kokkos.

EmilyBourne · 2026-04-21T09:56:46Z

-    if ((i_r > 1 && i_r < grid.nr() - 2) || (i_r == 1 && !DirBC_Interior)) {
+    if ((i_r > 0 && i_r < grid.nr() - 1)) {


Have you changed the indexing to be 0-based instead of 1-based? This would explain my previous question, except that here you are working on i_r not i_theta.

Why do you no longer need the check for DirBC_Interior?

Because in the Direct CSR Solver there is no symmetry shift. The stencil of next_to_boundary is the full 9 stencil.
Thus we dont need these. They are only necessary when we construct the matrix symmetric.

Currently DirectSolver Mumps constructs mazrices symmetric while DirectSolver CSR doesnt construct it symmetric.

julianlitz · 2026-04-24T21:09:10Z

@EmilyBourne We could merge #230 into this branch before merging it into main.

julianlitz added 2 commits April 3, 2026 23:26

Code improvements

a39112f

Code improvements

2a72cee

julianlitz requested a review from EmilyBourne April 3, 2026 21:47

julianlitz and others added 6 commits April 3, 2026 23:57

Use std::fabs

2e9f71d

Update buildInnerBoundaryAsc.inl

2efa57f

Update comment

fa3c7cd

Resolve operator precedence bug

90d3374

Resolve operator precedence bug

bee2de9

Merge remote-tracking branch 'origin/main' into litz_mjoelnir

ce1ab16

julianlitz mentioned this pull request Apr 20, 2026

Avoid Raw Pointers in CSR Solver #216

Open

14 tasks

EmilyBourne reviewed Apr 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Substitute COO_Mumps_Solver in ExtrapolatedSmoother#206

Substitute COO_Mumps_Solver in ExtrapolatedSmoother#206
julianlitz wants to merge 8 commits intomainfrom
litz_mjoelnir

julianlitz commented Apr 3, 2026 •

edited

Loading

Uh oh!

julianlitz commented Apr 3, 2026

Uh oh!

codecov Bot commented Apr 3, 2026 •

edited

Loading

Uh oh!

EmilyBourne left a comment

Uh oh!

Uh oh!

Uh oh!

EmilyBourne Apr 21, 2026

Uh oh!

julianlitz Apr 21, 2026 •

edited

Loading

Uh oh!

EmilyBourne Apr 21, 2026

Uh oh!

julianlitz Apr 21, 2026

Uh oh!

julianlitz Apr 21, 2026

Uh oh!

julianlitz Apr 21, 2026

Uh oh!

Uh oh!

julianlitz commented Apr 24, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if ((i_r > 1 && i_r < grid.nr() - 2) \|\| (i_r == 1 && !DirBC_Interior)) {
		if ((i_r > 0 && i_r < grid.nr() - 1)) {

Conversation

julianlitz commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Request - GuideLine Checklist

Checks by code author:

Checks by code reviewer(s):

Uh oh!

julianlitz commented Apr 3, 2026

Uh oh!

codecov Bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

EmilyBourne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EmilyBourne Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

julianlitz Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

julianlitz Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

julianlitz Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

julianlitz Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

julianlitz commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

julianlitz commented Apr 3, 2026 •

edited

Loading

codecov Bot commented Apr 3, 2026 •

edited

Loading

julianlitz Apr 21, 2026 •

edited

Loading

julianlitz commented Apr 24, 2026 •

edited

Loading