Adding per-atom torque controls#4795
Conversation
akohlmey
left a comment
There was a problem hiding this comment.
In addition to the suggested changes, also fix_addtorque.cpp and fix_addtorque.h need to be added to the file src/Purge.list
Furthermore, for the renamed and the added fixes the files doc/src/fix.rst and doc/src/Commands_fix.rst need to be updated.
Finally, the example input in examples/PACKAGES/addtorque/ needs to be updated and new logs created to replace the old ones.
No, this is a good way to make such a change. A few more additional steps are still needed, see my review comments and suggestions. Once the alias has been around for long enough, we move it to |
jibril-b-coulibaly
left a comment
There was a problem hiding this comment.
I only have minor form comments, everything looks good to me otherwise.
On naming, I find the /group suffix to be possibly confused with the LAMMPS group command and concept, e.g., fix settorque/atom still applies to a group.
What is your opinion on using a word that refers to a collection of atoms, but does not already have a specific LAMMPS meaning (i.e., not group, chunk, cluster, aggregate, ...) instead of group, e.g.:
fix settorque/collectionfix settorque/assemblyfix settorque/set
Thank you
@jibril-b-coulibaly I disagree on this. The difference is that fix addtorque/group applies a toque to the whole group (by adding forces) while fix addtorque/atom applies it to the individual atoms in the group. So the naming choice seems very adequate to me. The property that fixes and computes have fix/compute groups is a general property and thus need not be reflected in the name of the style. I don't think that will confuse anybody familiar with LAMMPS and beginners need to read the documentation well, anyway. Most of the suggested alternatives have no existing use in LAMMPS except for collection (for multi-style neighbor lists) and thus are much more confusing, IMO. P.S.: the current naming scheme would also work well with a potential future fix addtorque/chunk which the net torque would be applied to groups of atoms defined by compute chunk/atom. |
|
@jtclemm did some more corrections and updates to the documentation. |
332e7fe to
95e02cd
Compare
|
@jtclemm I like the style names of the new and old addtorque commands. I think they are clear. In principle this could work, but would require the user knows what they are doing. I.e. all atoms in If we want to allow this, possibly the code could check if atoms in rigid bodies are specified and give a warning. |
sjplimp
left a comment
There was a problem hiding this comment.
see my comment about rigid bodies
Good point. I added a note to both fixes. The one exception (actually one of the driving reasons for this PR) is if one simply wants to zero torques (and forces) to freeze rigid particles. Currently that could only be done by excluding those atoms from fix rigid (and therefore not creating a rigid body), but this won't work with some newer granular rigid pair styles being written that need information of the rigid body to calculate geometric terms. In the future, I may try to revisit how fix set/add force/torque and freeze interact with fix rigid. Maybe do something like what is done for fix gravity, but that's an issue for later.
Is there an easy way to check if an atom is in a rigid body? |
Please check out $ grep -l check_rigid_group_overlap fix*.cpp */fix*.cpp
fix_temp_berendsen.cpp
EXTRA-FIX/fix_temp_csld.cpp
EXTRA-FIX/fix_temp_csvr.cpp |
|
@jtclemm Conversely, the renamed addtorque/group commands could be used to change the torque on a rigid body if the group matched the body. But only a single body, so probably not so useful. But again, the use of those 2 commands with rigid bodies should probably be documented. A variant that might be useful would be addtorque/chunk, which could be used to adjust the torque on each of a collection of rigid bodies. E.g. make them all spin. Another (future) idea would be to enhance the velocity command to allow the initial translational or rotational state of rigid bodies (or molecules) to be set. |
I agree. The more I've looked at RIGID the more I've thought about how it could better integrate with many existing fixes/computes. Not a current task I want to take on, but maybe down the road. |
Summary
Mirroring fix addforce and setforce, this PR includes two new GRANULAR fixes for adding and setting peratom torques. To reduce the risk of confusion, the existing fix addtorque was renamed fix addtorque/group and the new fixes were named fix add(set)torque/atom.
I also reduced the number of references to
gmin several granular submodel classes because I found them a bit annoying.Related Issue(s)
NA
Author(s)
Joel Clemmer (SNL)
Licensing
By submitting this pull request, I agree, that my contribution will be included in LAMMPS and redistributed under either the GNU General Public License version 2 (GPL v2) or the GNU Lesser General Public License version 2.1 (LGPL v2.1).
Artificial Intelligence (AI) Tools Usage
By submitting this pull request, I confirm that I did NOT use any AI tools to generate
all or parts of the code and modifications in this pull request.
Backward Compatibility
An alias of fix addtorque for fix addtorque/group was added to preserve backwards compatibility. @akohlmey, please let me know if this isn't the best way to do this.
Implementation Notes
NA
Post Submission Checklist
Further Information, Files, and Links