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

[Core] Project-based multistage #11355

Merged
merged 56 commits into from
Jul 11, 2023
Merged

[Core] Project-based multistage #11355

merged 56 commits into from
Jul 11, 2023

Conversation

rubenzorrilla
Copy link
Member

📝 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

  • Creation of the (deliberately) pure python Project container. This new object is conceived to hold all the simulation data, that is to say the Model, 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 the Save and Load of the multistage.
  • The multistage_analysis.py is now the multistage_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.
  • Creation of a derived sequential_multistage_orchestrator.py that does the standard one-after-each-other (note that this emulates our current multistage_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

  • Registering the orchestrators as well as the analysis stage. I note that we still support the creation of unregistered stages but throwing a warning.
  • Adding a Save and Load method to current analysis_stage.py. This will remain empty by default as the current SequentialMultistageOrchestrator 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.

@rubenzorrilla rubenzorrilla added Kratos Core Feature Multistage Tag anything related to multistage simulation labels Jul 5, 2023
@rubenzorrilla rubenzorrilla requested review from a team as code owners July 5, 2023 15:10
@rubenzorrilla rubenzorrilla self-assigned this Jul 5, 2023
Copy link
Member

@philbucher philbucher left a 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!

Copy link
Member

@loumalouomega loumalouomega left a 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

@rubenzorrilla
Copy link
Member Author

@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 MainKratos.py will look like. You can execute it with python3 MainKratos.py ProjectParametersCustom.json. I'll update the examples repository and documentation accordingly once this is merged.

1x1_square_multistage_project.zip

@pooyan-dadvand
Copy link
Member

Just to add that, going through your example, adding the stage setting make it much more readable than without!

@rubenzorrilla
Copy link
Member Author

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):
Copy link
Member

Choose a reason for hiding this comment

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

abc?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@loumalouomega loumalouomega left a comment

Choose a reason for hiding this comment

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

Removing my block

@pooyan-dadvand
Copy link
Member

Thanks @rubenzorrilla for considering the suggestions.

I am unblocking as the pickleable problem would not affect the current implementation.

@rubenzorrilla rubenzorrilla merged commit 27b1981 into master Jul 11, 2023
15 of 16 checks passed
@rubenzorrilla rubenzorrilla deleted the core/multistage-project branch July 11, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Kratos Core Multistage Tag anything related to multistage simulation
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

7 participants