Fix periodic neighbor counts, intersecting stencils and coarse flags by rois1995 · Pull Request #2964 · su2code/SU2

rois1995 · 2026-10-07T13:46:17Z

Proposed Changes

Periodic geometry was counted twice with weak heat, halo neighbor counts were stale, and intersecting or one-cell periodic directions lost stencil contributions. This PR corrects the volume/count bookkeeping and completes the affected sensors, Laplacians and least-squares stencils.

The geometry bookkeeping prevents the weak heat solver from adding periodic geometry twice, refresh halo neighbor counts and correct the MSW pressure sensor. Stencil communication completes counts, sensors, Laplacians and least-squares stencils at intersecting periodic pairs using shared-face weights. It also inherits boundary flags on coarse nodes and preserves opposite-face neighbors in a one-cell periodic direction.

Tests cover two/three translated pairs, diagonal edges, compressible/incompressible JST, MSW, primitive least squares, coarse flags and a one-cell BOX. The inc_periodic_weak_heat regression checks flow/heat consistency. The existing MPI sbs_backward_step reference change is explained in the testcase comment; further full-suite reference impact remains to be checked. General intersecting rotational pairs still need solution-level validation.

Validation of the combined periodic source passes serial, partitioned MPI2, OpenMP2 and MPI2×OpenMP2 (10 cases / 3175 serial assertions). Individual branch CI and complete regression/reference checks are pending. Test configurations, meshes, logs and before/after values are in the testcase comment. New regression references are local x86 values and need CI confirmation.

Related Work

MPI Point-to-Point Refactoring + New Periodic BC Implementation #652 introduced the current periodic implementation.
Fixes for rotational periodic boundaries: implicit solver, multigrid and limiters #2961 lists the eight related periodic-boundary PRs. Their numbering groups the topics; it does not imply a linear dependency chain.

PR Checklist

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (complete branch CI pending).
My contribution is commented and consistent with SU2 style.
I ran the repository pre-commit checks on the changed files.
I have added tests that demonstrate the contribution.
I have updated appropriate documentation, if necessary.

Both solvers accumulated the periodic volume and the number of neighbors on the same geometry, the heat solver now does it only without a flow solver. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar

The periodic communication adds the neighbors of the periodic match to the owned points only, the halo copies kept the old count, which is used by the centered schemes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar

The division by the pressure was applied to the running maximum instead of the pressure difference of each edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar

…lags

rois1995 · 2026-10-07T13:46:21Z

Test cases

Files (configurations, histories, meshes, statistics, scripts) for each case: periodicBoundaries/counts. 2 MPI ranks, "develop" = 6db10127d1. The values of the new test are from my machine and will be updated from the CI.

1. Flow with the weakly coupled heat equation: periodic pin array with a body force (mesh of chtPinArray_2d). The heat equation should not change the flow here.

	iterations to rms[P] = -16	largest change of the pressure when the heat equation is on
develop	359 without, 378 with	0.21 Pa (2 % of the range)
this PR	359 without, 359 with	0

New test inc_periodic_weak_heat (10 iterations): rms[P] -4.965552 on develop, -4.712559 with this PR.

2. Neighbors of the halo points: the existing test sbs_backward_step (LD2, translational periodicity, 2 ranks): rms[P] and rms[W] are -6.352885 and -3.906544 with 1 rank (develop and this PR), -6.352884 and -3.906545 with 2 ranks on develop, and equal to the 1 rank values with this PR. This is the only reference value that changes.

The folder periodic2d_jst has a second check, run with this fix and the Jacobian fix of #2961 together: navierstokes/periodic2D with JST, converged with 1 and with 2 ranks. Largest difference of the momentum between the two runs: 2.2e-5 (2.4e-7 relative) on develop, 3.4e-9 with both fixes.

3. MSW sensor: only visible with triangles at the periodic boundary and a strong pressure jump there. mms/fvm_euler mesh with MSW, the pressure doubled at one neighbor of a periodic point, one iteration: rms[Rho] 0.094707 on develop, 0.064866 with this PR. No regression test: the field is artificial.

Existing tests

