-
Notifications
You must be signed in to change notification settings - Fork 252
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 the missing conditions to Geomechanics Application for 2D #11625
Conversation
…odified. CalculateIntegrationCoefficient's in UPwFaceLoadCondition are squashed to be a single function
UPwNormalFaceLoadCondition is modified
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codewise this is quite an improvement, the formulas have been written down using vector multiplications i.s.o. writing down component multiplication and summation.
However, new conditions have been added without adding tests for the new conditions. These have to be made.
Further, run clang format on the changed code.
# Conflicts: # applications/GeoMechanicsApplication/custom_conditions/Pw_condition.cpp
…on-working conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mohamed, just a small comment on READMe file but it is not blocking.
applications/GeoMechanicsApplication/tests/test_conditions/README.md
Outdated
Show resolved
Hide resolved
minor
Minor fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Mohamed and Gennady for all improvements.
Some small things left.
...ations/GeoMechanicsApplication/tests/test_conditions/documentation_data/load_flux_domain.svg
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/test_conditions/README.md
Outdated
Show resolved
Hide resolved
...tests/test_conditions/test_AxisymmetricUPwNormalFaceLoadCondition2D4N/ProjectParameters.json
Show resolved
Hide resolved
...tests/test_conditions/test_AxisymmetricUPwNormalFaceLoadCondition2D5N/ProjectParameters.json
Show resolved
Hide resolved
...ation/tests/test_conditions/test_LineNormalLoadDiffOrderCondition2D3N/ProjectParameters.json
Show resolved
Hide resolved
...ation/tests/test_conditions/test_LineNormalLoadDiffOrderCondition2D4N/ProjectParameters.json
Show resolved
Hide resolved
...ation/tests/test_conditions/test_LineNormalLoadDiffOrderCondition2D5N/ProjectParameters.json
Show resolved
Hide resolved
...Application/tests/test_conditions/test_UPwNormalFaceLoadCondition2D4N/ProjectParameters.json
Show resolved
Hide resolved
...Application/tests/test_conditions/test_UPwNormalFaceLoadCondition2D3N/ProjectParameters.json
Show resolved
Hide resolved
...Application/tests/test_conditions/test_UPwNormalFaceLoadCondition2D2N/ProjectParameters.json
Show resolved
Hide resolved
…com/KratosMultiphysics/Kratos into geo/11616-add-missing-conditions-2d
This keyword is needed, the code gives error if this keyword is not found
...ations/GeoMechanicsApplication/tests/test_conditions/documentation_data/load_flux_domain.svg
Outdated
Show resolved
Hide resolved
...tests/test_conditions/test_AxisymmetricUPwNormalFaceLoadCondition2D4N/ProjectParameters.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work in incorporating these changes @mnabideltares and @markelov208! This PR improves the code quality a lot!
There are a couple of minor suggestions and questions left from my side and there is a (very minor) merge conflict with master that still needs to be fixed (it's about added includes that should just both be included), but I think we're quite close now to getting this done!
applications/GeoMechanicsApplication/tests/test_conditions/README.md
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/test_conditions/README.md
Outdated
Show resolved
Hide resolved
@@ -80,7 +81,8 @@ def AssembleTestSuites(): | |||
KratosGeoMechanicsChangingWaterLevelTests, | |||
KratosGeoMechanicsStrainMeasureTests, | |||
KratosGeoMechanicsSetMultipleMovingLoadProcessTests, | |||
KratosGeoMechanicsRotationWithMovingLoadTests | |||
KratosGeoMechanicsRotationWithMovingLoadTests, | |||
KratosGeoMechanicsConditionTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, how close we already are to the limit, do you know @avdg81?
applications/GeoMechanicsApplication/custom_conditions/U_Pw_face_load_interface_condition.cpp
Show resolved
Hide resolved
...s/GeoMechanicsApplication/custom_conditions/axisymmetric_U_Pw_normal_face_load_condition.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My stack of remarks has been processed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work, looks good to me!
📝 Description
The conditions are not completely added to the GeoMechanics Application. For example, PwCondition is defined on 2 noded element (in 2D) but it is not extended to 3 noded element. There are also missing higher order condition elements in 3D.
Here we fix the 2D conditions. The 3D conditions will be done in a seperate issue in order to keep this commit managable
🆕 Changelog
** Acceptance