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

Initialize old solution with the current solution on restart when it is not found in checkpoint #30135

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

NamjaeChoi
Copy link
Contributor

Closes #30111.

@NamjaeChoi
Copy link
Contributor Author

NamjaeChoi commented Mar 19, 2025

@YaqiWang I think I understand restarting systems a bit better now, and I feel like we can easily implement a flexible restart where we can specify variables we want to restart. It's currently looping over all the variables found in the header sys_header.variables but we might be able to restrict this list on user's request.

@moosebuild
Copy link
Contributor

moosebuild commented Mar 19, 2025

Job Documentation, step Docs: sync website on 875842d wanted to post the following:

View the site here

This comment will be updated on new commits.

Copy link
Member

@loganharbour loganharbour left a comment

Choose a reason for hiding this comment

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

This makes sense to me. We do need a test for this, though.

Wouldn't a recover test that has something that depends on old values work?

@@ -19,6 +19,9 @@
const std::string RestartableEquationSystems::SystemHeader::system_solution_name =
"SYSTEM_SOLUTION";

const std::string RestartableEquationSystems::SystemHeader::system_old_solution_name =
Copy link
Member

Choose a reason for hiding this comment

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

Is there a static definition of this vector name somewhere else? It would be nice to not hard code it here if we can get it from somewhere else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see Moose::OLD_SOLUTION_TAG

@loganharbour loganharbour self-assigned this Mar 19, 2025
@NamjaeChoi
Copy link
Contributor Author

NamjaeChoi commented Mar 19, 2025

This makes sense to me. We do need a test for this, though.

Wouldn't a recover test that has something that depends on old values work?

There is a failing Bison test restart-transient-from-ss-with-stateful. I'm not yet sure if I messed up something or the test itself was wrong. I need to take a closer look. If it is the latter, this test would be able to cover my change.

@YaqiWang
Copy link
Contributor

@YaqiWang I think I understand restarting systems a bit better now, and I feel like we can easily implement a flexible restart where we can specify variables we want to restart. It's currently looping over all the variables found in the header sys_header.variables but we might be able to restrict this list on user's request.

We want to be able to just load variables in a checkpoint file into individual variables like for the weighting functions in IQS with adjoint solution.

@@ -264,6 +267,9 @@ RestartableEquationSystems::load(std::istream & stream)

bool modified_sys = false;

bool init_old_solution = !sys_header.vectors.count(SystemHeader::system_old_solution_name) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to do the same for older.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If checkpoint only has current and old, which should go into older?

Copy link
Contributor

Choose a reason for hiding this comment

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

That can be an error? Otherwise, I would say old go to older.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If checkpoint only has current, current goes to older? That sounds too many conditionals.

@loganharbour
Copy link
Member

Does the restart happen late enough that all objects have been initialized that request old and older states?

@YaqiWang
Copy link
Contributor

Does the restart happen late enough that all objects have been initialized that request old and older states?

Time integrator requests this in its constructor. I guess it will be safer if we throw an error if an object tries to do this when FEProblemBase::startedInitialSetup() returns true.

@lindsayad
Copy link
Member

@loganharbour do you want to dig into this? Otherwise I can

@loganharbour
Copy link
Member

@YaqiWang and @NamjaeChoi have you had more time to think about this?

@YaqiWang
Copy link
Contributor

@loganharbour I think Namjae and me are waiting for a help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always Populate Old State on Restart
5 participants