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

PBC issue with LAMMPS interface (probably with fix) #618

Open
andreas-er opened this issue Mar 16, 2024 · 1 comment
Open

PBC issue with LAMMPS interface (probably with fix) #618

andreas-er opened this issue Mar 16, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@andreas-er
Copy link

andreas-er commented Mar 16, 2024

Hi,
I ran some tests with the SchNetPack-LAMMPS interface using periodic systems and got strange energies/forces when atoms were crossing the cell boundaries. If I understood it correctly, SchNetPack calculates the pairwise distances using the cell offsets in cartesian coordinates (in distance units) and, therefore, the ASE or MatScipy neighbor lists convert the offsets from scaled coordinates (multiplier of the cell vectors) to cartesian:

schnetpack/atomistic/distances.py
l. 24: Rij = R[idx_j] - R[idx_i] + offsets

schnetpack/transform/neighborlist.py
l. 245:  offset = torch.mm(S, cell)

The LAMMPS interface does actually the opposite operation converting cartesian offsets to scaled ones before creating the input Dict:

interfaces/lammps/pair_schnetpack.cpp
l. 236: auto cell_inv = cell_tensor.inverse().transpose(0,1);
l. 268: torch::Tensor cell_shift_tensor = cell_inv.matmul(periodic_shift_tensor);

As a quick fix, I commented out some lines and made a few minor changes (see "pair_schnetpack_pbcfix.cpp" in "aspirin_pbcfix", lines 236, 268-280). I used your aspirin example to test both versions ("aspirin_orig" and "aspirin_pbcfix" attached) with the molecule within a periodic box and one that crosses the unit cell boundaries. I double-checked energies and forces with SchNetPack's ASE interface and it seems to work fine now :)

I hope this helps.
Best,
Andreas

ase_sp.tar.gz
aspirin_orig.tar.gz
aspirin_pbcfix.tar.gz

@jnsLs
Copy link
Collaborator

jnsLs commented Mar 18, 2024

Thank you a lot for your help, Andreas!
We will implement this bug fix asap.
Best,
Jonas

@jnsLs jnsLs added the bug Something isn't working label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants