Fix multigrid agglomeration #2712

bigfooted · 2026-01-18T14:55:07Z

Proposed Changes

Give a brief overview of your contribution here in a few sentences.

Implement multigrid agglomeration according to Nishikawa and Diskin.

euler walls are working correctly.
mpi still needs to be checked.
I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
I have added a test case that demonstrates my contribution, if necessary.
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

Common/src/geometry/CMultiGridGeometry.cpp

SU2_CFD/src/integration/CMultiGridIntegration.cpp

bigfooted · 2026-01-19T09:44:44Z

TestCases/euler/channel/inv_channel_RK.cfg

+MG_PRE_SMOOTH= ( 4, 4, 4, 4 )
+MG_POST_SMOOTH= ( 4, 4, 4, 4 )
+MG_CORRECTION_SMOOTH= ( 4, 4, 4, 4 )
+MG_DAMP_RESTRICTION= 0.5
+MG_DAMP_PROLONGATION= 0.5


When MG is finalized, these will be initial values and will be adaptively changed.

Common/src/geometry/CMultiGridGeometry.cpp

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

…ix_MG_part_1

Common/src/geometry/CMultiGridGeometry.cpp

+          if (nDim == 2) {
+            //agglomerate_seed = ((config->GetMarker_All_KindBC(copy_marker[0]) == SEND_RECEIVE) ||
+            //                    (config->GetMarker_All_KindBC(copy_marker[1]) == SEND_RECEIVE));

-          /* --- Euler walls can also not be agglomerated when the point has 2 markers ---*/
-          if ((config->GetMarker_All_KindBC(copy_marker[0]) == EULER_WALL) ||
-              (config->GetMarker_All_KindBC(copy_marker[1]) == EULER_WALL)) {
-            agglomerate_seed = false;
+          }


In general, to fix a futile conditional with an empty then-branch and no else-branch, either remove the conditional entirely if it serves no purpose, or implement the intended logic in the branch. Here, the if (nDim == 2) block is empty by design (its original body is commented out), and the 2D behavior is already described in the preceding comments, so the best minimal fix is to remove the empty if statement and keep behavior unchanged.

Concretely, in Common/src/geometry/CMultiGridGeometry.cpp, within the if (counter == 2) block around line 176–184, delete the if (nDim == 2) { ... } statement (including its now-comment-only body and braces), leaving the existing comments intact and retaining the if (nDim == 3) line. No new methods, imports, or definitions are required, and no other parts of the file need to change.

Common/src/geometry/CMultiGridGeometry.cpp

+        // /*--- If seed has 2 markers and one is SEND_RECEIVE, do not agglomerate with physical boundary ---*/
+        // if ((marker_seed.size() == 2) && (config->GetMarker_All_KindBC(marker_seed[0]) == SEND_RECEIVE)) {
+        //   if (copy_marker[0] == marker_seed[1]) {
+        //     agglomerate_CV = false;
+        //   }
+        // }
+        // if ((marker_seed.size() == 2) && (config->GetMarker_All_KindBC(marker_seed[1]) == SEND_RECEIVE)) {
+        //   if (copy_marker[0] == marker_seed[0]) {
+        //     agglomerate_CV = false;
+        //   }
+        // }


To fix the problem in general, remove or properly document any commented-out executable code. If the logic is needed, it should be reinstated as normal code and covered by tests; if it is not needed, it should be deleted. Comments should be reserved for explanations, high-level descriptions, or true examples clearly marked as such, not as a storage place for disabled logic.

In this specific case, the single best way to fix the issue without changing existing functionality is to delete the commented-out if blocks on lines 846–856 that implement the SEND_RECEIVE special case. The surrounding active logic (for counter == 1) already sets agglomerate_CV under its own conditions; removing the dead code leaves behavior unchanged while eliminating the static-analysis warning and improving readability. No new methods, imports, or definitions are needed; we just edit Common/src/geometry/CMultiGridGeometry.cpp to remove those commented if statements, keeping the rest of the comment block intact and preserving indentation.

SU2_CFD/src/integration/CMultiGridIntegration.cpp

Common/src/geometry/CMultiGridGeometry.cpp

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

…ix_MG_part_1

jblueh · 2026-02-02T20:08:51Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp

+      /*--- Force sequential execution of recursive calls for determinism ---*/
+      SU2_OMP_BARRIER
+      SU2_OMP_MASTER
+      {
+        // Empty - just ensures only one thread enters the recursion at a time
+      }
+      END_SU2_OMP_MASTER
+      SU2_OMP_BARRIER
+


What's the intent here? Right now, it seems equivalent to a plain SU2_OMP_BARRIER.

Thanks Johannes, for looking into this. I was testing some OMP stuff because it was giving me non-deterministic results (different convergence nr of iterations per run). I will clean this up as well.

SU2_CFD/src/integration/CMultiGridIntegration.cpp

Common/include/geometry/CMultiGridGeometry.hpp

Common/src/geometry/dual_grid/CPoint.cpp

pcarruscag · 2026-02-03T10:04:08Z

SU2_CFD/include/integration/CMultiGridIntegration.hpp

   */
  void SetRestricted_Solution(unsigned short RunTime_EqSystem, CSolver *sol_fine, CSolver *sol_coarse,
-                              CGeometry *geo_fine, CGeometry *geo_coarse, CConfig *config);
+                              CGeometry *geo_fine, CGeometry *geo_coarse, CConfig *config, unsigned short iMesh = 999);


why the magic 999?

Common/src/geometry/CMultiGridGeometry.cpp

pcarruscag · 2026-02-03T10:07:44Z

Common/src/geometry/CMultiGridGeometry.cpp

+            // agglomerate_seed = ((config->GetMarker_All_KindBC(copy_marker[0]) == SEND_RECEIVE) ||
+            //                     (config->GetMarker_All_KindBC(copy_marker[1]) == SEND_RECEIVE));


wip? or the comment above if counter == 2 needs to be updated?

pcarruscag · 2026-02-03T10:51:56Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp

+
+      /*--- Reset state at the beginning of a new solve (iter 0 or 1) ---*/
+      /*--- This ensures deterministic behavior across multiple runs ---*/
+      static unsigned long last_reset_iter = ULONG_MAX;


yeah this gets a bit out of hand xD

pcarruscag · 2026-02-03T10:52:57Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp

+    /*--- Update LocalCFL at each coarse grid point ---*/
+    SU2_OMP_FOR_STAT(roundUpDiv(geometry_coarse->GetnPoint(), omp_get_num_threads()))
+    for (auto iPoint = 0ul; iPoint < geometry_coarse->GetnPoint(); iPoint++) {
+      solver_coarse->GetNodes()->SetLocalCFL(iPoint, CFL_coarse_new);


Make a function for the adaptation, please.

pcarruscag · 2026-02-03T10:55:09Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp


      }
+
+      /*--- MPI sync after RK stage to ensure halos have updated solution for next smoothing iteration ---*/


I don't see why this would be necessary, in the whole code we always communicate everything after it is computed, so this is either missing somewhere else, or masking another issue.

ok, I'll check if this can now be removed, it seemed to be useful before.

pcarruscag · 2026-02-03T10:56:05Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp


  delete [] Solution;

+  /*--- Ensure all threads complete before MPI communication ---*/


Not needed, for loops have an implied barrier at the end.

ah ok, I guess this shows my lack of knowledge regarding OMP implementation.

That's what the standard says, it's not our implementation doing funny bussiness.
If you want a loop that doesn't sync, you need to explicitly use the "no wait" directive.

pcarruscag · 2026-02-03T10:56:56Z

SU2_CFD/src/integration/CMultiGridIntegration.cpp

+  /*--- Ensure MPI completion visible to all threads ---*/
+  SU2_OMP_BARRIER


Same comment about CompleteComms.
We use funneled communications, so I'm pretty sure we have a barrier somewhere in the comms.

Co-authored-by: Johannes Blühdorn <[email protected]>

Co-authored-by: Pedro Gomes <[email protected]>

bigfooted added 5 commits January 17, 2026 11:03

agglomeration only

8ef8e5c

agglomeration on mpi

ef5b3df

fix euler walls

7685588

remove debugging stuff

3d5cb12

helper function

e18050b

bigfooted added PR: draft changelog:fix labels Jan 18, 2026

bigfooted added 4 commits January 18, 2026 16:40

before helper function

2a70b87

remove debug info

69e5d5b

precommit

40b8638

remove implicit lines

bb3a25a

github-advanced-security bot found potential problems Jan 18, 2026

View reviewed changes

bigfooted added 6 commits January 18, 2026 18:48

cleanup commented stuff

c58caf9

precommit

ec0bf2e

cleanup

7ab2ea9

cleanup

6049a59

cleanup

b60fe4a

precommit

4bcc049

github-advanced-security bot found potential problems Jan 18, 2026

View reviewed changes

Common/src/geometry/CMultiGridGeometry.cpp Fixed Show fixed Hide fixed

bigfooted added 2 commits January 19, 2026 08:27

cleanup

2424d6e

add damping based on mesh level

626a242

github-advanced-security bot found potential problems Jan 19, 2026

View reviewed changes

SU2_CFD/src/integration/CMultiGridIntegration.cpp Fixed Show fixed Hide fixed

bigfooted added 2 commits January 19, 2026 10:39

small cleanup

