Skip to content

[WIP] Search translated wall images when computing periodic wall distance - #2969

Draft
rois1995 wants to merge 2 commits into
su2code:developfrom
rois1995:fix_periodic_wall_distance
Draft

rois1995 wants to merge 2 commits into
su2code:developfrom
rois1995:fix_periodic_wall_distance

Conversation

@rois1995

@rois1995 rois1995 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Proposed Changes

Periodic boundaries represent a domain that repeats. Near a periodic boundary, the closest wall can belong to the next repeated copy, rather than the copy stored in the mesh. The current wall-distance search can miss that wall and return a distance that is too large.

For example, imagine a channel with a bump on the floor, repeated end to end. A point near the end of one section may be closer to the bump in the next section. This PR includes those translated copies in the nearest-wall search. It also preserves the identity of the wall that was found.

The testcase makes the error visible at two points representing the same periodic location: develop returns 0.5 on one face, although the correct distance to the repeated sloping wall is about 0.265. With the fix, both faces return 0.5/sqrt(1+1.6^2) = 0.26499947. The test also checks the same query shifted into the next repeated section. Turbulence models use wall distance, so this mismatch can affect their predictions; a complete turbulence-flow comparison is still pending.

The implementation extends the shared nearest-wall search used by FV and FEM. It does not change the mesh or add physical walls.

Draft: currently covers repetition by translation, with independent repeat directions. Rotating sectors, helical repetition, and redundant repeat directions remain unfinished and retain the existing search behavior. Skew/multiple-generator, FEM/multizone, OpenMP, nearest-wall identity and full turbulence-regression validation remain pending. No claim that all periodic wall-distance cases are fixed.

Standalone H build and analytic-distance testcase pass in serial and partitioned MPI2 (4 assertions/rank); broader checks remain pending.

Related Work

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.

@rois1995

rois1995 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Test cases

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

Check develop translated-image draft
RANS BOX lower-wall hill, left periodic query 0.5 0.26499947; standalone serial/MPI2 pass
Matched right/shifted query 0.26499947 expected same analytic distance

geometry

The develop test executes 3 assertions with 1 failure. The new regression adds an independent analytic-distance check. Rotational/helical/dependent-generator support and broader turbulence/FEM/thread/identity validation remain open.

Standalone H build succeeds; the analytic-distance testcase passes all 4 assertions in serial and on each of two genuinely partitioned MPI ranks. No full turbulence/FEM/OpenMP pass is claimed.

@joshkellyjak

Copy link
Copy Markdown
Contributor

Is this WIP or ready to be reviewed?

@rois1995

rois1995 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@joshkellyjak Still WIP. The translated-image fix has an analytic serial/MPI check, but broader validation and the remaining periodic cases are unfinished. I'll keep it as a draft until it's ready for review.

@bigfooted

Copy link
Copy Markdown
Contributor

@rois1995 please mark WIP in the title and pr:draft as label when it is not ready and you only want to expose your work to the general public.

@rois1995 rois1995 changed the title Search translated wall images when computing periodic wall distance WIP - Search translated wall images when computing periodic wall distance Oct 8, 2026
@rois1995

rois1995 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@bigfooted Yeah, I forgot about this one, thanks!

@rois1995 rois1995 changed the title WIP - Search translated wall images when computing periodic wall distance [WIP] Search translated wall images when computing periodic wall distance Oct 10, 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 <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>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants