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 simulation restart when using averaging #1300

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

blaisb
Copy link
Contributor

@blaisb blaisb commented Sep 30, 2024

Description

There was an issue where simulation restarts would crash on large parallel simulation when the time-averaging of the velocity was enabled. This issue was caused by the fact that the vectors that were checkpointed were the locally_relevant solutions but the restart was reading into the locally_owned solutions.

Solution

I have fixed this by reading the restart into the locally_relevant solutions. However, I had to add a function to manually sanitize the time_averaging operator because the locally_owned vectors were now empty.

Testing

This should not affect any of the current test results. I do not know if it is necessary to add a new tests since restart tests are cumbersome. @lpsaavedra what do you think?

Documentation

No changes to documentation.

Miscellaneous (will be removed when merged)

Checklist (will be removed when merged)

See this page for more information about the pull request process.

Code related list:

  • All in-code documentation related to this PR is up to date (Doxygen format)
  • Lethe documentation is up to date
  • Fix has unit test(s) (preferred) or application test(s), and restart files are in the generator folder
  • The branch is rebased onto master
  • Changelog (CHANGELOG.md) is up to date
  • Code is indented with indent-all and .prm files (examples and tests) with prm-indent

Pull request related list:

  • Labels are applied
  • There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
  • If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
  • If the fix is temporary, an issue is opened
  • The PR description is cleaned and ready for merge

@blaisb blaisb added the Bug Something isn't working label Sep 30, 2024
@blaisb blaisb requested review from lpsaavedra and mivaia September 30, 2024 17:44
Copy link
Collaborator

@lpsaavedra lpsaavedra left a comment

Choose a reason for hiding this comment

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

Good that you found this bug. I don't think we need to add more tests since we already have restart tests and it is not easy to reproduce with small cases.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Laura Prieto Saavedra <[email protected]>
@blaisb blaisb merged commit e4e671c into master Oct 1, 2024
10 of 11 checks passed
@blaisb blaisb deleted the restart_with_average_velocities branch October 1, 2024 14:44
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.

2 participants