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

Add fix electron/stopping to USER-MISC #1399

Merged
merged 7 commits into from Apr 5, 2019

Conversation

@rtoijala
Copy link
Collaborator

commented Apr 1, 2019

Summary

Implements inelastic energy loss for fast particles in solids, as commonly required for the simulation of collision cascades.

Author(s)

Implementation by T. Metspalu (Tartu University) and K. Avchaciov (Tartu
University and University of Helsinki).
Bug fix by Henrique Vázquez Muíños (University of Helsinki).
Clean-up by Risto Toijala (University of Helsinki).

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

Backward Compatibility

No breaking changes.

Implementation Notes

Simple cpp+header in USER-MISC.
An attempt was made to follow the LAMMPS coding style.

Questions:

  • When using a compute ke/atom in the LAMMPS script, a warning is printed:
    WARNING: More than one compute ke/atom (src/compute_ke_atom.cpp:55)
    Is there a way to avoid this?
  • I was unable to find how the images in doc/src/Eqs are autogenerated, so the one included here is manually created. Is this OK? Even better, is it possible to embed display-style LaTeX in doc/src/fix_elstop.txt?

Post Submission Checklist

Please check the fields below as they are completed after the pull request has been submitted. Delete lines that don't apply

  • The feature or features in this pull request is complete
  • Licensing information is complete
  • Corresponding author information is complete
  • The source code follows the LAMMPS formatting guidelines
  • Suitable new documentation files and/or updates to the existing docs are included
  • The added/updated documentation is integrated and tested with the documentation build system
  • The feature has been verified to work with the conventional build system
  • The feature has been verified to work with the CMake based build system
  • A package specific README file has been included or updated
  • One or more example input decks are included
Add fix elstop to USER-MISC
Implements inelastic energy loss for fast particles in solids.

@rtoijala rtoijala requested a review from rbberger as a code owner Apr 1, 2019

@akohlmey

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Questions:

  • When using a compute ke/atom in the LAMMPS script, a warning is printed:
    WARNING: More than one compute ke/atom (src/compute_ke_atom.cpp:55)
    Is there a way to avoid this?

compute ke/atom creates a buffer for all atoms, this is wasteful and thus should be avoided.
in your input examples, your use of ke/atom could be avoided by directly accessing the "ke" thermo keyword.

looking at your fix, it would be just as simple to directly add the corresponding code from compute ke/atom (which is done in many other places, where the per-atom kinetic energy is used), e.g.:

  double massone = (atom->rmass) ? atom->rmass[i] : atom->mass[atom->type[i]];
  energy = 0.5 * force->mvv2e * massone * (v[i][0]*v[i][0] + v[i][1]*v[i][1] + v[i][2]*v[i][2]);
  • I was unable to find how the images in doc/src/Eqs are autogenerated, so the one included here is manually created. Is this OK? Even better, is it possible to embed display-style LaTeX in doc/src/fix_elstop.txt?

Those are created manually with pdflatex, pdftoppm, pnmcrop, pnmtopjpeg or an image editor like gimp.
For the command line tool conversion it is important, to have \pagestyle{empty} included, so pnmcrop is not confused by the line number.

as of very recent, it is now also possible to typeset with LaTeX inside the manual, by using MathJax. See for instance the documentation for the recently added pair style granular. https://lammps.sandia.gov/doc/pair_granular.html

@rtoijala

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2019

Thanks for the review.
I changed the docs to use inline LaTeX for the equation (much more pleasant, especially in the PDF).
I also inlined the kinetic energy computation, as suggested. This simplifies the code nicely.

Regarding the use of compute ke/atom in the example in.elstop.only, I tried just appending ke to the thermo line, but it always shows 0. I did not find why, so I ended up using the compute.

@akohlmey

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

@rtoijala @sjplimp i am wondering if fix elstop is a good name for this fix. The label "elstop" doesn't give me much of a hint of what it is doing. Not sure if fix ke/loss or fix inelastic/ke/loss or fix energy/loss or something else would be more appropriate. We have lots of legacy names for fixes that are similarly "cryptic", so there are plenty of precedents of such a name being acceptable. Still wondering...

@rtoijala

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 3, 2019

I consider the term "electronic stopping" well established, as does my supervising professor. "Inelastic" could be a problematic term since often is used when referring to the core electrons. How about electron/stopping?

@akohlmey

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

I consider the term "electronic stopping" well established, as does my supervising professor. "Inelastic" could be a problematic term since often is used when referring to the core electrons. How about electron/stopping?

more explicit like this is definitely better. i am looking at this purely from the point of a code maintainer of a software package, that would like to make it easy for people to spot those commands which may be relevant to them.

let's wait and see if @sjplimp has something to add. Changing the name of the fix style would require to change the name of the class and the files to be consistent (so it is easier for developers to find the files they are looking for 😉) and you don't want to do this more often than necessary because it is tedious.

@sjplimp

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

yes, I like electron/stop or stopping better than elstop. As Axel said, people will
scan the list of fixes and have a guess what that one does.

@akohlmey akohlmey self-assigned this Apr 3, 2019

@akohlmey akohlmey requested review from sjplimp, stanmoore1 and athomps Apr 3, 2019

@akohlmey akohlmey added this to the Stable Release Spring 2019 milestone Apr 3, 2019

@rtoijala

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 4, 2019

I have now renamed the fix to electron/stopping. I hope this is OK for everyone.

@akohlmey akohlmey changed the title Add fix elstop to USER-MISC Add fix electron/stopping to USER-MISC Apr 4, 2019

@athomps

athomps approved these changes Apr 5, 2019

Copy link
Contributor

left a comment

I approve.

@sjplimp

sjplimp approved these changes Apr 5, 2019

Copy link
Contributor

left a comment

looks good

@akohlmey akohlmey merged commit ed90596 into lammps:master Apr 5, 2019

8 checks passed

lammps/pull-requests/build-docs-pr head run ended
Details
lammps/pull-requests/cmake/cmake-serial-pr head run ended
Details
lammps/pull-requests/cmake/cmake-win32-serial head run ended
Details
lammps/pull-requests/cmake/cmake-win64-serial head run ended
Details
lammps/pull-requests/kokkos-omp-pr head run ended
Details
lammps/pull-requests/openmpi-pr head run ended
Details
lammps/pull-requests/serial-pr head run ended
Details
lammps/pull-requests/shlib-pr head run ended
Details

@rtoijala rtoijala deleted the rtoijala:fix-elstop branch Apr 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.