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

[GeoMechanicsApplication] Remove extra redundant calculations of displacement for thermal and Pw elements #12462

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

mnabideltares
Copy link
Contributor

📝 Description
In the solver, functions for displacement are called which are not necessary for thermal and Pw elements. These functions cause a lot of redundant calculations and significantly slow down the process.

Moreover, in the most of test cases related to thermal and Pw elements, "reset_displacement=true" is used. It came probably from GID generated files. This is redundant for such elements and need to be removed.

🆕 Changelog

  • Modified the solver
  • Modified the test cases (thermal and Pw) to remove unnecessary options.

…Pw elements

Adjusted the integration tests accordingly
Copy link
Contributor

@markelov208 markelov208 left a comment

Choose a reason for hiding this comment

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

Hi Mohamed, very nice that you have been able to speed up the tests so drastically.

Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

Hi Mohammed,

Thank you for the analysis and the following tedious work. With most I'm very happy.
Please have another look at what was done for calculate_reactions, Removing the item probably has no effect as it defaults to true in a lot of places.

Comment on lines 34 to 35
"scheme_type": "Backward_Euler",
"reset_displacements": true,
"newmark_beta": 0.25,
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed it can be deleted, the defaults in geomechanics_U_Pw_solver.py and in geomechanics_solver.py are both false.
( In future we probably should make sure there is only 1 place where this setting has its default. )

@@ -61,7 +60,6 @@
"min_alpha": 0.1,
"max_alpha": 2.0,
"line_search_tolerance": 0.5,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, deletion is correct, , the defaults in geomechanics_U_Pw_solver.py and in geomechanics_solver.py are both false.
( In future we probably should make sure there is only 1 place where this setting has its default. )

@@ -32,7 +32,6 @@
"block_builder" : true,
"solution_type" : "Transient_Groundwater_Flow",
"scheme_type" : "Backward_Euler",
Copy link
Contributor

Choose a reason for hiding this comment

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

Already false, but I agree with removing items that have no meaning for the analysis.

@@ -52,14 +51,12 @@
"desired_iterations" : 2,
"max_radius_factor" : 10.0,
"min_radius_factor" : 0.1,
"calculate_reactions" : true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentially deleted? In thermal analysis, I expect the reaction computation to compute nodal reaction flows at the Dirichlet boundary conditions and zero nodal flow and other nodes that are without Dirichlet and Neumann boundary conditions.

Furhter, I think remove the "calculate_reactions" : true, has no effect. The defaults in geomechanics_U_Pw_solver.py and geomechanics_solver.py, geomechanics_Pw_solver.py and geomechanics_T_solver.py are all true.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with deleting this file, the ProjectParameters file points to another place for the material.

@mnabideltares mnabideltares merged commit 89a88ba into master Jun 14, 2024
11 checks passed
@mnabideltares mnabideltares deleted the geo/12451-thermal-performance branch June 14, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Investigate thermal simulation performance
3 participants