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] Add multi-stage tests for the new line interface elements #12705

Merged

Conversation

avdg81
Copy link
Contributor

@avdg81 avdg81 commented Sep 30, 2024

📝 Description

Added two Python tests that cover the behavior of the new line interface element in a two-stage scenario. The first test uses Dirichlet boundary conditions, whereas the second one uses Neumann boundary conditions. Both tests carry out two time steps per stage.

Note that this PR needs to wait for another PR to be merged first, since that PR contains a fix that has been duplicated here. Once it has become available on master, we can remove it from this PR.

So far, the test comprises a single stage with a single time step. It doesn't include any initial relative displacement nor any initial traction.
The prescribed displacements are now interpolated from two tables (one for each direction). The expected values have been updated, too.
So far, this test consists of a single stage with two time steps.
To make it work I had to copy a fix from another PR. Once that one has been merged, we can revert the local copy here.
- Renamed a test case to avoid having non-unique names.
- The interface normal stiffness and the interface shear stiffness are now attributes of the test case class.
@avdg81 avdg81 added the GeoMechanics Issues related to the GeoMechanicsApplication label Sep 30, 2024
@avdg81 avdg81 self-assigned this Sep 30, 2024
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.

Fast first look.

…ommon one

As a result, the name of one model part had to be changed ("Interface" => "Interfaces").
As a result, the new helper function could be removed (it was copied from another PR).
As a result, the new helper function could be removed (it was copied from another PR).
Also removed an accidentally added blank line.
@avdg81 avdg81 marked this pull request as ready for review September 30, 2024 12:35
@avdg81 avdg81 requested a review from WPK4FEM September 30, 2024 14:08
WPK4FEM
WPK4FEM previously approved these changes Sep 30, 2024
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.

Minor typographical comments left, nothing blocking.

"scaling": false
},
"problem_domain_sub_model_part_list": ["Interfaces"],
"processes_sub_model_part_list": ["FixedNodes","AllNodes", "DisplacedNodes"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Either no space after comma, or always a space after a comma. Preferably not mixed as it is here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Fixed.

"scaling": false
},
"problem_domain_sub_model_part_list": ["Interfaces"],
"processes_sub_model_part_list": ["FixedNodes","AllNodes", "DisplacedNodes"],
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment about comma and space in previous projectparameters file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you.

"scaling": false
},
"problem_domain_sub_model_part_list": ["Interfaces"],
"processes_sub_model_part_list": ["FixedNodes","AllNodes", "LoadedSide"],
Copy link
Contributor

Choose a reason for hiding this comment

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

also here comma and space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

"scaling": false
},
"problem_domain_sub_model_part_list": ["Interfaces"],
"processes_sub_model_part_list": ["FixedNodes","AllNodes", "LoadedSide"],
Copy link
Contributor

Choose a reason for hiding this comment

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

no space, space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Thanks for adding these tests (and nice you used the setup method for the test class), I have a few suggestions left for duplication/clean-up, but other than that, this is good to go!

Comment on lines +15 to +16
self.normal_stiffness = 3.0e7
self.shear_stiffness = 1.5e7
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to re-use these in the single-stage, single-step tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course we can. Thanks, this makes the test code more consistent.

Comment on lines 62 to 71
"linear_solver_settings": {
"solver_type": "amgcl",
"smoother_type": "ilu0",
"krylov_type": "gmres",
"coarsening_type": "aggregation",
"max_iteration": 100,
"verbosity": 0,
"tolerance": 1.0e-6,
"scaling": false
},
Copy link
Contributor

Choose a reason for hiding this comment

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

In the end, I used this solver (a bit less of a hassle to set up and should do the same)

Suggested change
"linear_solver_settings": {
"solver_type": "amgcl",
"smoother_type": "ilu0",
"krylov_type": "gmres",
"coarsening_type": "aggregation",
"max_iteration": 100,
"verbosity": 0,
"tolerance": 1.0e-6,
"scaling": false
},
"linear_solver_settings": {
"solver_type": "LinearSolversApplication.sparse_lu"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. I have simplified the linear solver settings accordingly in the ProjectParameters.json files that have been added in this PR.

"buffer_size": 2,
"echo_level": 1,
"clear_storage": false,
"compute_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.

I think we could put this to false. I did some clean-up in the single stage tests (which is of course based of the same example), maybe you could diff and see which settings could be the same? The same holds then of course for the other jsons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have compared the single stage analysis files with those of the multi-stage analyses. For consistency, I have changed some flags for the multi-stage analyses which had no effect on the obtained results. Thanks for pointing this out.

@avdg81
Copy link
Contributor Author

avdg81 commented Oct 1, 2024

Minor typographical comments left, nothing blocking.

Thank you for your comments. I have processed them.

- Reuse attributes `normal_stiffness` and `shear_stiffness` in the single-stage tests.
- Extracted a method that asserts the results of a multi-stage test.
- Simplified the linear solver settings.
- Made the analysis flags more consistent with those of the single stage tests.
@avdg81
Copy link
Contributor Author

avdg81 commented Oct 1, 2024

Thanks for adding these tests (and nice you used the setup method for the test class), I have a few suggestions left for duplication/clean-up, but other than that, this is good to go!

Thank you very much for your thorough review. I have processed your suggestions to the best of my abilities. Could you please have another look, just to double-check that I didn't miss anything or misinterpreted a suggestion?

@avdg81 avdg81 requested a review from rfaasse October 1, 2024 08:57
Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Looks good to go to me!

@avdg81 avdg81 enabled auto-merge (squash) October 1, 2024 09:35
@avdg81 avdg81 merged commit ac2b1a3 into master Oct 1, 2024
9 of 11 checks passed
@avdg81 avdg81 deleted the geo/12693-add-multistage-tests-for-new-line-interface-elements branch October 1, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GeoMechanics Issues related to the GeoMechanicsApplication
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Add integration tests for the new line interface elements
3 participants