Add functionality for unsteady restart handling in CSinglezoneDriver …#2730
Add functionality for unsteady restart handling in CSinglezoneDriver …#2730Soumyadipta-Banerjee wants to merge 6 commits intosu2code:developfrom
Conversation
…and CMultizoneDriver - Implemented WriteUnsteadyRestartPreviousStep method to manage writing of previous timestep data when stopping calculations. - Updated COutput to conditionally write only restart files. - Introduced SwapSolutionWithOld method in CVariable for swapping current and previous solutions. - Enhanced driver output methods to support second-order unsteady time stepping requirements.
pcarruscag
left a comment
There was a problem hiding this comment.
Thanks. I think the cleaner way of doing this is to detect that we are about to stop due to max time, we already have similar logic for stopping due to max iterations.
I don't think what you implemented around the old solution generalizes to all types of problems.
- Changed condition from generic StopCalc flag to specific max_time detection - CSinglezoneDriver: Detects final_time_reached = (cur_time >= max_time) - CMultizoneDriver: Applies same pattern for all zones - Maintains original functionality with clearer, more maintainable code - Specific to time-domain simulations with second-order time stepping This addresses maintainer feedback to use explicit time checks instead of generic stopping conditions, ensuring the fix properly generalizes to all problem types and follows SU2 monitoring patterns.
|
Here is the PR with a similar fix for max iterations instead of max time. Do it this way because it's simpler, and swapping Solution and SolutionOld is not safe. https://github.com/su2code/SU2/pull/1237/changes |
|
Hi @pcarruscag, I have made the changes and the new code detects when max_time is reached and delays convergence by one iteration (modeled after PR #1237), which ensures both the current and previous timesteps are saved to the restart file. |
Implement max_time detection in GetCauchyCorrectedTimeConvergence() to delay convergence by one iteration, ensuring both current and previous timesteps are saved to unsteady restart files. This replaces the previous unsafe solution-swapping approach with a more robust pattern modeled after PR su2code#1237. The delay mechanism leverages existing Cauchy convergence infrastructure and properly handles second-order time stepping. Fixes su2code#2493
cb92730 to
50d25fa
Compare
|
Thanks, I will test your changes when I get some time, maybe this weekend. |
Proposed Changes
This PR fixes an issue where unsteady simulations with second-order time stepping would only save the current timestep (N) to restart files when max_time was reached, leaving out the previous timestep (N-1). For second-order time marching schemes, we need both timesteps to correctly restart the simulation.
The fix adds logic to detect when we're stopping due to max_time being reached and automatically writes the previous timestep before writing the current one. This way, the restart file contains all the data needed for a proper restart.
Related Work
*Resolves #2493 - Saving unsteady restart files mismatch
This addresses the issue where users had to work around missing N-1 timestep data by ensuring max_time aligns with output frequency, which isn't always practical or desired.
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.
pre-commit run --allto format old commits.