Unchanged history files, develop against this PR: poiseuille, bars_SST_2D, solid_periodic_pins, inc_heatTransfer_BC, sp_pinArray_cht_2d_dp_hf.

Additional tests (Oct 7)

Reproducers, configurations, numeric logs and scripts: stencils. Develop = 6db10127d1; BOX fixtures generate their meshes. The original B–D evidence above was recorded before the expanded follow-ups.

Check	develop	fixed combined source
Two/three-pair stencils, diagonal edges, JST/MSW/INC JST	52/80 assertions fail; count 4 vs 6, sensor 0.00217549 vs 0.00578035, Laplacian 0.1875 vs 0.75	passes
Primitive LS stencils	errors 0.75 / 1.5	passes
Coarse boundary flags	115/116 assertions fail	passes
One-cell periodic direction	count 5 vs 6, Rxx 1 vs 2	passes

This extends the original tests above. General intersecting rotational pairs and full-suite solution/reference effects remain pending.

Combined release checks: serial and OpenMP2 pass 10 cases / 3175 assertions; partitioned MPI2 and MPI2×OpenMP2 pass on both ranks (2334 / 2238 assertions). These are combined-source checks, not standalone builds of every branch. Complete branch CI and full regression/reference checks remain pending.

…and limiters (#2961) ## Proposed Changes This is one of **seven related periodic-boundary bug-fix PRs**. | PR | Scope | |---|---| | #2961 (this PR) | Rotational Jacobian, multigrid velocity rotation and limiters | | #2963 | Axis points, slip-wall normals and moving donor volumes | | #2964 | Periodic volumes, neighbor counts, sensors, stencils and coarse flags | | #2965 | Streamwise periodicity, auxiliary gradients and flamelet metadata | | #2966 | Inverse affine transforms in the parser and FEM | | #2968 | Diagnostics for unsupported periodic solver combinations | | #2969 | Translated wall-distance images | The other bug-fix PRs are drafts with their remaining validation and implementation limits documented. Their grouping does not imply a linear merge order; #2963 depends on this PR. The fully coupled implicit operator proposed in #2967 is a separate experimental enhancement. It has been closed and deferred because no overall benefit has been established and convergence/validation issues remain. This PR retains the existing implicit approximation. Three fixes for rotational periodic boundaries, one commit each. The test cases are in the comment below. - **Implicit runs:** the Jacobian block that a periodic point gets from its match was rotated on one side only. It is now rotated on both sides. - **Multigrid:** the coarse levels did not rotate the velocity, so the run did not converge. The velocity is now rotated on all levels. - **Limiters:** the limiters of the velocity components were rotated like a vector, but they are separate numbers. On the periodic boundary one velocity component became first order, and limiters could be negative. Each side now sends the velocity bounds and reconstruction increments in the frame of the other side, so each limiter uses the complete stencil. Nothing changes for a translation. New tests `periodic2d_no_limiter` and `periodic2d_multigrid`; `periodic2d` is now also in the MPI regression. Their x86 reference values pass in CI. Existing reference values that change: `aachen_turbine_restart`, `jones_turbocharger_restart` and `multi_interface` (rotational periodic, the Jacobian block), and the hybrid `periodic2d` (the Jacobian block and the limiters). The x86 periodic tests and the other changed cases pass in CI; the Jones restart residual references were refreshed from the serial, MPI and hybrid CI logs. The aarch64 references still need the CI runs. No conflict with #2945 and #2959. #2956 also changes the reference values of `jones_turbocharger_restart`; whichever is merged second takes them from the CI. ## Related Work - #652 added the present implementation of the periodic boundaries. - #1563 fixed the rotation in 2D and added the `periodic2D` case used here. - #1467, #763 and #1585 discuss the incomplete linear system at periodic points; this PR keeps the existing approximation. ## PR Checklist - [x] I am submitting my contribution to the develop branch. - [x] My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson). - [x] My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/). - [x] I used the pre-commit hook to prevent dirty commits and used `pre-commit run --all` to format old commits. - [x] 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. --------- Co-authored-by: rois1995 <rauxa1995@hotmail.it> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Josh Kelly <81244680+joshkellyjak@users.noreply.github.com> Co-authored-by: Nijso <bigfootedrockmidget@hotmail.com>

joshkellyjak · 2026-10-16T14:16:29Z

+        weight *= 0.5;
+      }
+    }
+    return weight;


