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.

Sign up for GitHub

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

Jump to bottom

LammpsPotential: adapt for using Nequip potentials #118

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

LammpsPotential: adapt for using Nequip potentials #118

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
598fa7b
LammpsPotential: adapt for using Nequip potentials
bastonero Mar 27, 2024
ffb555e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions aiida_lammps/calculations/base.py
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

  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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  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.

Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class LammpsBaseCalculation(CalcJob):
"parser_name": "lammps.base",
}

_POTENTIAL_FILENAME = "potential.dat"
_POTENTIAL_FILENAME = "potential.pth"

# In restarts, will not copy but use symlinks
_default_symlink_usage = True
Expand Down Expand Up @@ -365,15 +365,20 @@ def prepare_for_submission(self, folder):
handle.write(structure_filecontent)

# Write the potential to the remote folder
with folder.open(self._POTENTIAL_FILENAME, "w") as handle:
handle.write(self.inputs.potential.get_content())
# with folder.open(self._POTENTIAL_FILENAME, "w") as handle:
# handle.write(self.inputs.potential.get_content())

with folder.open(self._POTENTIAL_FILENAME, 'wb') as handle1:
with self.inputs.potential.open(mode='rb') as handle2:
handle1.write(handle2.read())

# Write the input file content. This function will also check the
# sanity of the passed parameters when comparing it to a schema
input_filecontent = generate_input_file(
potential=self.inputs.potential,
structure=self.inputs.structure,
parameters=_parameters,
potential_filename=self._POTENTIAL_FILENAME,
restart_filename=_restart_filename,
trajectory_filename=_trajectory_filename,
variables_filename=_variables_filename,
Expand Down
2 changes: 2 additions & 0 deletions aiida_lammps/data/lammps_potentials.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
"wavepacket"
],
"pair_style": {
"allegro":{"units": "metal", "atom_style": "atomic", "read_from_file": true},
"nequip":{"units": "metal", "atom_style": "atomic", "read_from_file": true},
"adp":{"units": "unknown", "atom_style": "atomic", "read_from_file": true},
"agni":{"units": "unknown", "atom_style": "atomic", "read_from_file": true},
"airebo":{"units": "unknown", "atom_style": "atomic", "read_from_file": true},
Expand Down