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

Geo/line piping element #12770

Merged
merged 38 commits into from
Oct 23, 2024
Merged

Geo/line piping element #12770

merged 38 commits into from
Oct 23, 2024

Conversation

WPK4FEM
Copy link
Contributor

@WPK4FEM WPK4FEM commented Oct 21, 2024

📝 Description
Based on the line Pw element a minimal line piping element is created.

  • basic setup, with permeability derived directly from PIPE_HEIGHT
  • unit tests added for whole public interface

Todo

  • documentation
  • publish element
  • integration tests
  • add 3 node and 3D 2 and 3 node versions

@WPK4FEM WPK4FEM requested a review from avdg81 October 21, 2024 11:47
@WPK4FEM WPK4FEM self-assigned this Oct 21, 2024
@WPK4FEM WPK4FEM linked an issue Oct 21, 2024 that may be closed by this pull request
Comment on lines 203 to 223
BoundedMatrix<double, TNumNodes, TNumNodes> CalculatePermeabilityMatrix(
const GeometryType::ShapeFunctionsGradientsType& rShapeFunctionGradients,
const Vector& rIntegrationCoefficients) const
{
RetentionLaw::Parameters RetentionParameters(GetProperties());
const auto& r_properties = GetProperties();
const double dynamic_viscosity_inverse = 1.0 / r_properties[DYNAMIC_VISCOSITY];

auto constitutive_matrix = FillPermeabilityMatrix(r_properties[PIPE_HEIGHT]);

auto result = BoundedMatrix<double, TNumNodes, TNumNodes>{ZeroMatrix{TNumNodes, TNumNodes}};
for (unsigned int integration_point_index = 0;
integration_point_index < GetGeometry().IntegrationPointsNumber(GetIntegrationMethod());
++integration_point_index) {
result += GeoTransportEquationUtilities::CalculatePermeabilityMatrix<TDim, TNumNodes>(
rShapeFunctionGradients[integration_point_index], dynamic_viscosity_inverse,
constitutive_matrix, 1.0, rIntegrationCoefficients[integration_point_index]);
}

return result;
}
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 experiment branch with calculators (see #12763), the calculation of the permeability matrix is delegated to these new calculators. Here, the calculation is of course slightly different. In the longer term, should we have a version of the permeability calculator that does it slightly different for piping vs 'normal', or should we handle that differently?

Looping @avdg81

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

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

This looks like a great start to introduce a line element for piping analysis. I have a few questions and several suggestions, but none of them is major.

Comment on lines 63 to 66
PointerVector<Node> nodes;
nodes.push_back(r_model_part.CreateNewNode(0, 0.0, 0.0, 0.0));
nodes.push_back(r_model_part.CreateNewNode(1, 1.0, 0.0, 0.0));
const auto p_geometry = std::make_shared<Line2D2<Node>>(nodes);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be possible to reuse the first helper function here (i.e. CreateNodes)...? I see there is a minor difference: here you explicitly request the model part to create new nodes, whereas the helper function creates "free" nodes. Not sure whether that is a problem.

Suggested change
PointerVector<Node> nodes;
nodes.push_back(r_model_part.CreateNewNode(0, 0.0, 0.0, 0.0));
nodes.push_back(r_model_part.CreateNewNode(1, 1.0, 0.0, 0.0));
const auto p_geometry = std::make_shared<Line2D2<Node>>(nodes);
const auto p_geometry = std::make_shared<Line2D2<Node>>(CreateNodes());

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

Comment on lines 75 to 78
PointerVector<Node> nodes;
nodes.push_back(r_model_part.CreateNewNode(1, 0.0, 0.0, 0.0));
nodes.push_back(r_model_part.CreateNewNode(2, 1.0, 0.0, 0.0));
const auto p_geometry = std::make_shared<Line2D2<Node>>(nodes);
Copy link
Contributor

Choose a reason for hiding this comment

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

If you believe that the previous suggestion makes sense, then you may want to apply it here, too:

Suggested change
PointerVector<Node> nodes;
nodes.push_back(r_model_part.CreateNewNode(1, 0.0, 0.0, 0.0));
nodes.push_back(r_model_part.CreateNewNode(2, 1.0, 0.0, 0.0));
const auto p_geometry = std::make_shared<Line2D2<Node>>(nodes);
const auto p_geometry = std::make_shared<Line2D2<Node>>(CreateNodes());

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

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

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

This PR looks ready to go to me.

@WPK4FEM WPK4FEM enabled auto-merge (squash) October 23, 2024 08:43
@WPK4FEM WPK4FEM merged commit 698b64a into master Oct 23, 2024
9 of 11 checks passed
@WPK4FEM WPK4FEM deleted the geo/line_piping_element branch October 23, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Add Geo_steadystatePwPipingElement2D2N
3 participants