[GeoMechanicsApplication] Convert two stage line loads test to the orchestrator based analysis and test #13057
Conversation
There was a problem hiding this comment.
Thank you so much for investigating how to convert the original two-stage test to a test that uses the orchestrator. The result of your efforts show clearly what is needed to run multi-stage analysis with this novel approach. Well done!
I have a few suggestions that you may find helpful.
| "VAN_GENUCHTEN_AIR_ENTRY_PRESSURE" : 2.561, | ||
| "VAN_GENUCHTEN_GN" : 1.377, | ||
| "VAN_GENUCHTEN_GL" : 1.25, |
There was a problem hiding this comment.
Nitpicking: I believe we don't need these input parameters, since we're not using the Van Genuchten retention law.
| "VAN_GENUCHTEN_GL" : 1.25, | ||
| "MINIMUM_RELATIVE_PERMEABILITY" : 0.0001 | ||
| }, | ||
| "Tables": {} |
There was a problem hiding this comment.
More nitpicking: I believe we can also get rid of this input parameter.
| @@ -0,0 +1,80 @@ | |||
| # Test line loads in staged analysis | |||
|
|
|||
| **Author:** [Anne van de Graaf](https://github.com/avdg81) | |||
There was a problem hiding this comment.
Please feel free to replace my name with yours. Or, if you feel that is too much, add your name next to mine.
There was a problem hiding this comment.
Added my name because the main part is yours.
| if not Kratos.Registry.HasItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis"): | ||
| Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ClassName", "GeoMechanicsAnalysis") | ||
| Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ModuleName", "KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis") |
There was a problem hiding this comment.
I'm wondering whether these lines should be at test level, or that we should incorporate them elsewhere in the GeoMechanicsApplication code (although I'm not sure exactly where).
There was a problem hiding this comment.
removed following your suggestion below.
There was a problem hiding this comment.
This file is a copy of line_loads_in_stages/MaterialParameters.json. Also the file test.mdpa is a copy of the corresponding file in line_loads_in_stages/test.mdpa. I'm wondering whether it would be possible to share these files (i.e. to avoid having any copies)? Perhaps we could have a common subdirectory in line_loads_in_stages, where we move these two files to. And then we could have two more subdirectories in line_loads_in_stages: e.g. legacy and orchestrator. Those two subdirectories then contain the required ProjectParameters.json files. Such a structure also makes it clearer that for the orchestrator-based multi-stage analysis, we can keep the .mdpa and MaterialParameters.json files as-is (compared to the "legacy" approach).
There was a problem hiding this comment.
Thank you. The structure is done like that
| if not Kratos.Registry.HasItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis"): | ||
| Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ClassName", "GeoMechanicsAnalysis") | ||
| Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ModuleName", "KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis") |
There was a problem hiding this comment.
I would suggest to put these lines elsewhere, to avoid that we need to include them in each and every test file. Following along the lines as described in docs/pages/Kratos/For_Developers/General/Registry.md, I have added the following lines to applications/GeoMechanicsApplication/python_scripts/geomechanics_analysis.py (just before the line containing if __name__ == '__main__':):
# For orchestrator-based multi-stage analyses, we need to register our own analysis stage class
Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ClassName", "GeoMechanicsAnalysis")
Kratos.Registry.AddItem("Stages.KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis.GeoMechanicsAnalysis.ModuleName", "KratosMultiphysics.GeoMechanicsApplication.geomechanics_analysis")Subsequently, I removed the above lines from the test script, rebuilt the Kratos project, and reran the test and it all worked. In this way, we can keep our test scripts cleaner and we don't need to request our end-users to add these lines to their own scripts.
There was a problem hiding this comment.
Thank you for the suggestion and checking. Done.
| orchestrator_reg_entry = Kratos.Registry[project.GetSettings()["orchestrator"]["name"].GetString()] | ||
| orchestrator_module = importlib.import_module(orchestrator_reg_entry["ModuleName"]) | ||
| orchestrator_class = getattr(orchestrator_module, orchestrator_reg_entry["ClassName"]) | ||
| orchestrator_instance = orchestrator_class(project) | ||
| orchestrator_instance.Run() |
There was a problem hiding this comment.
We will probably need these lines for any future tests with the orchestrator, too. I can imagine that by then we wrap them in a helper function, which we'll put in test_helper.py. Not for now, but something to keep in mind.
Perhaps the same applies to lines 136 to 142, which essentially creates a Project instance.
|
Hi @rubenzorrilla , here is our first case with the orchestrator. Please could you have a brief look at it? Perhaps, you may suggest some improvements. Thank you. |
📝 Description
A brief description of the PR.