-
Notifications
You must be signed in to change notification settings - Fork 248
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
[DEMApplication] New class for global damping #13019
Conversation
${CMAKE_CURRENT_SOURCE_DIR}/custom_constitutive/DEM_global_damping_model.cpp | ||
${CMAKE_CURRENT_SOURCE_DIR}/custom_constitutive/DEM_global_damping_model_nonviscous.cpp | ||
${CMAKE_CURRENT_SOURCE_DIR}/custom_constitutive/DEM_global_damping_model_viscous.cpp | ||
${CMAKE_CURRENT_SOURCE_DIR}/custom_constitutive/DEM_global_damping_model_viscousforcedependent.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.
I wonder how we should name those damping models. For the one you used, it is the same with eq. 18 in Zhao's work [2014]. However, he said, "To ensure quasi-static condition, a simple local non-viscous damping force [40] is added to dissipate kinetic energy".
- I understand that we are using "global damping" to identify it with the "contact viscous damping" we used for CLs.
- As Zhao said that's a non-viscous damping force, please check whether you can call it "viscous force dependent". I am confused about your description.
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.
True. I changed the model names to what I think is more representative and, maybe, correct.
const double E = p_element->GetYoung(); | ||
|
||
if (vel_magnitude != 0.0) { | ||
const array_1d<double, 3> damping_force = -2.0 * mGlobalDamping * sqrt(m*r*E) * vel; |
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.
Thank you for putting this model (if it can be called a model) here. Some time ago, I was using this equation to apply some additional global damping forces to the particle together with the "standard" global damping force to force the system to calm down. It is function is like we are putting the particle in a viscous fluid. To be honest, I don't like this equation myself, as it is less physical. We can keep it here until someday we don't need it anymore.
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.
Additionally, I will clean up the old implementation after you merge this.
|
||
self.global_damping_option = self.global_damping != 0.0 | ||
|
||
existing_damping_models = ["NonViscous","Viscous","ViscousForceDependent"] |
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 name problem again
|
||
// Public methods | ||
virtual void SetGlobalDampingModelInProperties(Properties::Pointer pProp, bool verbose = true); | ||
virtual void AddGlobalDampingForceAndMoment(SphericParticle* p_element, array_1d<double,3>& total_forces, array_1d<double,3>& total_moment) {} |
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.
Maybe you can add the following here:
KRATOS_ERROR << "This function (....) shouldn't be accessed, use derived class instead"<<std::endl;"
GeometryFunctions::normalize(velocity); | ||
const double force = DEM_MODULUS_3(total_forces); | ||
|
||
if (central_node.IsNot(DEMFlags::FIXED_VEL_X)) { |
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 difference between this model and the "NonViscous" one is that this model applies a damping force proportional to the velocity component in each component of the total force. Thus, the direction of the total force is modified. The "NonViscous" one keeps the direction of the total force unchanged and reduces the components by the same proportion. I guess understanding this difference may help you get a better name.
|
||
namespace Kratos | ||
{ | ||
DEMGlobalDampingModel::Pointer DEMGlobalDampingNonViscousCteForceDir::Clone() const { |
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.
NICE! Those new names are much clearer now. Only one point, for "DEMGlobalDampingNonViscousCteForceDir", I am confused about "Cte". For "Constant", a commonly used short name is "Const". Or maybe "Cst" if you prefer to use three letters, although it's less common. But "Cte" may be a bit hard to understand.
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.
True, 'cte' is appropriate in Spanish
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.
Nice done!
Let's see whether another team member has some more comments @KratosMultiphysics/dem |
Implementation of a new class for global damping (also with a few formatting organization).
Observations:
In this subclass, I use the GLOBAL_DAMPING parameter, instead of GLOBAL_VISCOUS_DAMPING, for the computations. Therefore, @ChengshunShang1996 please check it, and if it is okay, you could remove GLOBAL_VISCOUS_DAMPING and your implementation from here. Or let me know if you'd like me to do it.