Move this to CGeometry.cpp it pulls geometry_toolbox into every file that includes a geometry header. This is also called in a per-edge loop that runs every iteration. It should be calculated once in pre-processing or if the mesh has changed

Addressed in e5a3370. The calculation now lives in CGeometry.cpp and runs during periodic geometry preprocessing on each mesh level. All consumers read cached weights by edge index, without marker scans or trigonometry in iteration loops. PreprocessPeriodicGeometry rebuilds the metadata after connectivity, membership or transform changes; coordinate motion alone does not require it. A cache-rebuild regression is included. The CFD and unit-test executables build successfully; runtime validation follows separately.

joshkellyjak · 2026-10-16T14:19:05Z

+      } else {
+        weight *= 0.5;
+      }
+    }


Is this the same as the functionality that was moved to GeometryToolbox in one of your other PRs? If so update this to use that helper

Addressed in 91393ce. I added GeometryToolbox::RotationAngle and use it here, with the 2D z-angle handling and the axis-angle divisibility check. The current head of #2963 still has its calculation locally, so this provides the helper for it to reuse. Known 2D and composed 3D rotations are covered by tests.

joshkellyjak · 2026-10-16T14:26:15Z

    SU2_MPI::Error("The NEMO solvers do not support rotational periodicity yet.", CURRENT_FUNCTION);
  }

+  const bool intersectingPairs = config->GetnMarker_Periodic() > 2;


Every MZ turbomachinery case now takes the new path but each zones mesh has one pair and nothing intersects. Could this be decided from the zone’s own markers, ideally by whether a point lies on two different pairs, and could intersecting rotational pairs error out until they’re validated?

Addressed in 71fc386. The decision now comes from points shared by distinct periodic pairs in the zone mesh and an MPI reduction, and is used consistently by assembly and communication. The two faces of one pair meeting on its axis do not count as an intersection. Intersections involving rotation now fail with an explicit diagnostic. Tests cover a globally configured pair absent from the zone, agreement with ranks without a local intersection, and rotational rejection.

joshkellyjak · 2026-10-16T14:30:57Z

+                for (auto iCoordinate = 0u; iCoordinate < nDim; ++iCoordinate)
+                  bufDSend[buf_offset++] = rotBlock[iField][iCoordinate];
+              break;
+            }


Using a lot of magic numbers here and if the internal layout of the solver's matrix changes this will silently break

Addressed in c7dc21d. The gradient implementation now owns the shared normal-matrix entry convention and the rotation/packing helper. Gradient assembly and periodic packing use the same convention, retaining the existing packet format. I added 2D/3D rotated normal-matrix checks and a 2D periodic linear-gradient check alongside the existing 3D checks.

joshkellyjak · 2026-10-16T14:32:31Z

    case PERIODIC_NEIGHBORS:
      COUNT_PER_POINT  = 1;
-      MPI_TYPE         = COMM_TYPE::UNSIGNED_SHORT;
+      MPI_TYPE         = COMM_TYPE::DOUBLE;


I think this is really old left over code from pre v7 but there are still some periodic send/recv buffer declarations in the code, can you remove these in this PR too.

Addressed in 23f4300. Removed the obsolete periodic unsigned-short buffers, their allocation and cleanup, and the MPI/serial dispatch branches. Unsigned-short communication used elsewhere is unchanged.

joshkellyjak · 2026-10-16T14:35:51Z

+              if (rotate_periodic) Rotate(zeros, &bufDSend[buf_offset + 1], &Und_Lapl[1]);
+              if (rotate_periodic)
+                for (auto iCoordinate = 0u; iCoordinate < nDim; ++iCoordinate)
+                  bufDSend[buf_offset + 1 + iCoordinate] = Und_Lapl[1 + iCoordinate];


Suggested change

bufDSend[buf_offset + 1 + iCoordinate] = Und_Lapl[1 + iCoordinate];

const su2double* undLapl = base_nodes->GetUndivided_Laplacian(iPoint);

for (auto iField = 0u; iField < nVar; ++iField) bufDSend[buf_offset + iField] = undLapl[iField];

if (rotate_periodic) Rotate(zeros, &undLapl[1], &bufDSend[buf_offset + 1]);

break;

Not sure why this needs to be check twice?

Applied your suggestion in deb3cb8: copy from the node Laplacian array and rotate its vector components directly into the send buffer once, with separate source and destination arrays.

rois1995 · 2026-10-18T18:39:36Z

I traced the Aachen change to the coarse periodic-boundary flags. The old coarse mesh overcounted shared periodic neighbors. An independent check of the rotated stencil unions confirms all 7,876 corrected coarse counts (e.g. 8 -> 5 and 6 -> 4); all fine-grid counts were already correct. This changes coarse Lax-Friedrichs dissipation and shared-face gradient assembly, so the multigrid history changes. MGLEVEL=0 remains unchanged.

Aachen already failed in the October 16 CI run, before these review edits. The downloaded current CI executables reproduce their reported serial and MPI values exactly.

I updated both Aachen iteration-5 reference lists in 6a7fb5f, using each mode's values from the CI run for cc4c4cd. The tolerance remains 1e-5. Both refreshed cases pass through SU2's regression harness with those CI executables, with zero difference in all 15 values. The integer-promotion clarification was compiled and passes the focused periodic tests; it leaves all four local Aachen history tables unchanged.

The six-step restart check does not establish converged turbine accuracy.

rois1995 and others added 4 commits October 6, 2026 08:28

Complete intersecting periodic stencils and inherit coarse boundary f…

f289001

…lags

rois1995 added priority changelog:fix labels Oct 7, 2026

rois1995 mentioned this pull request Oct 7, 2026

Fixes for rotational periodic boundaries: implicit solver, multigrid and limiters #2961

Merged

5 of 6 tasks

Normalize assignment spacing reported by CodeFactor

c500a3a

github-advanced-security Bot found potential problems Oct 7, 2026

View reviewed changes

rois1995 added 2 commits October 7, 2026 19:00

Avoid shadowing intersecting periodic communication indices

afb7842

Merge develop into fix_periodic_counts

b38ec51

Merge branch 'develop' into fix_periodic_counts

bbeb23b

joshkellyjak marked this pull request as ready for review October 16, 2026 14:12

joshkellyjak requested changes Oct 16, 2026

View reviewed changes

joshkellyjak self-assigned this Oct 16, 2026

rois1995 added 7 commits October 18, 2026 18:49

Cache periodic edge weights during geometry preprocessing

e5a3370

Share the principal periodic rotation angle calculation

91393ce

Detect periodic pair intersections within each mesh zone

71fc386

Share least-squares normal matrix storage and packing

c7dc21d

Remove obsolete periodic unsigned-short buffers

23f4300

Rotate periodic Laplacian directly into the send buffer

deb3cb8

Keep cache rebuild regression collectives aligned across MPI ranks

cc4c4cd

github-advanced-security Bot found potential problems Oct 18, 2026

View reviewed changes

Comment thread SU2_CFD/src/solvers/CSolver.cpp Fixed

rois1995 added 2 commits October 18, 2026 20:38

Promote periodic least-squares buffer stride before multiplying

fbb7d3a

Refresh serial and MPI Aachen references after periodic stencil fixes

6a7fb5f

-                  bufDSend[buf_offset + 1 + iCoordinate] = Und_Lapl[1 + iCoordinate];
+                  const su2double* undLapl = base_nodes->GetUndivided_Laplacian(iPoint);
+  for (auto iField = 0u; iField < nVar; ++iField) bufDSend[buf_offset + iField] = undLapl[iField];
+  if (rotate_periodic) Rotate(zeros, &undLapl[1], &bufDSend[buf_offset + 1]);
+  break;

Conversation

rois1995 commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

Uh oh!

rois1995 commented Oct 7, 2026

Test cases

Existing tests

Additional tests (Oct 7)

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rois1995 commented Oct 18, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rois1995 commented Oct 7, 2026 •

edited

Loading