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

Adding new hydraulic utilities #12280

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Conversation

uxuech
Copy link
Contributor

@uxuech uxuech commented Apr 15, 2024

📝 Description
In this PR, new hydraulic utilities have been added, each thoroughly tested with corresponding tests. Below are the main changes:
🆕 Changelog

  • Corner detection to prevent velocity peaks.
  • Enabling the option to turn off gravity in air elements.
  • Hydraulic inlet under subcritical conditions (possibility of having a submerged inlet).
  • Assigning artificial viscosity to uncut elements (this has been moved from the hydraulic solver in order to optimize simulation times)."

});
}

void HydraulicFluidAuxiliaryUtilities::TurnOffGravityOnAirElements(ModelPart &rModelPart){
Copy link
Member

Choose a reason for hiding this comment

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

How do you turn it on when an element changes back from air to water?

}
if (FluidAuxiliaryUtilities::IsPositive(distances)){
for (unsigned int i_nodes = 0; i_nodes < r_geom.PointsNumber(); i_nodes++)
{
Copy link
Member

Choose a reason for hiding this comment

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

Please be consistent in the formatting, you either put the { in the same line or in a new one.

void HydraulicFluidAuxiliaryUtilities::FixCornerNodeVelocity(ModelPart &rModelPart, double MaximumAngle)
{
// Obtain for each condition each neighbor condition
block_for_each(rModelPart.Nodes(), [&](NodeType &rNode){ rNode.SetValue(AUX_INDEX, 0); });
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a method in the variable utils for doing this.

Comment on lines +336 to +337
double acceptable_cos = cos(angle_corner_rad);
double max_height = block_for_each<MaxReduction<double>>(rModelPart.Nodes(), [&](NodeType &rNode){return rNode.Z();});
Copy link
Member

Choose a reason for hiding this comment

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

I think both can be const.

Comment on lines +358 to +359
array_1d<double, 2> ids_neig;
array_1d<double, 2> ids_cond;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
array_1d<double, 2> ids_neig;
array_1d<double, 2> ids_cond;
array_1d<double, 2> ids_neig;
array_1d<double, 2> ids_cond;

Allocate these two outside the loop.

Comment on lines +373 to +375
double repeated = edgelist[edge][i].GetValue(AUX_INDEX);
repeated += 1;
edgelist[edge][i].SetValue(AUX_INDEX, repeated);
Copy link
Member

Choose a reason for hiding this comment

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

You can avoid the get/set by directly using the get with += 1.

}
}
block_for_each(rModelPart.Nodes(),[&](NodeType &rNode){
if (rNode.GetValue(AUX_INDEX)*0.5>=3 && rNode.Is(MARKER)){
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (rNode.GetValue(AUX_INDEX)*0.5>=3 && rNode.Is(MARKER)){
if (rNode.GetValue(AUX_INDEX)*0.5>=3 && rNode.Is(MARKER)){

Why this?

}
block_for_each(rModelPart.Nodes(),[&](NodeType &rNode){
if (rNode.GetValue(AUX_INDEX)*0.5>=3 && rNode.Is(MARKER)){
Vector veloctiy = ZeroVector(3);
Copy link
Member

Choose a reason for hiding this comment

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

Allocate the vector outside the loop.

Comment on lines +390 to +391
double value= rNode.GetValue(AUX_INDEX);
rNode.SetValue(AUX_INDEX,value*0.5);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment, you can avoid the get/set.


bool HydraulicFluidAuxiliaryUtilities::MaximumWaterDepthChange(const ModelPart &rModelPart){
// make sure the Ids of the Nodes start with one
const double &min_Z = block_for_each<MinReduction<double>>(rModelPart.Nodes(), [&](const auto &rNode){ return rNode.Z(); });
Copy link
Member

Choose a reason for hiding this comment

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

Why taking a reference in here?

if (std::abs(rNode.Z() - min_Z) < 1.0e-10 && !maximum_water_depth_change){
const double inlet_phi = rNode.GetValue(AUX_DISTANCE);
const double domain_phi = rNode.FastGetSolutionStepValue(DISTANCE);
if (std::abs(domain_phi) > std::abs(inlet_phi)){
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you want to check this? Shouldn't you check that the absolute value of the difference is below a threshold?

return maximum_water_depth_change;
}

void HydraulicFluidAuxiliaryUtilities::CalculateArtificialViscosity(ModelPart &rModelPart, double LimiterCoefficient)
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the style guide for the arguments.

Copy link
Member

Choose a reason for hiding this comment

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

Besides, I just realized that MaximumArtificialViscosity would be a much convenient variable name for this magnitude. As far as I understand, this is not a limiting coefficient but a viscosity.

void HydraulicFluidAuxiliaryUtilities::CalculateArtificialViscosity(ModelPart &rModelPart, double LimiterCoefficient)
{
const auto &r_process_info = rModelPart.GetProcessInfo();
block_for_each(rModelPart.Elements(), [&](Element &rElement){
Copy link
Member

Choose a reason for hiding this comment

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

Can you please respect indentation in this loop? It would ease quite a lot going through the code inside the block_for_each...

Comment on lines +55 to +59
struct NodeConditionDataStruct
{
NodeType Node; // Condition normal
double CornerNode; // Gauss point shape functions values
};
Copy link
Member

Choose a reason for hiding this comment

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

I think this is used nowhere...

@@ -26,12 +26,26 @@ void AddCustomUtilitiesToPython(pybind11::module& m)
{
Copy link
Member

Choose a reason for hiding this comment

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

Please, avoid all the non-necessary changes in the format as these complicate very much differentiating the new additions.

Kratos.VariableUtils().AddDof(Kratos.VELOCITY_Z,self.model_part)
KratosFluidHydraulics.HydraulicFluidAuxiliaryUtilities.FixCornerNodeVelocity(self.model_part , corner_angle)
tested_node = self.model_part.GetNode(33)
self.assertAlmostEqual(tested_node.GetSolutionStepValue(Kratos.BODY_FORCE_Z),0.0,12)
Copy link
Member

Choose a reason for hiding this comment

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

What are you checking in here? Shouldn't you check the fixity status?

Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

Besides some things that I consider tiny errors, there is still room for improvement.

uxuech and others added 19 commits April 16, 2024 10:36
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.h

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…raulic_fluid_auxiliary_utilities.py

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.h

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
…es/hydraulic_fluid_auxiliary_utilities.cpp

Co-authored-by: Rubén Zorrilla <rubenzorrillamartinez@hotmail.com>
@rubenzorrilla
Copy link
Member

@uxuech any advance in this regard?

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.

None yet

2 participants