Conversation
With returning to the original trial stresses, average the stresses, average the mapping direction, then map it again
markelov208
left a comment
There was a problem hiding this comment.
Hi Mohamed, finally your hard work can be used by others. Here are my comments. Hope some of them may be useful.
applications/GeoMechanicsApplication/custom_constitutive/coulomb_with_tension_cut_off_impl.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/coulomb_yield_surface.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
...icsApplication/tests/cpp_tests/custom_constitutive/test_mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
...icsApplication/tests/cpp_tests/custom_constitutive/test_mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
...icsApplication/tests/cpp_tests/custom_constitutive/test_mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/custom_constitutive/test_yield_surface.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/custom_constitutive/test_yield_surface.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
...tions/GeoMechanicsApplication/custom_constitutive/interface_coulomb_with_tension_cut_off.cpp
Outdated
Show resolved
Hide resolved
… stresses only once.
…sses now outside if else condition again
…ulomb yield surface and the classes using coulomb yield surface.
markelov208
left a comment
There was a problem hiding this comment.
Hi Wijtze Pieter, thank you very much for the changes, especially, enum class. They made the code is cleaner and more understandable. I have a few minor comments.
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
| mCoulombWithTensionCutOffImpl.DoReturnMapping(r_prop, trial_sigma_tau, averaging_type); | ||
| mapped_principal_stress_vector = StressStrainUtilities::TransformSigmaTauToPrincipalStresses( | ||
| mapped_sigma_tau, averaged_principal_trial_stress_vector); | ||
| mapped_principal_stress_vector[1] = mapped_principal_stress_vector[static_cast<std::size_t>(averaging_type)]; |
There was a problem hiding this comment.
I am afraid using the static_cast is error prone. I would suggest to use switch with enum. Then there is no need to assign enum values explicitly,
There was a problem hiding this comment.
changed to a switch with enum
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
...icsApplication/tests/cpp_tests/custom_constitutive/test_mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
| auto p_coulomb_yield_surface = | ||
| std::make_unique<CoulombYieldSurface>(friction_angle, cohesion, dilatancy_angle); |
There was a problem hiding this comment.
I am just curious, what is the reason for this change? It requires change in line 66.
There was a problem hiding this comment.
This was done for a previous different attempt of refactoring, I have reverted the change now.
rfaasse
left a comment
There was a problem hiding this comment.
Thank you for all the work on this (both the coding as well as all investigations and debugging sessions). It looks clear to me, although I don't know how to check all formulations etc.
I have some minor suggestions, but I think the main question is if we need to change the documentation
applications/GeoMechanicsApplication/custom_constitutive/coulomb_yield_surface.cpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
| averaged_principal_trial_stress_vector); | ||
| mapped_sigma_tau = | ||
| mCoulombWithTensionCutOffImpl.DoReturnMapping(r_prop, trial_sigma_tau, averaging_type); | ||
| mapped_principal_stress_vector = StressStrainUtilities::TransformSigmaTauToPrincipalStresses( |
There was a problem hiding this comment.
When there is averaging, the TransformSigmaTauToPrincipalStresses function is called twice and the result of the first call is overwritten. We might be able to remove the redundant call in case of averaging.
There was a problem hiding this comment.
Before the result of the first call is overwritten, the result of the first call is used to determine ( with FindAveragingType() ) whether the averaging route should be taken. There is no redundant call.
There was a problem hiding this comment.
I am not sure what you mean. We can discuss it when you are available.
applications/GeoMechanicsApplication/tests/test_mohr_coulomb_with_tension_cutoff.py
Show resolved
Hide resolved
| bool IsStressAtCornerReturnZone(const Vector& rTrialSigmaTau, | ||
| const Vector& rDerivativeOfFlowFunction, | ||
| const Vector& rCornerPoint) | ||
| { | ||
| return rTrialSigmaTau[0] - rCornerPoint[0] - | ||
| (rTrialSigmaTau[1] - rCornerPoint[1]) * std::sin(DilatancyAngleInRadians) >= | ||
| return (rTrialSigmaTau[0] - rCornerPoint[0]) * rDerivativeOfFlowFunction[1] - | ||
| (rTrialSigmaTau[1] - rCornerPoint[1]) * rDerivativeOfFlowFunction[0] >= | ||
| 0.0; |
There was a problem hiding this comment.
This is now general, such that it can be used for all return directions.
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/coulomb_yield_surface.cpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/custom_constitutive/mohr_coulomb_with_tension_cutoff.cpp
Outdated
Show resolved
Hide resolved
| averaged_principal_trial_stress_vector); | ||
| mapped_sigma_tau = | ||
| mCoulombWithTensionCutOffImpl.DoReturnMapping(r_prop, trial_sigma_tau, averaging_type); | ||
| mapped_principal_stress_vector = StressStrainUtilities::TransformSigmaTauToPrincipalStresses( |
There was a problem hiding this comment.
Before the result of the first call is overwritten, the result of the first call is used to determine ( with FindAveragingType() ) whether the averaging route should be taken. There is no redundant call.
applications/GeoMechanicsApplication/tests/test_mohr_coulomb_with_tension_cutoff.py
Show resolved
Hide resolved
WPK4FEM
left a comment
There was a problem hiding this comment.
With the updated documentation, for me this is ready to go.
📝 Description
In order to make the currently implemented Mohr-Coulomb consistent to UDSM of Plaxis, we need to change the reordering of the principal stresse to averaging type.
🆕 Changelog