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] Implement well constitutive behaviour for line pressure element #12997

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

Conversation

mnabideltares
Copy link
Contributor

Description:

To enhance our computational model for simulating high capacity heat storage systems, we need to implement a pressure "wel/filter" element on top of the existing pressure line element. The pressure filter element will enable us to model the behavior of heat storage under the ground, accounting for their heat exchange processes. This addition is crucial for achieving realistic simulations of heat storage systems and optimizing their performance.

🆕 Changelog

  • Added the contribution to bodyflow, compressibility matrix (by means of Biot), and constutative law
  • Added test cases
  • Documentation

Copy link
Contributor

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

Thanks for implementing this functionality! I have a few suggestions for clean-up but all in all it's clear (of course I may be a bit biased here 😄 )!

Comment on lines +39 to +40
const double equivalent_radius_square = Prop[CROSS_AREA] / Globals::Pi;
rPermeabilityMatrix(0, 0) = equivalent_radius_square * 0.125;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not expect the 'FillPermeabilityMatrix' function to perform a calculation here ($H(0,0) = 0.125 A/\pi$). I'm not too sure if there is a better place for it though, do you have any ideas?

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.

Hi Mohammed,
Thank you for the work done. I'm not understanding an important point, the nature of the law itself. Please teach me.
Wijtze Pieter

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 seems a nice extension to me, with documentation and tests. Well done! I have several minor suggestions and a few questions, but nothing major.

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.

Dear Mohamed,
Thank you for all the work.
I have still some remarks of which some may even be done at an other time.
I would appreciate if the specific resulting formulas from you PDF document could be includes in GeoMechanicsApplication/custom_constitutive/README.md
Regards, Wijtze Pieter

Comment on lines +71 to +74
const double result =
1.0 / (r_properties[DENSITY_WATER] * ProjectedGravity * r_properties[FILTER_LENGTH]) +
1.0 / r_properties[BULK_MODULUS_FLUID];
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.

Why not:
return 1.0 / (r_properties[DENSITY_WATER] * ProjectedGravity * r_properties[FILTER_LENGTH]) +
1.0 / r_properties[BULK_MODULUS_FLUID];
There is no need for the intermediate variable result.

Comment on lines +284 to +287
for (std::size_t i = 0; i < number_integration_points; ++i) {
J_container[i].resize(GetGeometry().WorkingSpaceDimension(),
GetGeometry().LocalSpaceDimension(), false);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably Sonar Cloud will give a remark about this that can be avoided as follows:
for ( auto& j: J_container ) {
j.resize(GetGeometry().WorkingSpaceDimension(), GetGeometry().LocalSpaceDimension(), false);
}

Comment on lines +10 to +20
"IGNORE_UNDRAINED" : false,
"DENSITY_SOLID" : 2.650000e+03,
"DENSITY_WATER" : 1.000000e+03,
"POROSITY" : 1.000000e-01,
"BULK_MODULUS_SOLID" : 9.000000e+19,
"BULK_MODULUS_FLUID" : 1.000000e+20,
"PERMEABILITY_XX" : 9.084000e-06,
"DYNAMIC_VISCOSITY" : 1.000000e-03,
"RETENTION_LAW" : "PressureFilterLaw",
"CROSS_AREA" : 1.0,
"FILTER_LENGTH" : 3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

You have gotten rid of quite some redundant material parameters already. I'm guessing that also IGNORE_UNDRAINED is not used and that POROSITY is only there because the transient_Pw_line_element CheckProperties is looking at it. This CheckProperties is doing what the check of the constitutive law check should do. The specific filter law does not seem to contain POROSITY. This repeats in the material properties for the other test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Implement well constitutive behaviour for line pressure element
4 participants