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.

Sign up for GitHub

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

Jump to bottom

[Core] Project-based multistage #11355

Merged
merged 56 commits into from
Jul 11, 2023
Merged

[Core] Project-based multistage #11355

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
bad13d2
Create GetFinalData
rubenzorrilla Jun 22, 2023
b32fc3b
Restart support
rubenzorrilla Jun 22, 2023
f7f3577
Register stages
rubenzorrilla Jun 22, 2023
43e2b55
Registry based instantiation and custom
rubenzorrilla Jun 22, 2023
a234fcd
Output validated settings
rubenzorrilla Jun 22, 2023
f8a818d
Remove useless comment
rubenzorrilla Jun 22, 2023
2c21077
Fix checkpoint list
rubenzorrilla Jun 22, 2023
9cc4215
More descriptive error
rubenzorrilla Jun 22, 2023
aaa5072
Discussion changes
rubenzorrilla Jun 26, 2023
9eabcab
Merge branch 'master' into core/multistage-project
rubenzorrilla Jun 29, 2023
8414844
FinalData agreed name
rubenzorrilla Jun 29, 2023
ac27c3a
Major advances (not working yet)
rubenzorrilla Jun 30, 2023
5975031
More advances (working)
rubenzorrilla Jun 30, 2023
e1a4106
Use registry
rubenzorrilla Jun 30, 2023
ca8d87a
Type hints
rubenzorrilla Jun 30, 2023
886748f
Renaming
rubenzorrilla Jul 5, 2023
48ac60b
Merge branch 'master' into core/multistage-project
rubenzorrilla Jul 5, 2023
8914914
Final discussion changes
rubenzorrilla Jul 5, 2023
275821b
Merge branch 'master' into core/multistage-project
rubenzorrilla Jul 5, 2023
3477001
Merge branch 'master' into core/multistage-project
rubenzorrilla Jul 7, 2023
af21953
Correct imports
rubenzorrilla Jul 7, 2023
2ef69aa
Merge branch 'master' into core/multistage-project
rubenzorrilla Jul 7, 2023
c5adf05
Some of Pooyan's suggestions
rubenzorrilla Jul 7, 2023
e45fe13
update helper utils
sunethwarna Jul 7, 2023
8b5dcdd
update stepping analysis
sunethwarna Jul 7, 2023
38e04c0
update independent analysis execution policy
sunethwarna Jul 7, 2023
04a5093
update kratos analysis execution policy
sunethwarna Jul 7, 2023
1f19695
update tests
sunethwarna Jul 7, 2023
0d4d8a1
minor
sunethwarna Jul 7, 2023
b915e77
Update applications/OptimizationApplication/python_scripts/utilities/…
sunethwarna Jul 8, 2023
528d9ea
Update kratos/python_scripts/multistage_orchestrators/multistage_orch…
rubenzorrilla Jul 10, 2023
57c7813
Merge pull request #11371 from KratosMultiphysics/optapp/fix_for_ocha…
sunethwarna Jul 10, 2023
64bac3f
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
59390eb
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
396509c
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
85915a0
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
52fd8cc
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
652e5aa
@philbucher suggestions
rubenzorrilla Jul 10, 2023
6734444
Update kratos/python_scripts/project.py
rubenzorrilla Jul 10, 2023
535577d
Merge branch 'core/multistage-project' of https://github.com/KratosMu…
rubenzorrilla Jul 10, 2023
9c3c6b8
Project type hints completed
rubenzorrilla Jul 10, 2023
f3d34e1
Update kratos/python_scripts/multistage_orchestrators/multistage_orch…
rubenzorrilla Jul 10, 2023
495b8bd
Type hints and encapsulating settings
rubenzorrilla Jul 10, 2023
4243919
stage_settings leftowver
rubenzorrilla Jul 10, 2023
62e3e69
Remove multistage keyword from names
rubenzorrilla Jul 10, 2023
abf6843
Pooyan's documentation request
rubenzorrilla Jul 10, 2023
41165dd
Test
rubenzorrilla Jul 10, 2023
743d714
Merge branch 'core/multistage-project' of https://github.com/KratosMu…
rubenzorrilla Jul 10, 2023
057a2ab
TODO comment
rubenzorrilla Jul 10, 2023
d4c3e35
Adding test to suite
rubenzorrilla Jul 10, 2023
32ae92f
Merge branch 'master' into core/multistage-project
rubenzorrilla Jul 10, 2023
765d021
Missing import in test_KratosCore.py
rubenzorrilla Jul 10, 2023
41405d4
Better documentation
rubenzorrilla Jul 11, 2023
49dabec
Update kratos/python_scripts/orchestrators/orchestrator.py
rubenzorrilla Jul 11, 2023
f61a82e
@philbucher's suggestion on the types
rubenzorrilla Jul 11, 2023
dd213d7
Import suggestion
rubenzorrilla Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion applications/FluidDynamicsApplication/python_registry_lists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,10 @@
python_processes_to_be_registered = [
"apply_boussinesq_force_process.ApplyBoussinesqForceProcess",
"apply_inlet_process.ApplyInletProcess"
]
]

