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] Revisit and document test element lab #11975

Merged
merged 28 commits into from
Feb 13, 2024

Conversation

@rfaasse rfaasse added Documentation GeoMechanics Issues related to the GeoMechanicsApplication labels Jan 23, 2024
@rfaasse rfaasse self-assigned this Jan 23, 2024
@rfaasse rfaasse linked an issue Jan 23, 2024 that may be closed by this pull request
@rfaasse rfaasse marked this pull request as ready for review January 23, 2024 12:56
@rfaasse
Copy link
Contributor Author

rfaasse commented Jan 23, 2024

Not all codacy issues make sense to fix. There are some that change how the md is rendered, and the current layout is as intended

@rfaasse rfaasse requested a review from WPK4FEM January 23, 2024 12:57
@rfaasse rfaasse requested a review from mcgicjn2 January 23, 2024 13:04
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.

Having a description that includes pictures of the mesh and boundary conditions is very illustrative


## Assertions
In this test, there are two results checks:
1. It is asserted that the effective stress is -1000000 kPa (?) in the integration points of both elements in the y direction and 0 in the other directions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and on line 16 the (?) can be removed.
0. stress in other directions must be a result of a 0. Poisson's ratio.

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! And added the bit about the Poisson ratio to clarify this.

Copy link
Contributor Author

@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.

@WPK4FEM Thanks for your thorough review, I processed most, there are a few discussion points left and the matter of moving part of the descriptions to the image

@rfaasse rfaasse requested a review from WPK4FEM January 24, 2024 15:40
WPK4FEM
WPK4FEM previously approved these changes Jan 24, 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.

Nice. Done.

- The X and Y displacement in the bottom two nodes (3 and 4) are fixed.
- The X displacement in all nodes is fixed.
- Material:
- The material is described using a linear elastic material with a GeoLinearElasticPlaneStrain2DLaw.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add the Young's Modulus here, E, here, as this will enable users to assert the displacement below. Due to being Linear Elastic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This indeed clarifies a lot, added, thanks for the suggestion!

1. It is asserted that the effective stress is -1000000 kPa in the integration points of both elements in the y
direction and 0.0 in the other directions, due to the Poisson ration being 0.0.
2. It is asserted that the displacement of one of the top nodes (1) is close to -0.0909090909516868 as a result of the
applied line load.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you could add the displacement calculation due to Youngs Modulus, E.

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 did it for the diff order, for this one, we'll first check if we can get this to linear elastic behavior


1. It is asserted that the effective stress is -1000000 kPa in the integration points of both elements in the y
direction and 0.0 in the other directions, due to the Poisson ration being 0.0.
2. It is asserted that the displacement of one of the top nodes (1) is close to -0.0909090909516868 as a result of the
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this is correct -0.0909090909516868 (I think it should be -0.1 = -1.e6/1.e7)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We did a bit more digging here and it turns out it is due to the non-linear behavior of the geometry. Here we have a plot of displacement vs time, where the load ramps up from 0 to -100000000 in this time interval. Since we are using the updatedlagrange element, we see that the displacement is non-linear.
image

When setting the move_mesh flag to false (making the geometry more linear) we get the following graph:
image

The reason the diff order element approximately behaved as it should, was because the load was a lot smaller, meaning the displacement was still in the linear region.

For now we propose to keep the result like it is now. I added a line to explain this in the README.md to explain the number.

FYI @WPK4FEM

Copy link
Contributor

Choose a reason for hiding this comment

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

Measurements should be added to the diagrams or it should be clearly stated as a unit square in the README.md

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 suggestion, added to the diagrams!

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the result variables from the ProjectParameters.json in oedometer_ULFEM test + oedometer_ULFEM_diff_order test (well in one of them !)

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, I indeed missed one

Copy link
Contributor

@mcgicjn2 mcgicjn2 left a comment

Choose a reason for hiding this comment

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

Forgot to tick request changes

@rfaasse rfaasse requested review from mcgicjn2 and WPK4FEM January 29, 2024 08:02
@rfaasse rfaasse enabled auto-merge (squash) January 31, 2024 15:11
@rfaasse rfaasse merged commit d3917a3 into master Feb 13, 2024
15 of 17 checks passed
@rfaasse rfaasse deleted the geo/11969-revisit-and-document-test_element_lab branch February 13, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation GeoMechanics Issues related to the GeoMechanicsApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Revisit test_element_lab.py
3 participants