Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix restart transient with variable time step #1093

Merged
merged 14 commits into from
Apr 16, 2024
Merged

Conversation

blaisb
Copy link
Contributor

@blaisb blaisb commented Apr 16, 2024

Description of the problem

  • Transient simulation with adaptive time-stepping were not restarted correctly (their time-step was not fixed adequately)

Description of the solution

  • The issue was that the time-step value was not reset correctly when the time-step values vector was read in the checkpoint

How Has This Been Tested?

  • Added. a new test for this feature

Documentation

  • Changelog added

Comments

  • This class uses an old checkpointing mechanism that is not boost serialize. I will need to fix it. It will be a good opportunity to streamline the class afterwards also and make it simpler to understand. Maybe reduce the tree of possibilities in term of classes.

@blaisb blaisb added the Bug Something isn't working label Apr 16, 2024
Copy link
Collaborator

@PierreLaurentinCS PierreLaurentinCS left a comment

Choose a reason for hiding this comment

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

Great job, I've got nothing to say expect for .prm formating.

Edit : It seems to be working for CHNS cases, though the residual is not exactly the same (~0.1% of error) after checkpointing and before, but the timestep is the correct one. Same problem as Helene pointed I think

Comment on lines +170 to +172

// Fix time step to be the last time_step that was used
time_step = time_step_vector[0];
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is crazy how one line can fix this problem so easily

Copy link
Collaborator

@hepap hepap left a comment

Choose a reason for hiding this comment

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

I'm still looking for why we have a difference in the residuals for NS-VOF cases, but I think this change can still be merged in the meanwhile :)

blaisb and others added 10 commits April 16, 2024 20:59
…1-adaptive/generator.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive/generator.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive/generator.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive/generator.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive/generator.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
…1-adaptive.prm

Co-authored-by: Pierre Laurentin <81017102+PierreLaurentinCS@users.noreply.github.com>
@blaisb blaisb merged commit b521d0a into master Apr 16, 2024
8 checks passed
@blaisb blaisb deleted the fix_restart_transient branch April 16, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants