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

Second Moment Aproximation to the Tight Binding addes as pair style #3031

Merged
merged 23 commits into from Apr 29, 2022

Conversation

Iximiel
Copy link
Contributor

@Iximiel Iximiel commented Nov 18, 2021

Summary

I added the Second Moment Aproximation to the Tight Binding (SMATB) potential as a pair style, along with some very simple examples and the documentation to start a simulation with this potential.

Author(s)

Daniele Rapetti - Politecnico di Torino - daniele.rapetti@polito.it (personal: iximiel(at)gmail(dot)com)

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

Post Submission Checklist

  • 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
  • Suitable tests have been added to the unittest tree.
  • A package specific README file has been included or updated
  • One or more example input decks are included

@akohlmey
Copy link
Member

@Iximiel thanks for your submission. Before looking at the implementation and making some (required) suggestions, can you please discuss how your implementation differs from what functionality is available in the SMTBQ package
with its pair style smtbq

@Iximiel
Copy link
Contributor Author

Iximiel commented Nov 19, 2021

Thank you for the fast reply.
The main applicative difference between SMTB-Q and SMATB is that the SMTB-Q aims at simulating metal oxides while SMATB aims at simulating metal and alloys from the transition group.
More on the physics side, even if they share partially a term of the potential, i.e., the covalent part of the SMTB-Q (eq. 4 in 10.1016/j.susc.2013.05.015) is similar to the bonding part of the SMATB (eq 18 in 10.1103/PhysRevB.23.6265). This similarity is obviously due to the fact we both use a second moment approximation of the TB. Another difference is the absence of the charge terms in SMATB.

Initially I thought that the acronym SMATB describes more correctly the idea behind the form of the potential. Anyway, I agree that maybe this name similarity can be misleading, and I am keen in accepting any kind of change in the keywords. A possible solution is given by the fact that the SMATB potential is also known as "Gupta" or "RGL" (according to the community that is using it).

Thank you again for your time
Daniele

@akohlmey
Copy link
Member

@Iximiel thanks for the info, that eliminates the concern that we would add duplicate functionality.
There is one general concern: we cannot accept adding your contribution to the core LAMMPS sources folder. It would have to be added to an existing package or become a new package. Due to the similarity with the smtbq pair style, it would make sense to join those pair styles in a single package. That however would require renaming the SMTBQ package (again, ugh!) so it would properly cover both implementations and also the pair styles for consistency.

@sjplimp what do you think?

  • The package would be renamed from SMTBQ to SMATB
  • The existing pair style from smtbq to smatb/qeq to more correctly represent that it is a second moment approximation to tight binding with charge equilibration added (of course the old pair style name would be retained as an alias for backward compatibility)
  • The new pair styles in this pull request would be moved there
  • The documentation for each of the package pair styles would include a note explaining the difference and referring to the other

@Iximiel once we have sorted out these "administrative matters", I will start looking at the code, examples and other details.

Please note that you added your changes to the "develop" branch in your fork. We usually encourage people to keep the branches from the upstream repository unchanged (and set the corresponding branch in the original LAMMPS repository as their upstream branch for simple synchronization with upstream) and instead work on a feature branch and submit that as a pull request. But no worries, you can correct that later. For the time being, please don't make any changes to your branch, since I plan to do the renaming and moving of files by myself and then push them into your repository. Thanks, Axel.

@sjplimp
Copy link
Contributor

sjplimp commented Nov 22, 2021

I'm fine with a merged package and rename of the package and old pair style. As a courtesy, running this by the original smtbq folks would be a good idea.

@nsalles33
Copy link

Hi all, I implemented pair_SMTBQ in LAMMPS with Olivier Politano and Robert Tétot. We agree to merge the package SMTBQ to SMATB/QEq.
Thanks
Nicolas Salles

@Iximiel
Copy link
Contributor Author

Iximiel commented Apr 13, 2022

Hi, I am sorry for answering so late.
I agree with the proposed merging of the packages.
If anything else is needed from me, consider me at your disposal.

Thanks
Daniele

@Iximiel Iximiel requested a review from rbberger as a code owner April 27, 2022 19:38
@akohlmey akohlmey self-assigned this Apr 27, 2022
@akohlmey
Copy link
Member

Hi, I am sorry for answering so late. I agree with the proposed merging of the packages. If anything else is needed from me, consider me at your disposal.

Thanks Daniele

@Iximiel

No worries. We've been slow to process this as well. I've finally worked my way down the queue far enough to review this again. I've updated the code to the most recent upstream changes, made some cosmetic changes and also aligned its execution more with how things are done elsewhere in LAMMPS. Also, I've added some unit test inputs for the pair style tester tool. The good news is, that restarting works fine. The bad news is, that using newton pair off does not work. I've made some smal changes that resulted in consistent energies, but forces are off. For the time being, I have made using newton pair off an error and also told the unit test input to skip those.

I've ultimately decided to not rename the SMTBQ package or its pair style, but just moved the new styles to the existing package and updated the documentation accordingly. This way there is less disruption and backward compatibility.

Copy link
Contributor

@athomps athomps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve.

@akohlmey akohlmey merged commit 6fb3cef into lammps:develop Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants