-
Notifications
You must be signed in to change notification settings - Fork 277
[GeoMechanicsApplication] Move all non-template code to .cpp files and make sure any .h file does not contain any implementations (elements) #14055
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
base: master
Are you sure you want to change the base?
Conversation
…-cpp-elements # Conflicts: # applications/GeoMechanicsApplication/test_setup_utilities/element_setup_utilities.cpp
rfaasse
left a comment
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.
Thanks for the effort on the element! I mainly have comments about includes that can now be (re)moved from some headers and I think for the stress_state_policy.h header, we could still move some functions to the cpp.
| #include "custom_utilities/transport_equation_utilities.hpp" | ||
| #include "custom_utilities/variables_utilities.hpp" | ||
| #include "geo_mechanics_application_variables.h" | ||
| #include "includes/cfd_variables.h" |
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.
These includes can probably be moved to the source file
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.
moved
| Matrix CalculateNContainer() | ||
| { | ||
| return GetGeometry().ShapeFunctionsValues(GetIntegrationMethod()); | ||
| } |
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.
This one can be moved to the cpp file
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.
moved
| Vector CalculateIntegrationCoefficients() | ||
| { | ||
| GetGeometry().DeterminantOfJacobian(mDetJCcontainer, this->GetIntegrationMethod()); | ||
| return mIntegrationCoefficientsCalculator.Run<Vector>( | ||
| GetGeometry().IntegrationPoints(GetIntegrationMethod()), mDetJCcontainer, this); | ||
| } |
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.
This one can be moved to the cpp file
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.
moved
| std::vector<double> CalculateFluidPressure() | ||
| { | ||
| return GeoTransportEquationUtilities::CalculateFluidPressures( | ||
| mNContainer, VariablesUtilities::GetNodalValuesOf<TNumNodes>(WATER_PRESSURE, this->GetGeometry())); | ||
| } |
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.
This one can be moved to the cpp file
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.
moved
| [[nodiscard]] DofsVectorType GetDofs() const | ||
| { | ||
| return Geo::DofUtilities::ExtractDofsFromNodes(GetGeometry(), WATER_PRESSURE); | ||
| } |
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.
This one can be moved to the cpp file
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.
moved
| #include "custom_elements/U_Pw_base_element.hpp" | ||
| #include "custom_elements/U_Pw_base_element.h" | ||
| #include "custom_utilities/interface_element_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
Same here!
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.
moved to cpp
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
These two includes are no longer used (maybe in the cpp)
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.
stress_strain_utilities is removed and geo.. moved to cpp
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
Same here!
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.
the same ;)
|
|
||
| /// Default Constructor | ||
| UpdatedLagrangianUPwDiffOrderElement() : SmallStrainUPwDiffOrderElement() {} | ||
| UpdatedLagrangianUPwDiffOrderElement(); |
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 think this could be just an =default in the header:
| UpdatedLagrangianUPwDiffOrderElement(); | |
| UpdatedLagrangianUPwDiffOrderElement() = default; |
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.
done
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
These two seem to be unused (maybe also element_utilities?)
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.
stress_strain_utilities is removed and geo.. moved to cpp.
markelov208
left a comment
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.
Hi Richard, many thanks for the review. I moved more includes to cpp, as a result some test files have been updated.
|
|
||
| typename FluidBodyFlowCalculator<TNumNodes>::InputProvider CreateFluidBodyFlowInputProvider(); | ||
|
|
||
| auto MakePropertiesGetter() |
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.
It is possible to move them to cpp. They shall be converted to real functions that is a lot of work and may slow down Kratos.
| #include "custom_utilities/transport_equation_utilities.hpp" | ||
| #include "custom_utilities/variables_utilities.hpp" | ||
| #include "geo_mechanics_application_variables.h" | ||
| #include "includes/cfd_variables.h" |
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.
moved
| Matrix CalculateNContainer() | ||
| { | ||
| return GetGeometry().ShapeFunctionsValues(GetIntegrationMethod()); | ||
| } |
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.
moved
| Vector CalculateIntegrationCoefficients() | ||
| { | ||
| GetGeometry().DeterminantOfJacobian(mDetJCcontainer, this->GetIntegrationMethod()); | ||
| return mIntegrationCoefficientsCalculator.Run<Vector>( | ||
| GetGeometry().IntegrationPoints(GetIntegrationMethod()), mDetJCcontainer, this); | ||
| } |
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.
moved
| std::vector<double> CalculateFluidPressure() | ||
| { | ||
| return GeoTransportEquationUtilities::CalculateFluidPressures( | ||
| mNContainer, VariablesUtilities::GetNodalValuesOf<TNumNodes>(WATER_PRESSURE, this->GetGeometry())); | ||
| } |
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.
moved
| #include "custom_elements/U_Pw_base_element.hpp" | ||
| #include "custom_elements/U_Pw_base_element.h" | ||
| #include "custom_utilities/interface_element_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
moved to cpp
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
stress_strain_utilities is removed and geo.. moved to cpp
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
the same ;)
|
|
||
| /// Default Constructor | ||
| UpdatedLagrangianUPwDiffOrderElement() : SmallStrainUPwDiffOrderElement() {} | ||
| UpdatedLagrangianUPwDiffOrderElement(); |
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.
done
| #include "custom_utilities/stress_strain_utilities.h" | ||
| #include "geo_mechanics_application_variables.h" |
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.
stress_strain_utilities is removed and geo.. moved to cpp.
rfaasse
left a comment
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.
Thanks for processing the review comments, to me this looks ready to go!
|
Hi @avdg81 , Richard made a very thorough review. If you would like to take a look, please could you let me know? Thank you. |
📝 Description
A brief description of the PR.
Actions have been done for files in
custom_elementsfoldersteady_state_Pw_element, transient_Pw_element, and updated_lagragianU_Pw_diff_order_element