Skip to content

Reject unsupported periodic adjoint, radiation and structural solvers - #2968

Draft
rois1995 wants to merge 3 commits into
su2code:developfrom
rois1995:fix_periodic_support
Draft

rois1995 wants to merge 3 commits into
su2code:developfrom
rois1995:fix_periodic_support

Conversation

@rois1995

@rois1995 rois1995 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Proposed Changes

Unsupported periodic solver combinations are accepted without a periodic boundary implementation. This PR rejects them before solver construction.

Develop accepts periodic continuous-adjoint, radiation and structural configurations without implementations of those periodic boundaries. Configuration validation now stops these combinations with a specific error before solver construction.

Complete BOX configuration controls verify that develop accepts each input and the fixed source rejects it for the intended reason. A supported direct-flow control remains accepted. This adds support diagnostics; it does not implement those missing solvers.

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.

The PR only adds configuration guards. The additional unit tests and subprocess runner were removed following review; the before/after configurations and diagnostic logs remain available in the testcase comment.

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 verified the contribution with the published before/after configuration controls (unit-test additions removed following review).
  • 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: support. Develop = 6db10127d1; BOX fixtures generate their meshes. The original B–D evidence above was recorded before the expanded follow-ups.

Configuration develop fixed
Continuous adjoint + periodic accepted (exit 0) intended early support error
Radiation + periodic accepted (exit 0) intended early support error
Elasticity + periodic accepted (exit 0) intended early support error
Supported direct flow + periodic accepted accepted

The full configs, exact diagnostic logs and Python check are provided. The combined diagnostic runner also verifies the four streamwise restrictions handled in part D. Missing continuous-adjoint/radiation/structural periodic physics is not implemented by these guards.

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.

@joshkellyjak joshkellyjak 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.

Nice catch, not sure a unit test for this functionality is strictly necessary

Comment thread Common/src/CConfig.cpp Outdated

if (Kind_SU2 == SU2_COMPONENT::SU2_CFD && nMarker_PerBound > 0) {
if (ContinuousAdjoint)
SU2_MPI::Error("Continuous adjoints do not implement MARKER_PERIODIC. Use MATH_PROBLEM= DISCRETE_ADJOINT.",

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.

"MARKER_PERIODIC is not currently supported in the continuous adjoint solver. Use MATH_PROBLM= DISCRETE_ADJOINT"

same for two below

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated all three messages in 522998a to use "not currently supported". The discrete-adjoint suggestion remains only in the continuous-adjoint error.

@rois1995

rois1995 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@joshkellyjak Removed the additional unit tests and subprocess runner in 522998a, as suggested. The PR now only adds the configuration guards; the before/after configurations and diagnostic logs remain linked in the testcase comment.

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