-
Notifications
You must be signed in to change notification settings - Fork 244
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
[Core] Project-based multistage #11355
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a couple of comments, but only minor, will address them in the next days
Looking good overall, happy to see that this makes progress!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay from my side. It would be nice to have a test or at least an example in the Example repo
…estrator.py Co-authored-by: Philipp Bucher <[email protected]>
…ltiphysics/Kratos into core/multistage-project
@philbucher @pooyan-dadvand I went through all your requirements / suggestions. Besides I added a test running a "fake" multistage workflow. Last but not least I'm sharing a working "real" example in here so you can see how the |
Just to add that, going through your example, adding the stage setting make it much more readable than without! |
Agree. Besides, it makes easier the inclusion of already existing json files. |
from KratosMultiphysics.analysis_stage import AnalysisStage | ||
from KratosMultiphysics.model_parameters_factory import KratosModelParametersFactory | ||
|
||
class Orchestrator(abc.ABC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing my block
Thanks @rubenzorrilla for considering the suggestions. I am unblocking as the pickleable problem would not affect the current implementation. |
📝 Description
This PR adds the upgraded version of our current multistage capabilities. The enhancements can be summarized into refactoring the code to be much more flexible/extensible and adding checkpointing (i.e. restarting) between stages.
The main changes are the following
Project
container. This new object is conceived to hold all the simulation data, that is to say theModel
, the settings, the active stages dictionary and the stages output data dictionary (see [FastPR][Core] Add GetAnalysisStageFinalData to AnalysisStage #11335). Note that it is also in charge of doing theSave
andLoad
of the multistage.multistage_analysis.py
is now themultistage_orchestrator.py
. We conceived the orchestrators as the class that knows the multistage simulation workflow. Note that this implies knowing at which points we are calling the Project save and load in order to do so.sequential_multistage_orchestrator.py
that does the standard one-after-each-other (note that this emulates our currentmultistage_analysis.py
. On top of that, we added some features to it that we believe could help usability such as the output of the project parameters to be used together with each restarting file.There are also some minor changes that we require in order to make this work. There are
Save
andLoad
method to currentanalysis_stage.py
. This will remain empty by default as the currentSequentialMultistageOrchestrator
does the checkpointing between stages or, in other words, no stage instances are pickled. Nonetheless, this might not be the case in the future or in some forks, so we are providing the interface to achieving so.As a final note I'd like to add that these changes require some minor modifications in our current multistage
MainKratos.py
and I/O. Having said this, I'll update current examples accordingly once this PR is merged.