Skip to content

Fixes for the slope limiters of the flow - #2959

Open
rois1995 wants to merge 12 commits into
developfrom
fix_limiters
Open

rois1995 wants to merge 12 commits into
developfrom
fix_limiters

Conversation

@rois1995

@rois1995 rois1995 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

Fixes for the slope limiters of the flow. One commit per fix; the test cases follow in the comment.

  • NISHIKAWA_R4: a sign error in the formula limited undershoots less than overshoots. Now both are limited the same way.
  • WALL_DISTANCE without a turbulence model: the wall distance was never computed, so the limiter was zero everywhere and the solution was first order. It is now computed; a case without viscous walls stops with an error.
  • SHARP_EDGES and WALL_DISTANCE with VENKAT_LIMITER_COEFF= 0 or ADJ_SHARP_LIMITER_COEFF= 0: the case ran at first order without a warning. This is now a configuration error.
  • NEMO limiter outputs: they had the names of the conservative variables but hold the limiters of the primitive variables. They now have the names of the primitive variables, as was done for the residuals in Fix NEMO history MAX_/BGS_ residual groups: species-first indexing and finite maximum residuals #2882.

New tests ramp_r4 and flatplate_wall_limiter, and two unit tests. No existing reference value changes. The values of ramp_r4 will be updated from the CI.

Related Work

PR Checklist

  • 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.

🤖 Generated with Claude Code

https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar

rois1995 and others added 5 commits October 5, 2026 10:39
…minimum

The R4 function uses the magnitudes a = |delta| and b = |proj| (Nishikawa, AIAA 2022-1374), but its
denominator used the signed delta^3. On the minimum side, where delta and proj are negative, it returned
e.g. 7/6 instead of 7/8, so undershoots of the reconstruction below the neighbor minimum were limited
less than overshoots above the maximum. R3 and R5 use even powers and are not affected. A unit test
checks that the R3, R4 and R5 functions give the same value for (proj, delta) and (-proj, -delta).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
The supersonic ramp of the ramp test with NISHIKAWA_R4 instead of NISHIKAWA_R5. With the previous sign
error, after 10 iterations from the restart rms[Rho] is -2.180 instead of -2.787.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
The wall distance was computed only for RANS (and FEM LES/RANS). With the WALL_DISTANCE limiter in a laminar
or inviscid case all distances stayed 0 and the limiter was 0 everywhere, i.e. the whole domain was first
order. The wall distance is now also computed when a flow, turbulence, species or continuous adjoint limiter
is WALL_DISTANCE, and a case without viscous walls stops with an error. New regression test
flatplate_wall_limiter (laminar flat plate, limiter band of 2 mm).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
These limiters are first order within a distance proportional to VENKAT_LIMITER_COEFF times
ADJ_SHARP_LIMITER_COEFF. With either coefficient 0 the geometric factor divides by zero and is 0 everywhere,
so the case runs silently at first order. Now a configuration error asks for both coefficients > 0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
The NEMO limiters are those of the primitive variables (species densities, T, Tve, velocity, pressure, ...),
but the outputs used the names of the conservative variables at the first indices: for AIR-5,
LIMITER_MOMENTUM-X, LIMITER_MOMENTUM-Y and LIMITER_ENERGY were the limiters of the densities of species 1,
2 and 3. They are now LIMITER_DENSITY_<i>, LIMITER_TEMPERATURE_TR, LIMITER_TEMPERATURE_VE,
LIMITER_VELOCITY-X/Y/Z and LIMITER_PRESSURE. A unit test checks the names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UkNcoCEH8nFNrHWzhJCar
@rois1995

rois1995 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Test cases

Files (configurations, histories, limiter statistics) for each case: limiters. Meshes are from the TestCases repository. 2 MPI ranks, "develop" = 6db10127d1.

1. NISHIKAWA_R4: new test ramp_r4 (euler/ramp/inv_ramp_r4.cfg, the ramp case with NISHIKAWA_R4, 10 iterations from the restart).

