LammpsPotential: adapt for using Nequip potentials

[bastonero]

The current implementation cannot exploit other potentials, such as neural network potentials, in this case, coming from Nequip architecture. The implementation now is extended to be able to use this ML family of potentials.

[bastonero]

Hi, I wanted to implement these modifications to be able to run LAMMPS using NN-ML from Nequip architectures. They are still to refine, so happy to have your feedbacks.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.44%. Comparing base (6611b2a) to head (ffb555e).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #118   +/-   ##
=======================================
  Coverage   85.43%   85.44%           
=======================================
  Files          19       19           
  Lines        1614     1615    +1     
=======================================
+ Hits         1379     1380    +1     
  Misses        235      235           

Flag	Coverage Δ
pytests	85.44% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JPchico]

A couple of questions

1. Why do you need to change the base extension of the potential file?
2. Why do you need to change how the file is being read? Is there any reason why you need to do it in this way? Would this affect other kinds of potentials? I do not think so but I just wonder if you have tested this.
3. Why do you need to pass the potential file name to the input file generator?

[bastonero]

1. This is needed as the potentials used by Nequip/Allegro are written in Pytorch, and I believe it wouldn't work otherwise with .dat extension.
2. As it is a .pth file, then one needs to specify the binary format for the reading.
3. The reason being otherwise it would write it as .dat, whereas one needs to specify .pth

These changes are needed when one patches LAMMPS with pair_nequip/pair_allegro potentials. Of course, maybe we want to make these decision automatically, e.g. by checking the name of the potential (in these cases either nequip or allegro), and then use the corresponding filename and reading format.

[JPchico]

1. That is okay, I was mostly curious, I just wonder why the extension matters but if it does I se no harm in changing it. A question are these potentials created before hand by the program?
2. Okay so it is a binary format, but in the current implementation one stores the contents of the file in a SinglefileData orm. That is why my confusion, do you need the two handlers?

These changes are needed when one patches LAMMPS with pair_nequip/pair_allegro potentials. Of course, maybe we want to make these decision automatically, e.g. by checking the name of the potential (in these cases either nequip or allegro), and then use the corresponding filename and reading format.

I'd just like to keep that the potential class handles all the potentials in the same way if it is possible. I'm not familiar with these potentials, but for what I understand these are binary files that LAMMPS reads, which maybe are created before hand? Or maybe they are updated on the fly by some active learning approach? In either case, what would be preferable is that one can treat these potentials in the same way that other types to reduce the complexity of the plugin.

[JPchico]

Hi @bastonero nice to have your inputs, I left a couple of messages in your code about some changes that I do not fully understand why they are necessary,

[JPchico]

I notice also that the tests are failing and this seems to be a general issue after the release, I've been very busy the last weeks, but I will try to look into this.

[JPchico]

Hi @bastonero I have made a new release which should take care of the failing tests. Feel free to try it out.