Skip to content

Fix misleading "All convergence criteria satisfied." message on interrupted runs - #2881

Open
bellonarts wants to merge 1 commit into
su2code:developfrom
babybluechips:fix_interrupt_exit_banner
Open

Fix misleading "All convergence criteria satisfied." message on interrupted runs#2881
bellonarts wants to merge 1 commit into
su2code:developfrom
babybluechips:fix_interrupt_exit_banner

Conversation

@bellonarts

Copy link
Copy Markdown

Proposed Changes

Interrupt-driven termination is not limited to a user's Ctrl-C. SIGTERM is the normal way batch schedulers terminate a job at its wall-clock limit: SLURM sends SIGTERM when a job reaches its TimeLimit and on scancel (waiting KillWait before SIGKILL), and PBS/Torque and LSF behave equivalently. Today every SU2 job that exhausts its allocation on a cluster ends with a log stating "All convergence criteria satisfied." directly above a Solver Exit table whose every criterion reads "No". Any parameter sweep or optimization loop that classifies runs by scraping that banner silently admits timed-out, unconverged runs as converged, and a human skimming the log is misled the same way. That population is far larger than the manual-interrupt case.

Mechanism: on SIGTERM the signal handler sets STOP, and COutput::ConvergenceMonitoring forces convergence = true so the run stops and saves. Both CSinglezoneDriver::Monitor and CMultizoneDriver::Monitor then read that flag as InnerConvergence and print the satisfied banner.

Fix: a new convergenceInterrupted member of COutput records that the stop came from the interrupt path rather than from the criteria. It is propagated across ranks in the existing convergence Allreduce as a count-2 element-wise MPI_MAX (SU2's serial MPI stub does not define MPI_BOR, and the AD layer maps only SUM/MIN/MAX/PROD), exposed through GetConvergenceInterrupted(), and both drivers print

Interrupt signal received, exiting before the convergence criteria were satisfied.

on that path. Stop-and-save behavior, exit code 0, and the genuine-convergence and maximum-iteration messages are unchanged.

Files: SU2_CFD/include/output/COutput.hpp, SU2_CFD/src/output/COutput.cpp, SU2_CFD/src/drivers/CSinglezoneDriver.cpp, SU2_CFD/src/drivers/CMultizoneDriver.cpp (+29/-8).

Verification (QuickStart inv_NACA0012.cfg, serial release build of develop 07aa46b1):

  • SIGTERM sent to the exact SU2_CFD process about 8 s into the run: the new interrupt message is printed, the satisfied banner does not appear, restart and solution files are written, exit code 0.
  • Run to genuine convergence and run to maximum iterations: output byte-identical to the unpatched build (checked when the change was first made on develop 81ce6a68f9).
  • Touched files re-checked with -Wall -Wextra: no new warnings.

Related Work

The defect is present at v8.5.0 and at the current develop tip. I found no existing upstream issue or PR that addresses it. The change is independent of the other NEMO-related PRs from the same qualification campaign; it touches only the output and driver exit path.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

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

…rupted runs

When a SIGTERM/interrupt signal is received, ConvergenceMonitoring forces
convergence = true so the solver stops and saves the solution. The steady
exit path in CSinglezoneDriver and CMultizoneDriver then printed 'All
convergence criteria satisfied.' directly above a convergence summary
table marking every field 'No'.

Track the signal-forced stop in a separate flag, propagated across ranks
in the same Allreduce, and print an explicit interrupt message instead.
Genuine convergence and maximum-iteration exits are unchanged, as is the
stop-and-save behavior on interrupt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bellonarts added a commit to babybluechips/SU2 that referenced this pull request Sep 6, 2026
Reseal PR su2code#2881 at 2b0b332 after replacing the flagged shell invocation with posix_spawnp/waitpid. All other exact PR and stage identities remain unchanged.
@pcarruscag

Copy link
Copy Markdown
Member

Revert the changes after I approved please.
If the pull request is not ready for review mark it as draft.

@bellonarts
bellonarts force-pushed the fix_interrupt_exit_banner branch from 2b0b332 to 07801f0 Compare September 6, 2026 02:14
@bellonarts

Copy link
Copy Markdown
Author

You’re right. I accidentally kept working on the branch after you approved it because I was trying to add regression coverage. That test created a CodeFactor warning, so I pushed another change to address it. I have now removed both changes and restored the branch exactly to 07801f0, the commit you approved. The PR is ready for review again. Sorry about that, and thank you for flagging it.

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.

2 participants