Geo/14021 interface fluid body flow calculator#14215
Geo/14021 interface fluid body flow calculator#14215
Conversation
…ationPoint for Pw_element.
# Conflicts: # applications/GeoMechanicsApplication/custom_elements/U_Pw_interface_element.h # applications/GeoMechanicsApplication/custom_utilities/element_utilities.hpp
…permeability matrix
Added use of fluid body flow calculator and the projected gravity vector needed for it.
I'm missing a - sing somewhere, because now PermeabilityFlow and FluidBodyFlow are equal where they should be opposite.
…d body flow vector. Now they are in an unexpected order for me.
Added unit test for interface at 90 deg with gravity.
additional tests
rfaasse
left a comment
There was a problem hiding this comment.
Thank you for connecting the fluid body flow and permeability calculators to the interface! I did pay slightly less attention to the functions I expect will also be merged back from master (due to the overlap with the coupling) since they've been reviewed before. I didn't see any major issues, just have some minor suggestions/questions.
applications/GeoMechanicsApplication/custom_elements/U_Pw_interface_element.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_elements/U_Pw_interface_element.cpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_elements/U_Pw_interface_element.h
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_elements/U_Pw_interface_element.cpp
Show resolved
Hide resolved
| {12, array_1d<double, 3>{0.0, 0.0, 0.0}}, {13, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {14, array_1d<double, 3>{0.0, 0.0, 0.0}}, {15, array_1d<double, 3>{0.0, 0.0, 0.0}}}; | ||
|
|
||
| // verbeteren, dit zijn indexen en geen knoopnummers |
There was a problem hiding this comment.
I think this comment is no longer accurate
There was a problem hiding this comment.
This was corrected and the comment removed.
| {12, array_1d<double, 3>{0.0, 0.0, 0.0}}, {13, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {14, array_1d<double, 3>{0.0, 0.0, 0.0}}, {15, array_1d<double, 3>{0.0, 0.0, 0.0}}}; | ||
|
|
||
| // verbeteren, dit zijn indexen en geen knoopnummers |
There was a problem hiding this comment.
Same here, I think the comment can be removed
| const auto prescribed_displacements = PrescribedDisplacements{ | ||
| {0, array_1d<double, 3>{0.0, 0.0, 0.0}}, {1, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {2, array_1d<double, 3>{0.0, 0.0, 0.0}}, {3, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {4, array_1d<double, 3>{0.0, 0.0, 0.0}}, {5, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {6, array_1d<double, 3>{0.0, 0.0, 0.0}}, {7, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {8, array_1d<double, 3>{0.0, 0.0, 0.0}}, {9, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {10, array_1d<double, 3>{0.0, 0.0, 0.0}}, {11, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {12, array_1d<double, 3>{0.0, 0.0, 0.0}}, {13, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {14, array_1d<double, 3>{0.0, 0.0, 0.0}}, {15, array_1d<double, 3>{0.0, 0.0, 0.0}}}; |
There was a problem hiding this comment.
We could probably leave this part out (I believe displacements are initialized as zeros by default)
There was a problem hiding this comment.
It fills the 1st of 3 optional arguments, I wanted to fill it clearly when the following optional arguments are needed.
|
|
||
| for (const auto& [idx, disp] : prescribed_displacements) { |
There was a problem hiding this comment.
We might be able to make this for-loop a simple (probably templated) utility within this test file, such that we don't need to repeat the for-loop this often 👍
There was a problem hiding this comment.
Added a utility function SetVariableOnGeometry in the anonymous namespace and used it throughout this cpp file
| constexpr auto number_of_pw_dofs = std::size_t{8}; | ||
| const auto expected_u_block_vector = Vector{number_of_u_dofs, 0.0}; | ||
| AssertUBlockVectorIsNear(actual_right_hand_side, expected_u_block_vector, number_of_u_dofs, number_of_pw_dofs); | ||
| // The fluid body flow of a vertical interface subjected to vertical gravity should be zero |
There was a problem hiding this comment.
Based on the title of this test, I would expect this to be described as a horizontal interface
| // The fluid body flow of a vertical interface subjected to vertical gravity should be zero | |
| // The fluid body flow of a horizontal interface subjected to vertical gravity should be zero |
There was a problem hiding this comment.
Changed, thank you for the sharp observation.
…-fluid-body-flow-calculator Also resolved several merge conflicts.
452f9d4 to
f54a696
Compare
avdg81
left a comment
There was a problem hiding this comment.
Hi Wijtze Pieter,
Thanks a lot for all the hard work that you've done to include the fluid body flow and the permeability contribution. I have several suggestions, but all of them are minor. It's perfectly fine if you'd rather pick up some of the suggestions in a separate PR. I'll leave that up to you.
| #include "contribution_calculators/fluid_body_flow_calculator.hpp" | ||
| #include "contribution_calculators/permeability_calculator.hpp" |
There was a problem hiding this comment.
These two includes have also been added to the header file of the U-Pw interface element. Therefore, it's not needed to repeat them here.
There was a problem hiding this comment.
Removed all contribution_calculators includes from the cpp file.
| const auto number_of_dofs = GetDofs().size(); | ||
| rLeftHandSideMatrix = ZeroMatrix{number_of_dofs, number_of_dofs}; | ||
| const auto ignore_undrained = | ||
| this->GetProperties().Has(IGNORE_UNDRAINED) ? this->GetProperties()[IGNORE_UNDRAINED] : false; |
There was a problem hiding this comment.
Nitpicking: this-> is redundant here.
| this->GetProperties().Has(IGNORE_UNDRAINED) ? this->GetProperties()[IGNORE_UNDRAINED] : false; | |
| GetProperties().Has(IGNORE_UNDRAINED) ? GetProperties()[IGNORE_UNDRAINED] : false; |
In addition, we may consider to put this logic in a local helper function, e.g. GetIgnoreUndrained. We could start by adding it to the anonymous namespace. If we later find we also need it elsewhere, we can move it to a utility function class.
There was a problem hiding this comment.
Tried a function with the properties as argument.
|
|
||
| void UPwInterfaceElement::CalculateAndAssignPermeabilityMatrix(Element::MatrixType& rLeftHandSideMatrix) | ||
| { | ||
| switch (GetGeometry().PointsNumber()) { |
There was a problem hiding this comment.
I would suggest to be specific about the geometry that we mean: either the one for the displacement field (GetDisplacementGeometry) or the one for the water pressure field (GetWaterPressureGeometry). GetGeometry refers to the displacement geometry. I'm not sure whether that's what you intended...?
Furthermore, I would suggest to rewrite the switch statement using a map, as was done before for other member function, e.g. CalculateAndAssignUPCouplingMatrix.
There was a problem hiding this comment.
Now using the water pressure geometry and did it in the form of a map.
| case 6: | ||
| CalculateAndAssemblePermeabilityFlowVector<6>(rRightHandSideVector); |
There was a problem hiding this comment.
This suggests that the interface geometry (either a 3+3 line or a 3+3 surface) is irrelevant here; it's just the number of nodes that matters. Am I correct in saying that?
There was a problem hiding this comment.
Its just the number of Pw D.o.F.
| r_water_pressure_mid_geometry.ShapeFunctionsLocalGradients(shape_functions_local_gradient, rIntegrationPoint); | ||
| // local derivative | ||
| shape_functions_local_gradient /= r_water_pressure_mid_geometry.DeterminantOfJacobian(rIntegrationPoint); |
There was a problem hiding this comment.
This code suggests that ShapeFunctionsLocalGradients returns gradients that include the determinants of the Jacobian matrices, which apparently is undesirable here. I was just wondering whether class Geometry perhaps offers API that returns those gradients instead, just so we don't need to correct for that here?
There was a problem hiding this comment.
Looked in e.g. line_2d_3.h, but did not find such a gradient.
| {12, array_1d<double, 3>{0.0, 0.0, 0.0}}, {13, array_1d<double, 3>{0.0, 0.0, 0.0}}, | ||
| {14, array_1d<double, 3>{0.0, 0.0, 0.0}}, {15, array_1d<double, 3>{0.0, 0.0, 0.0}}}; | ||
|
|
||
| // verbeteren, dit zijn indexen en geen knoopnummers |
There was a problem hiding this comment.
Does this (Dutch) comment still apply?
| const auto prescribed_volume_accelerations = std::vector<std::pair<std::size_t, array_1d<double, 3>>>{ | ||
| {0, array_1d<double, 3>{0.0, 0.0, -10.0}}, {1, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {2, array_1d<double, 3>{0.0, 0.0, -10.0}}, {3, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {4, array_1d<double, 3>{0.0, 0.0, -10.0}}, {5, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {6, array_1d<double, 3>{0.0, 0.0, -10.0}}, {7, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {8, array_1d<double, 3>{0.0, 0.0, -10.0}}, {9, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {10, array_1d<double, 3>{0.0, 0.0, -10.0}}, {11, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {12, array_1d<double, 3>{0.0, 0.0, -10.0}}, {13, array_1d<double, 3>{0.0, 0.0, -10.0}}, | ||
| {14, array_1d<double, 3>{0.0, 0.0, -10.0}}, {15, array_1d<double, 3>{0.0, 0.0, -10.0}}}; |
There was a problem hiding this comment.
Instead of defining the 3D acceleration vector many times, we could define is just once (make it a static const variable, e.g. gravity_acceleration_3d). We could make it even nicer by having a nonmember factory function that receives a list of node IDs and a vector to be associated with each node, which then produces these prescribed volume accelerations. Perhaps that is a bit too much for now, so feel free to pick it up in a separate PR.
There was a problem hiding this comment.
Did so in many places.
| // The fluid body flow of a vertical interface subjected to vertical gravity should be zero | ||
| const auto expected_p_block_vector = Vector{number_of_pw_dofs, 0.0}; | ||
|
|
||
| KRATOS_INFO("Actual RHS") << actual_right_hand_side << std::endl; |
There was a problem hiding this comment.
I think this is a left-over from a debugging session?
| // The fluid body flow of a vertical interface subjected to vertical gravity should be zero | ||
| const auto expected_p_block_vector = Vector{number_of_pw_dofs, 0.0}; | ||
|
|
||
| KRATOS_INFO("Actual RHS") << actual_right_hand_side << std::endl; |
There was a problem hiding this comment.
I assume this print statement is no longer needed.
| constexpr auto number_of_pw_dofs = std::size_t{8}; | ||
| const auto expected_u_block_vector = Vector{number_of_u_dofs, 0.0}; | ||
| AssertUBlockVectorIsNear(actual_right_hand_side, expected_u_block_vector, number_of_u_dofs, number_of_pw_dofs); | ||
| auto flow = (1.0 - 0.5 * sqrt(3.0)) * 62.5; |
There was a problem hiding this comment.
Nitpicking:
| auto flow = (1.0 - 0.5 * sqrt(3.0)) * 62.5; | |
| const auto flow = (1.0 - 0.5 * sqrt3) * 62.5; |
…-fluid-body-flow-calculator' into geo/14021-interface-fluid-body-flow-calculator
Compute components once and reuse.
…it anyway. ( Review comment Anne )
Gave repeated values a name. Used AssertRhsVectorBlocksAreNear ( once, more to come )
Gave more repeated values a name. Used more AssertRhsVectorBlocksAreNear ( not yet possible on limited precision regression values )
Renamed types Corrected quadrilateral typo's
…Near Gave some values machine precision. Name more repeatedly used values.
📝 Description
Use of permeability and fluid body flow calculator on interface element
🆕 Changelog