[GeoMechanicsApplication] Fix standard k0 procedure for C++ workflow#12321
[GeoMechanicsApplication] Fix standard k0 procedure for C++ workflow#12321
Conversation
…ialization of the processes)
| rProjectParameters["solver_settings"], GetComputationalModelPart()); | ||
| KRATOS_ERROR_IF_NOT(solving_strategy) << "No solving strategy was created!" << std::endl; | ||
|
|
||
| solving_strategy->Initialize(); |
There was a problem hiding this comment.
In the end, this was the culprit, the too early initialization (before the processes were initialized).
There was a problem hiding this comment.
I'm not sure yet what would be the best way to add a python test for this settlement workflow and what to assert. Probably the 'whole-file' approach we have for c++ isn't the best, however, I'd like to keep the c++ and python route as coupled as possible so we minimize our chances of this happening again. Let's discuss this next week @avdg81 @WPK4FEM
…ement-route-differences
…rances, but still using the common res files with the C++ test.
applications/GeoMechanicsApplication/tests/test_GeoMechanicsApplication.py
Show resolved
Hide resolved
WPK4FEM
left a comment
WPK4FEM
left a comment
There was a problem hiding this comment.
Dear Richard,
It's nice that both routes are properly compared to ensure similar behaviour. Material parameters and the model are really shared, The projectparameters files are equal, but 2 copies exist. Thinking more about that, I prefer that they too are shared.
Thank you for the concise effort.
Wijtze Pieter
| self.assertAlmostEqual(actual_nodal_values[j][0], expected_nodal_values[j][0], 3) | ||
| self.assertAlmostEqual(actual_nodal_values[j][1], expected_nodal_values[j][1], 3) | ||
| self.assertAlmostEqual(actual_nodal_values[j][2], expected_nodal_values[j][2], 3) |
There was a problem hiding this comment.
These 3 together form a vector comparison, maybe group them in a function. In this case the 3rd component is easy, as it is 0 for the 2D plane strain computation.
There was a problem hiding this comment.
Done (seems there is already a function for it)
applications/GeoMechanicsApplication/tests/test_GeoMechanicsApplication.py
Show resolved
Hide resolved
avdg81
left a comment
avdg81
left a comment
There was a problem hiding this comment.
Thanks for solving this issue. The implemented solution is simple and well-tested. In particular, I like the way you check whether the C++ route and the Python route still yield the same results. Well done! I have several minor remarks, none of them is blocking.
applications/GeoMechanicsApplication/tests/settlement_workflow.py
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/settlement_workflow.py
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/settlement_workflow.py
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/settlement_workflow.py
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/settlement_workflow.py
Outdated
Show resolved
Hide resolved
...chanicsApplication/tests/test_settlement_workflow/cpp_workflow/ProjectParameters_stage1.json
Show resolved
Hide resolved
...ions/GeoMechanicsApplication/tests/test_settlement_workflow/python_workflow/Kratos_stages.py
Outdated
Show resolved
Hide resolved
...ions/GeoMechanicsApplication/tests/test_settlement_workflow/python_workflow/Kratos_stages.py
Show resolved
Hide resolved
… and runs the test there.
|
@WPK4FEM @avdg81 Processed the review comments and removed the duplication of inputs. The way it's solved is: the python test runs in the original location and the C++ copies the original and creates a temporary working dir. If the test fails, the working dir will be there for inspection. If the test passes, the tmp dir is cleaned up. |
WPK4FEM
left a comment
WPK4FEM
left a comment
There was a problem hiding this comment.
Thank you for removing the duplication of the .json files.
Only some nagging left about the similarity of the construction of the file names.
| result_file_name = os.path.join(file_path, f'test_model_stage{i+1}.post.res') | ||
| expected_result_file_name = (test_helper.get_file_path( | ||
| os.path.join(test_name, f'test_model_stage{i+1}.post.orig.res'))) |
There was a problem hiding this comment.
Expected:
expected_result_file_name = os.path.join(file_path, f'test_model_stage{i+1}.post.res')
very similar to line 20.
avdg81
left a comment
avdg81
left a comment
There was a problem hiding this comment.
I believe we're good to go. Thanks for the work you have put into this, Richard!
3 participants
📝 Description
The standard k0 procedure, created in this PR did not work for our C++ settlement workflow. The reason was that the solving strategy was initialized too early (before the k0 process was initialized), meaning that the standard k0 procedure flag was not correctly applied to the model. This PR fixes that issue and revalidates the c++ settlement test.