42fa32d

Merge branch 'develop' into fix_MG_part_1

64cd69b

bigfooted commented Jan 19, 2026

View reviewed changes

bigfooted added 3 commits January 19, 2026 13:42

change CFL adjustment to safe value.

9845ff2

change CFL adjustment to safe value.

fc1de16

auto CFL adjustment for coarse meshes

457f160

github-advanced-security bot found potential problems Jan 19, 2026

View reviewed changes

Common/src/geometry/CMultiGridGeometry.cpp Fixed Show fixed Hide fixed

precommit

53a6c28

bigfooted and others added 5 commits January 26, 2026 14:02

Potential fix for code scanning alert no. 6152: Commented-out code

96caa0e

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Potential fix for code scanning alert no. 6153: Commented-out code

336aeae

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

fix codi constexpr is not a literal

4ecbde5

Merge branch 'fix_MG_part_1' of https://github.com/su2code/su2 into f…

d57ab24

…ix_MG_part_1

fix codi different type error

9bfcd71

bigfooted commented Jan 26, 2026

View reviewed changes

Common/src/geometry/CMultiGridGeometry.cpp Outdated Show resolved Hide resolved

bigfooted and others added 7 commits January 27, 2026 09:26

precommit

7e0ccf1

fix symmetry shared node

b63a428

fix constexpr

e86b74f

cleanup

f30060b

merge

4cdb106

Merge branch 'fix_symmetry' into fix_MG_part_1

135cdd5

fix adjoint local cfl

e1523ef

github-advanced-security bot found potential problems Jan 28, 2026

View reviewed changes

precommit

3269c08

github-advanced-security bot found potential problems Jan 28, 2026

View reviewed changes

Common/src/geometry/CMultiGridGeometry.cpp Fixed Show fixed Hide fixed

bigfooted and others added 4 commits January 28, 2026 17:06

Potential fix for code scanning alert no. 6158: Commented-out code

4be3e7c

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Potential fix for code scanning alert no. 6157: Commented-out code

b3c88fe

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

fix easy regression tests

cf8f7d1

Merge branch 'fix_MG_part_1' of https://github.com/su2code/su2 into f…

60d3441

…ix_MG_part_1

bigfooted removed the PR: draft label Jan 28, 2026

bigfooted changed the title ~~[WIP] Fix multigrid agglomeration~~ Fix multigrid agglomeration Jan 28, 2026

bigfooted added 2 commits January 29, 2026 14:28

merge, fix data race

208bf95

modify some OMP stuff

2c65015

jblueh reviewed Feb 2, 2026

View reviewed changes

cleanup some omp

0082b3e

jblueh reviewed Feb 2, 2026

View reviewed changes

SU2_CFD/src/integration/CMultiGridIntegration.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 3, 2026

View reviewed changes

bigfooted and others added 2 commits February 3, 2026 19:37

Update SU2_CFD/src/integration/CMultiGridIntegration.cpp

4491882

Co-authored-by: Johannes Blühdorn <[email protected]>

Update Common/src/geometry/CMultiGridGeometry.cpp

15811bf

Co-authored-by: Pedro Gomes <[email protected]>

@@ -175,10 +175,7 @@
                     /*--- In 3D, we agglomerate if the 2 markers are the same. ---*/
                     if (counter == 2) {
                       /*--- agglomerate if one of the 2 markers are MPI markers. ---*/
-                      if (nDim == 2) {
-                        // agglomerate_seed = ((config->GetMarker_All_KindBC(copy_marker[0]) == SEND_RECEIVE) ||
-                        //                     (config->GetMarker_All_KindBC(copy_marker[1]) == SEND_RECEIVE));
-                      }
+                      // In 2D, agglomeration for this case is intentionally disabled; see comments above.
                       /*--- agglomerate if both markers are the same. ---*/
                       if (nDim == 3) agglomerate_seed = (copy_marker[0] == copy_marker[1]);

		// agglomerate_seed = ((config->GetMarker_All_KindBC(copy_marker[0]) == SEND_RECEIVE) \|\|
		// (config->GetMarker_All_KindBC(copy_marker[1]) == SEND_RECEIVE));


		}

		/--- MPI sync after RK stage to ensure halos have updated solution for next smoothing iteration ---/


		delete [] Solution;

		/--- Ensure all threads complete before MPI communication ---/

		/--- Ensure MPI completion visible to all threads ---/
		SU2_OMP_BARRIER

Fix multigrid agglomeration #2712

Are you sure you want to change the base?

Fix multigrid agglomeration #2712

Conversation

bigfooted commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bigfooted Feb 2, 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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bigfooted commented Jan 18, 2026 •

edited

Loading

bigfooted Feb 2, 2026 •

edited

Loading