Repository navigation
Conversation
…ensity terms FDS added the grid-motion term inside a loop over the variables (nVar times). The term is now added once, as the exact derivative with respect to (p, u, h), including d(rho)/dh; the same terms are added to the incompressible JST and Lax-Friedrichs schemes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
SetRangePressure reduced the local minima with MPI_MAX, so with more than one rank MinP was the largest of the local minima (used by the mass-flow outlet). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
The flow solvers build one numerics object per source slot, so two active sources of the same slot overwrote each other (e.g. body force + gravity gave twice the body force). First slot: body force, rotating frame, axisymmetric, gravity, Boussinesq, streamwise periodic. Second slot: radiation, periodic outlet, vorticity confinement. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
It read the Giles kind of the marker (copied from BC_Giles), which reads past the end of the Giles list and crashes when the case has no Giles marker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
The boundary flux array had 20 entries and no bound check (NEMO with 10 or more species in 3D). Use 32, as the limiters, and stop with an error above it. The error of both checks is raised by the master thread only, since these functions run inside OpenMP parallel regions (MPI_THREAD_FUNNELED). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
The multigrid arrays hold at most 10 coarse levels (MAX_MG_LEVELS); MGLEVEL= 12 with MG_MIN_MESHSIZE= 1 crashed with stack smashing. Clamp with a warning, MG_MIN_MESHSIZE can still reduce the levels. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
…e Jacobian The term 1/2*(Gamma-1) of d(v (rho E + p))/d(rho v) was 0 (implicit Jacobian only). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
The consistent extrapolation and the low-Mach correction called the fluid model with the reconstructed pressure and density before checking that they are positive (possible NaN or fluid-model errors with real-gas models at start-up). Check them first, also rejecting zero and NaN, and check again after the low-Mach correction. Valid states give the same result as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6
Test casesAll runs: the build before this PR (PR 3) against this PR, 2 MPI ranks. Files and scripts: pr4. Incompressible pitching airfoil. NACA0012, pitching by 5 deg, 60 inner iterations per time step. Before this PR the inner iterations stop converging (worst end-of-step pressure residual -2.05) and the lift reaches -178 and +388. With this PR the worst residual is -3.85 and the lift follows the motion. Other fixes:
|
| /*--- The multigrid arrays hold at most 10 coarse levels (CMultiGridIntegration::MAX_MG_LEVELS). Clamp instead of | ||
| stopping, MG_MIN_MESHSIZE can still reduce the number of levels during the agglomeration. ---*/ | ||
| if (nMGLevels > 10) { | ||
| if (rank == MASTER_NODE) cout << "WARNING: MGLEVEL > 10 is not supported, using MGLEVEL= 10." << endl; |
There was a problem hiding this comment.
If this is in preprocessing I think it is better to throw an error rather than clamp with a warning to the screen
There was a problem hiding this comment.
Agreed. MGLEVEL > 10 now fails during configuration preprocessing with a diagnostic giving the supported range, instead of warning and clamping. Tests verify that levels 0 and 10 are retained and that 11 terminates with the diagnostic. All three configured test suites pass.
| if (implicit) { | ||
| auto addGridJacobian = [&](su2double **Jac, su2double rho, const su2double *vel, su2double h, su2double T, | ||
| su2double cp) { | ||
| const su2double dRhodh = variable_density ? su2double(-rho / (T * cp)) : su2double(0.0); |
There was a problem hiding this comment.
can you update the variable definition for variable_density so they agree in this file
There was a problem hiding this comment.
Both centered-flow constructors now use GetVariable_Density_Model() for variable_density.
| if (implicit) { | ||
| auto addGridJacobian = [&](su2double **Jac, su2double rho, const su2double *vel, su2double h, su2double T, | ||
| su2double cp) { | ||
| const su2double dRhodh = variable_density ? su2double(-rho / (T * cp)) : su2double(0.0); |
There was a problem hiding this comment.
Changed both dRhodh declarations to const auto.
|
The additional fixes are now in place:
The branch includes the updated parent PRs and develop changes. The wall and HLLC unit checks pass, and CI is currently passing on this PR's latest commit. The pressure-outlet reconstruction is handled separately in #2975; it is not required by the wall correction here. |
…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>

Proposed Changes
Each fix is in its own commit.
Incompressible solver on moving grids. The Jacobian term of the grid motion was added once per variable instead of once. The results of each step do not change, but the inner iterations of unsteady cases converge much worse. In a pitching airfoil, the lift reached values of several hundred because the time steps did not converge. The term is now added once, including the effect of a variable density.
SU2/SU2_CFD/src/numerics/flow/convection/fds.cpp
Lines 252 to 265 in a6b7496
Source terms that cannot be combined. Some source terms share one internal object, so when two are active, one silently replaces the other. For example, body force + gravity gave twice the body force and no gravity. The first source slot is one object for all of them:
SU2/SU2_CFD/src/drivers/CDriver.cpp
Lines 1843 to 1857 in a6b7496
These combinations now stop with a clear error.
Turbomachinery Riemann boundary. It read the settings of the Giles boundary instead of its own, so it crashed when no Giles marker was present.
SU2/SU2_CFD/src/solvers/CEulerSolver.cpp
Line 5800 in a6b7496
It now reads its own settings.
Reconstructed face states (MUSCL). Pressure and density are now checked to be positive before the fluid model uses them. Before, the fluid model was called first:
SU2/SU2_CFD/src/solvers/CEulerSolver.cpp
Lines 1952 to 1968 in a6b7496
Incompressible minimum pressure. With MPI it took the largest of the local minima.
SU2/SU2_CFD/src/solvers/CIncEulerSolver.cpp
Line 2238 in a6b7496
It now takes the smallest.
Axisymmetric source. One Jacobian term was always zero, because
1/2is an integer division in C++.SU2/SU2_CFD/src/numerics/flow/flow_sources.cpp
Line 141 in a6b7496
Green-Gauss gradient. A fixed-size array had no size check (mixtures with many species could overflow it).
SU2/SU2_CFD/include/gradients/computeGradientsGreenGauss.hpp
Line 72 in a6b7496
It is now as large as the one in the limiters, with an error above it.
MGLEVELabove 10 could exceed the fixed multigrid arrays. Configuration preprocessing now rejects these settings with the supported range. Unit tests preserve levels 0 and 10 and verify the diagnostic for 11.Compressible moving slip walls. The density/momentum/energy projection is now applied consistently to the state, residual, Jacobian and multigrid paths. A boundary/multigrid frame-covariance regression covers the correction. The associated convergence checks and separate outlet work are linked from [WIP] Fix the HLLC scheme on moving grids and its Jacobians #2943.
The test cases are in the comments below.
This PR builds on #2944. It is part of the split of #2941.
Related Work
PR Checklist
pre-commit run --allto format old commits.🤖 Generated with Claude Code
https://claude.ai/code/session_01LEL91DW5WPbPwgFtCvHga6