rms[Rho] CL CD
develop -2.179615 -0.076739 0.054872
this PR -2.786698 -0.076172 0.054828

The new unit test [Limiters] checks that a limiter gives the same value for an overshoot and for the same undershoot. Develop fails 13 of 50 checks.

2. WALL_DISTANCE without a turbulence model: navierstokes/flatplate/lam_flatplate.cfg with SLOPE_LIMITER_FLOW= WALL_DISTANCE and ADJ_SHARP_LIMITER_COEFF= 0.02, 50 iterations.

points with limiter 0 mean limiter
develop 4225 of 4225 0
this PR 45 (the wall points) 0.69

New test flatplate_wall_limiter (the same case, 20 iterations): rms[Rho] -5.719472 on develop, -5.373432 with this PR.

3. Zero width: rans/flatplate/turb_SA_flatplate.cfg with SLOPE_LIMITER_FLOW= WALL_DISTANCE, 20 iterations.

points with limiter 0
develop, VENKAT_LIMITER_COEFF= 0.05 113 of 13289 (the wall points)
develop, VENKAT_LIMITER_COEFF= 0 13289 of 13289, the run ends normally
this PR, VENKAT_LIMITER_COEFF= 0 stops with a configuration error

4. NEMO limiter outputs: nonequilibrium/invwedge/invwedge_roe.cfg (5 species, 2D) with the LIMITER volume output.

develop field what it holds name in this PR
LIMITER_MOMENTUM-X limiter of the O2 density LIMITER_DENSITY_1
LIMITER_MOMENTUM-Y limiter of the NO density LIMITER_DENSITY_2
LIMITER_ENERGY limiter of the N density LIMITER_DENSITY_3

The new unit test [NEMO][Output] checks the names. Develop fails 25 of 26 checks.

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>
Comment thread Common/src/CConfig.cpp
Comment thread Common/src/geometry/CGeometry.cpp Outdated
@bigfooted

Copy link
Copy Markdown
Contributor

we do not have a testcase that uses nishikawa_R4, can you add one?

@rois1995

Copy link
Copy Markdown
Contributor Author

It is in this PR: ramp_r4 in TestCases/parallel_regression.py, with the config TestCases/euler/ramp/inv_ramp_r4.cfg. It is the ramp case with SLOPE_LIMITER_FLOW= NISHIKAWA_R4, 10 iterations from the restart.

Comment thread Common/src/geometry/CGeometry.cpp Fixed
rois1995 and others added 2 commits October 15, 2026 10:05
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…harger_restart references

flatplate_wall_limiter wrote restart_flow.dat, which flatplate_supersonic reads next.
jones_turbocharger_restart changes at round-off level (wall distance fallback).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@bigfooted bigfooted left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rois1995

Copy link
Copy Markdown
Contributor Author

@bigfooted should we merge this?

SetVolumeOutputValue("LIMITER_VELOCITY-Y", iPoint, Node_Flow->GetLimiter_Primitive(iPoint, nSpecies+3));
if (nDim == 3)
SetVolumeOutputValue("LIMITER_VELOCITY-Z", iPoint, Node_Flow->GetLimiter_Primitive(iPoint, nSpecies+4));
SetVolumeOutputValue("LIMITER_PRESSURE", iPoint, Node_Flow->GetLimiter_Primitive(iPoint, nSpecies+nDim+2));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the index class for NEMO instead of offsets

Comment thread Common/src/CConfig.cpp

/*--- The SHARP_EDGES and WALL_DISTANCE limiters are first order within a distance proportional to
* VENKAT_LIMITER_COEFF * ADJ_SHARP_LIMITER_COEFF, with a zero width they are first order everywhere. ---*/
for (const auto kind : {Kind_SlopeLimit_Flow, Kind_SlopeLimit_Turb, Kind_SlopeLimit_Species, Kind_SlopeLimit_Heat,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of looping over all possible limiters can we instead loop only over the active limiters in the config

@joshkellyjak joshkellyjak self-assigned this Oct 16, 2026

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.

4 participants