Repository navigation
Conversation
…aries The residual of the periodic match is added as Q*R, and the solution of the match is Q*U, so the block added to the diagonal must be Q*J*Q^T. Only the rows were rotated. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
The coarse levels summed the momentum residuals, gradients and Jacobian blocks of a rotational periodic pair without rotating them. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
…stencil The limiters of the velocity components were rotated like a vector when taking the minimum over a periodic pair, and the min and max velocity vectors were rotated instead of the velocity of each neighbour. Now each side sends, in the frame of its match, the min/max of the rotated velocities and of the rotated reconstruction increments, and the limiter is computed once. Nothing changes without rotation. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
A point on the rotation axis is its own periodic match. It was handled like a pair of points: it received its own data rotated once in each direction, and in implicit runs its residual and Jacobian were then removed, so its solution never changed. Now it receives the data of every other copy of its control volume (the rotation applied 1 to N-1 times, N = 360 deg / angle), it keeps its equations, and after an implicit update its solution is averaged over the copies, which removes the velocity normal to the axis. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
The mesh of the new test periodic3d_axis is in the TestCases branch fix_periodic_axis. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
Contributor
Author
Test casesFiles (configurations, histories, meshes, statistics, scripts): periodicBoundaries/axis. 2 MPI ranks, "develop" = Laminar pipe flow on a 30 degree sector with one cell around the axis, against the full pipe (no periodic boundary).
One iteration from the same field, largest difference to the full pipe at the same points (30 degree sector with one cell and 90 degree sector with three cells):
The same holds for a 180 degree sector (two copies). With an upwind scheme or with JST the residual of periodic points differs from the full pipe also away from the axis (a few percent), because the flux over a face cut by the periodic boundary is computed in two parts. This is not changed here. Convergence on the sector that is one cell wide (CFL 10): first-order ROE converges as in the table. Second order without limiter reaches rms[Rho] = -5.4 in 3000 iterations and needs CFL 2 to converge; JST stalls at -3.2 at CFL 10. The 90 degree sector with three cells converges with both (1538 and 2147 iterations, full pipe 1237 and 1314). New test No existing reference value changes. Additional tests (Oct 7)Reproducers, configurations, numeric logs and scripts: geometry. Develop =
These checks extend the original axis evidence above. ALE/GCL and complete moving-geometry sensitivities remain open. 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. |
This was referenced Oct 7, 2026
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This was referenced Oct 12, 2026
bigfooted
added a commit
that referenced
this pull request
Oct 13, 2026
…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 <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Josh Kelly <[email protected]> Co-authored-by: Nijso <[email protected]>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Axis points kept an incomplete control volume, slip walls used a one-sided periodic normal, and mesh updates left donor volumes stale. This PR completes that periodic geometry.
Dependency: #2961. Its commits are included until that PR merges; the focused diff is rotation...axis.
Axis points now accumulate all rotated copies of the sector instead of keeping an incomplete control volume. Slip-wall normals merge raw face areas across periodic matches before normalization, including fine and coarse Euler walls. Mesh updates refresh donor volumes before later gradients and residuals use them.
The
periodic3d_axisregression and partitioned unit checks cover wall normals and repeated deformation/Green–Gauss gradients. Its tiny mesh is in TestCases PR #210 on the branchfix_periodic_axis. The pre-existing workflow commit temporarily selects that branch and must be reverted before merging.Moving-flow ALE/GCL and complete geometry sensitivities remain to be checked. Nonlinear dissipation on split faces can still differ from a full mesh. Complete regression-reference checks remain pending.
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
PR Checklist