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

LAMMPS non-unique molecule ID hotfix #1000

Merged
merged 18 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ab2b4ef
Salted hash in _write_atoms() with molecule ID to guarantee unique ID…
timbernat Jul 9, 2024
2b4bc0c
Wrote unit test to check unique molecule IDs in Interchange.to_lammps…
timbernat Jul 9, 2024
48ef64b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 9, 2024
d988cbf
Apply suggestions from code review
timbernat Jul 16, 2024
2e3de08
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2024
424de3b
Removed unnecessary typing imports
timbernat Jul 16, 2024
4c50cd3
Consolidated pilot_mol init and conformer generation using MoleculeWi…
timbernat Jul 16, 2024
46505da
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2024
8c1fc7b
Updated sidelen default, added assertion to enforce test for multiple…
timbernat Jul 16, 2024
99d3fdf
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2024
5266a28
Wrapped mol ID + CTAB hash with hash() to produce more literal value …
timbernat Jul 16, 2024
8b72acd
Re-implemented LAMMPS file molecule ID extraction with in-Python lamm…
timbernat Jul 16, 2024
5d49deb
Merge remote-tracking branch 'refs/remotes/origin/lammps-hotfix' into…
timbernat Jul 16, 2024
e3f1d27
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2024
652ad43
Added pytest parametrization of auxiliary args in test_unique_lammps_…
timbernat Jul 16, 2024
e4ffb11
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2024
04bfc1d
MAINT: Streamline, skip generating positions
mattwthompson Jul 16, 2024
2911b38
FIX: Use benzene, not fulvene
mattwthompson Jul 16, 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
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
from pathlib import Path

import lammps
import numpy
import pytest
from openff.toolkit import Topology, unit
from openff.toolkit import ForceField, Quantity, Topology, unit
from openff.utilities import temporary_cd

from openff.interchange import Interchange
from openff.interchange._tests import MoleculeWithConformer, needs_lmp
from openff.interchange.components.mdconfig import MDConfig
from openff.interchange.drivers import get_lammps_energies, get_openmm_energies

rng = numpy.random.default_rng(821)


@needs_lmp
class TestLammps:
Expand All @@ -21,7 +28,12 @@ class TestLammps:
"C1COC(=O)O1", # This adds an improper, i2
],
)
def test_to_lammps_single_mols(self, mol, sage_unconstrained, n_mols):
def test_to_lammps_single_mols(
self,
mol: str,
sage_unconstrained: ForceField,
n_mols: int,
) -> None:
"""
Test that Interchange.to_openmm Interchange.to_lammps report sufficiently similar energies.

Expand Down Expand Up @@ -62,3 +74,65 @@ def test_to_lammps_single_mols(self, mol, sage_unconstrained, n_mols):
"Torsion": 0.02 * unit.kilojoule_per_mole,
},
)

@pytest.mark.parametrize("n_mols", [2, 3])
@pytest.mark.parametrize(
"smiles",
[
"CCO",
"N1CCCC1",
"c1ccccc1",
],
)
def test_unique_lammps_mol_ids(
self,
smiles,
sage_unconstrained,
n_mols,
) -> bool:
"""Test to see if interop.lammps.export._write_atoms() writes unique ids for each distinct Molecule"""

molecule = MoleculeWithConformer.from_smiles(smiles)
topology = Topology.from_molecules(n_mols * [molecule])

# Just use random positions since we're testing molecule IDs, not physics
topology.set_positions(
Quantity(
rng.random((topology.n_atoms, 3)),
"nanometer",
),
)
topology.box_vectors = Quantity([4, 4, 4], "nanometer")

with temporary_cd():
lammps_input_path = Path.cwd() / "temp.in"
lammps_data_path = Path.cwd() / "out.lmp"

interchange = sage_unconstrained.create_interchange(topology)
interchange.to_lammps(lammps_data_path)

mdconfig = MDConfig.from_interchange(interchange)
mdconfig.write_lammps_input(
interchange=interchange,
input_file=lammps_input_path,
)

# Extract molecule IDs from data file
with lammps.lammps(
cmdargs=["-screen", "none", "-log", "none"],
) as lmp:
lmp.file(
"temp.in",
)
written_mol_ids = {
mol_id
for _, mol_id in zip(
range(lmp.get_natoms()),
lmp.extract_atom("molecule"),
)
}

# these are expected to be [1...N] for each of N molecules
expected_mol_ids = {i + 1 for i in range(interchange.topology.n_molecules)}

assert expected_mol_ids == written_mol_ids
8 changes: 7 additions & 1 deletion openff/interchange/interop/lammps/export/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,13 @@ def _write_atoms(lmp_file: IO, interchange: Interchange, atom_type_map: dict):
atom_map[atom_index] = atom

for molecule_index, molecule in enumerate(interchange.topology.molecules):
molecule_hash = molecule.ordered_connection_table_hash()
# inject mol ID into hash to allow chemically-identical Molecules
# to be labelled with distinct IDs
molecule_hash = hash(
f"{molecule_index}_{molecule.ordered_connection_table_hash()}",
)
# TODO: see if this fix also applied for the same issue in GROMACS/AMBER writers

molecule_map[molecule_hash] = molecule_index
for atom in molecule.atoms:
atom_molecule_map[atom] = molecule_hash
Expand Down
Loading