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 a short range repulsive potential as baseline potential #265

Closed
PicoCentauri opened this issue Jun 19, 2024 · 4 comments
Closed

Add a short range repulsive potential as baseline potential #265

PicoCentauri opened this issue Jun 19, 2024 · 4 comments
Labels
Discussion Issues to be discussed by the contributors Priority: Medium Important issues to address after high priority.

Comments

@PicoCentauri
Copy link
Contributor

Problem

@HannaTuerk and @bananenpampe recently saw that for some of their systems atoms either cluster or their energy is smaller than one would expect if they are tiny. These are typical indications for holes in the potential. To get rid of them one can add more training data. But this can be very hard because sampling these small distances is very hard.

Possible solution

Another approach is adding a short-range repulsive potential as baseline potential and subtracting the energy and force before the actual training is started. This is very similar to the composition model.

The canonical candiate would be the Ziegler-Biersack-Littmark (ZBL) potential which is also implemented in MACE and GPUMD. @bananenpampe found also a Python implementation.

Implementation

The easiest approach is that we provide a function that computes the energy and forces based on the ZBL potential and each architecture can use this function if they like

Another idea I had, is that we provide these operations in a base class and architecture developers don't have to do it by themselves. Still, we already provide them the energies "corrected" by the composition energy and the short-range repulsion. Basically, they have to call super().forward() in their forward function. I don't know if this is a clever idea though...

@PicoCentauri PicoCentauri added Priority: Medium Important issues to address after high priority. Discussion Issues to be discussed by the contributors labels Jun 19, 2024
@Luthaf
Copy link
Member

Luthaf commented Jun 19, 2024

Another idea I had, is that we provide these operations in a base class [...] Basically, they have to call super().forward() in their forward function.

This will not work, TorchScript does not support calling super().forward(). In general, having a ShortRangeZBL() module — like we have a LinearComposition module for isolated atoms energies — that contributors are using explicitly feels closer to the usual Torch way of doing things.

@PicoCentauri
Copy link
Contributor Author

Fine for me as well.

Maybe I will on top create an architecture ShortRange that does the LinearComposition() plus ShortRangeZBL()

@HannaTuerk
Copy link

I implemented a ShortRangeZBL model which we can integrate. I am not sure where best to put it in the code, so it could discuss this. Otherwise I also talked to Sergey to put it directly in PET, which is also a possibility.
@Luthaf I did all the changes we talked about yesterday except for the requested_neighbor_lists, I not sure how to exactly implement that

@frostedoyster
Copy link
Collaborator

This is now completed and/or tracked in #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues to be discussed by the contributors Priority: Medium Important issues to address after high priority.
Projects
None yet
Development

No branches or pull requests

4 participants