Repository navigation
[WIP] Improve implementation of periodicity for implicit integration - #1585
pcarruscag wants to merge 5 commits into
Conversation
|
I'll take a more in-depth look my cases tomorrow (Saturday) to adapt the regression values, and options if necessary. In some cases I saw a massive improvement in convergence speed so I'll decrease the prescribed iterations to reflect that 👍 I hope it is ok to push to this PR directly (won't be that many lines ;) |
|
Whilst the primal computations work perfectly (greatly increased convergence speed in some cases, whilst not/or very very minorly altering the final solution), I see some issues with the discrete adjoint: The cases I have diverge after ~20-40 iterations. I tried to increase Linear solver iterations and lower the CFL-number quite a bit, without success. Whether I use the primal solution-file created with the old version or with this branch, did not matter. da_sp_pinArray_cht_2d_dp_hf from parallel_regression_AD is maybe a good case to start but I'll try to make a fluid-only case from that which might be easier to debug (in case that would be necessary) and post it here |
|
Here a fluid-only case that somewhat quickly converges. Contains the mesh, 1 cfg for primal and adjoint and a little bash-script where the path to the SU2 binaries has to be adapted to fit your system. The idea was to first run primal and adjoint with vanilla develop and then the same with the new code. I left the log-files of my run in there as well for a quick look If there is something else I can do let me know. |
|
Yeah... I had not tested discrete adjoint but my spider senses were tingling. |
pcarruscag
changed the title
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still a relevant issue please comment on it to restart the discussion. Thank you for your contributions. |
|
WIP |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still a relevant issue please comment on it to restart the discussion. Thank you for your contributions. |
…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
This is a simple re-shuffling to have matrix-vector products (and therefore linear systems) that are aware of periodic connections.
It uses something like:
PERIODIC_RESIDUAL" communication (sum contributions) applied to the result of matrix-vector products.PERIODIC_IMPLICIT" communication (enforce periodicity) applied to the result of preconditioners.It seems to avoid the performance degradation that @TobiKattmann saw when implementing periodicity for heat transfer, and also the issues in issue #1467.
However, making the matrix-vector product "periodicity-aware" through comms, without modifying the matrix, means that any preconditioner derived from the matrix is not consistent with the product, thus it is a worse preconditioner and more linear iterations are needed for the same reduction in residuals.
Related Work
Should help #1467
Should avoid #763
PR Checklist