python_stages_to_be_registered = [
"fluid_dynamics_analysis.FluidDynamicsAnalysis"
]

pooyan-dadvand marked this conversation as resolved.
Show resolved Hide resolved
python_multistage_orchestrators_to_be_registered = []
2 changes: 1 addition & 1 deletion ...ionApplication/python_scripts/execution_policies/independent_analysis_execution_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import KratosMultiphysics as Kratos
from KratosMultiphysics.analysis_stage import AnalysisStage
from KratosMultiphysics.multistage_analysis import MultistageAnalysis
from KratosMultiphysics.multistage_orchestrators.sequential_multistage_orchestrator import SequentialMultistageOrchestrator
from KratosMultiphysics.OptimizationApplication.execution_policies.execution_policy import ExecutionPolicy
from KratosMultiphysics.OptimizationApplication.utilities.helper_utilities import GetClassModuleFromKratos

Expand Down
2 changes: 1 addition & 1 deletion applications/OptimizationApplication/tests/test_execution_policies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def test_IndependentAnalysisExecutionPolicy(self):
"name" : "test",
"type": "independent_analysis_execution_policy",
"settings": {
"analysis_type" : "MultistageAnalysis",
"analysis_type" : "SequentialMultistageOrchestrator",
"analysis_settings": {
"stages": [],
"execution_list":[]
Expand Down
9 changes: 9 additions & 0 deletions kratos/python_interface/python_registry_lists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,13 @@
"assign_vector_variable_to_elements_process.AssignVectorVariableToElementsProcess",
"assign_vector_variable_to_entities_process.AssignVectorVariableToEntitiesProcess",
"assign_vector_variable_to_nodes_process.AssignVectorVariableToNodesProcess"
]

python_stages_to_be_registered = [
"analysis_stage.AnalysisStage"
]

python_multistage_orchestrators_to_be_registered = [
"multistage_orchestrators.multistage_orchestrator.MultistageOrchestrator",
"multistage_orchestrators.sequential_multistage_orchestrator.SequentialMultistageOrchestrator"
]
10 changes: 10 additions & 0 deletions kratos/python_interface/python_registry_utilities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ def RegisterAll(PythonModuleName, PythonRegistryListModule):
RegisterModelersList(PythonModuleName, PythonRegistryListModule)
RegisterOperationsList(PythonModuleName, PythonRegistryListModule)
RegisterProcessesList(PythonModuleName, PythonRegistryListModule)
RegisterStagesList(PythonModuleName, PythonRegistryListModule)
RegisterMultistageOrchestratorsList(PythonModuleName, PythonRegistryListModule)

def RegisterModelersList(PythonModuleName, PythonRegistryListModule):
__CheckRegistryListIsInModule(PythonRegistryListModule, "modelers")
Expand All @@ -17,6 +19,14 @@ def RegisterProcessesList(PythonModuleName, PythonRegistryListModule):
__CheckRegistryListIsInModule(PythonRegistryListModule, "processes")
__RegisterItemList(PythonModuleName, PythonRegistryListModule.python_processes_to_be_registered, "Processes")

def RegisterStagesList(PythonModuleName, PythonRegistryListModule):
__CheckRegistryListIsInModule(PythonRegistryListModule, "stages")
__RegisterItemList(PythonModuleName, PythonRegistryListModule.python_stages_to_be_registered, "Stages")

def RegisterMultistageOrchestratorsList(PythonModuleName, PythonRegistryListModule):
rubenzorrilla marked this conversation as resolved.
Show resolved Hide resolved
__CheckRegistryListIsInModule(PythonRegistryListModule, "multistage_orchestrators")
__RegisterItemList(PythonModuleName, PythonRegistryListModule.python_multistage_orchestrators_to_be_registered, "MultistageOrchestrators")

def __CheckRegistryListIsInModule(PythonRegistryListModule, ListKeyword):
list_variable_name = f"python_{ListKeyword}_to_be_registered"
if not list_variable_name in dir(PythonRegistryListModule):
Expand Down
14 changes: 14 additions & 0 deletions kratos/python_scripts/analysis_stage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,20 @@ def ChangeMaterialProperties(self):
"""this function is where the user could change material parameters as a part of the solution step """
pass

def Save(self, serializer : KratosMultiphysics.StreamSerializer):
pooyan-dadvand marked this conversation as resolved.
Show resolved Hide resolved
"""Serializes current analysis stage instance

This method is intended to make the class pure Python (pickable). This means serialize all the Kratos objects with the given serializer to then destroy them.
Copy link
Member

Choose a reason for hiding this comment

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

Why destroy them?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you don't destroy (i.e. assigning them to None) the non pure Python object the class wouldn't be pickable.

Copy link
Member

Choose a reason for hiding this comment

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

I don't get the point. A typical use case would be save and continue. No?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. But we are using Python pickle for doing so. Pickle is only capable to save / load pure Python types. Hence, we need to convert all Kratos objects to a pure Python type (this is what we do with our stream serializer). Then you "destroy" these objects in order to make the class Pickable. Let me try to write an example.

We have a MyStage analysis stage class with whatever Kratos object e.g. a Newton-Raphson instance. At this point, the class is not pickable as self.strategy cannot be dump. In order to do so, we call the stream serializer within the Save method of MyClass to create a string "representing" it. At this point, the MyStage is still not pickable as self.strategy is not a Python type but a Kratos one. Then, we need to do self.strategy = None after the stream serialization. At this point the clas IS pickable. This is what I meant by destroy. In pseudocode will be something like

def Save(self, serializer: KratosMultiphysics.StreamSerializer):
    serializer.Save(self.strategy) # class is not pickable yet
    self.strategy = None # class is now pickable

Copy link
Contributor

Choose a reason for hiding this comment

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

Pickle is only capable to save / load pure Python types.

https://pybind11.readthedocs.io/en/latest/advanced/classes.html#pickling-support

Copy link
Contributor

@matekelemen matekelemen Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We could skip Loading everything back in after Save if all members of the analysis stage were pickleable to begin with (i.e. the bindings define __setstate__ and __getstate__). That means adding these extra bindings for every C++ type that may end up as a member of an AnalysisStage.

Yes that's a lot of work. However, it's just mundane copy-pasta work. We already have this functionality implemented with serializers, so we could just reuse them for the bindings:

pybind11::class_<KratosClass, ...>(rModule, "KratosClass")
    ...
    .def(pybind11::pickle(
        [](const KratosClass& rInstance) {
                // use the serializer to convert rInstance into a string
                return pybind11::make_tuple(/*get string from the serializer*/);
        },
        [](pybind11::tuple Arguments) {
                // Arguments contains exactly one string
                // use the serializer to construct an instance from a string
         }
    ))
    ;

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to say that I added more details to the docstring.

Copy link
Member

Choose a reason for hiding this comment

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

Here's my take on it:

An object is pickleable if it defines __setstate__ and __getstate__, or all its member variables are pickleable. Note the recursion here.

This is what I understood

Before AnalysisStage::Save

The analysis stage may have objects coming from pybind, such as solvers, processes, etc. By default, pickle cannot handle these because classes bound from C++ are only picklable if someone explicitly added support for it (i.e.: defined __setstate__ and __getstate__ in the bindings).

In AnalysisStage::Save, before pickle

Before passing the analysis stage on to pickle, these members coming from C++ need to be dealt with. I'm guessing @rubenzorrilla proposes to use the kratos serializers to take over pickle's job for these classes. After the kratos serializers did their job, the C++ classes are captured by the serializer so they can be removed from the analysis stage to make it pickleable (self.cpp_class_instance = None).

This is exactly my question: Why we should do self.cpp_class_instance=None? Would you please point me to the documentation in this respect?

pickle

The analysis stage is now free of unpickleable members, so it can be safely passed on to pickle.

My problem with this approach is that the analysis stage is left in an invalid state after AnalysisStage::Save. This means that at every checkpoint, we must not only Save the analysis stage but also Load it back in => i.e.: perform a restart every time.

And this is my direct concern. In the case of this PR the stage is already deleted. But this limits the usage very much.

Copy link
Contributor

@matekelemen matekelemen Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is exactly my question: Why we should do self.cpp_class_instance=None? Would you please point me to the documentation in this respect?

I can't point you to the docs, because this is just one possible roundabout solution to a problem we created for ourselves, so I don't think the docs would mention anything like this. The followings are just as valid solutions:
self.cpp_class_instance = 0
self.cpp_class_instance = "whatever"
self.cpp_class_instance = 6.0 * 1e23
self.cpp_class_instance = anything_that_is_pickleable
del self.cpp_class_instance

The point is to remove any unpickleable member.

The proper (albeit more laborious) solution is to make our C++ types pickleable.

Copy link
Member

Choose a reason for hiding this comment

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

the thing is more delicate than simple copy-pasting.

Imagine you want to store a Model and then some other python object.

one way of doing this could be to create a serializer object in the pickle pybind call, write it to a string and then write the resulting char array to pickle.

the problem of this approach is that if we now want to have a different save for the second object, we don't have access to the serializer used to save the first object. We could generate a second StreamSerializer insance, however this implies that common things (like the kernel) would be serialized twice (and differently) in the first object and in the second.

What i am trying to say is that we need to first serialize all of the cpp objects USING THE FIRST SERIALIZER, and then write the serialized object using pickle. That is exactly what we are doing.
no doubts we could have a global serializer used in python ... however this complicates things quite a lot.

having said this, this is an implementation detail. If you like, let's open a different thread to discuss about that

"""
pass

def Load(self, serializer : KratosMultiphysics.StreamSerializer):
rubenzorrilla marked this conversation as resolved.
Show resolved Hide resolved
"""Loads current analysis stage instance

From the given serializer, this method restores current class from a pure Python status (pickable) to the one in the serializer.
"""
pass

def _GetSolver(self):
if not hasattr(self, '_solver'):
self._solver = self._CreateSolver()
Expand Down
158 changes: 0 additions & 158 deletions kratos/python_scripts/multistage_analysis.py
View file
Open in desktop

This file was deleted.

Loading
Loading