openforcefield / openff-interchange

A project (and object) for storing, manipulating, and converting molecular mechanics data.
https://docs.openforcefield.org/projects/interchange
MIT License
71 stars 22 forks source link

`_apply_hmr` can silently lead to negative values #1061

Open IAlibay opened 3 days ago

IAlibay commented 3 days ago

Description

Appying HMR has no validation / checks in place to avoid zero / negative mass setting.

Generally I would say this is rare, but I managed to accidentally create this with a 4 amu style test on a methane.

I haven't gone all the way through to a simulation, but whilst I suspect negative mass will fail, zero mass is a special case for openmm which will just freeze the atom (you'd have to be really lucky to set things to zero though).

Possibly just checking for the negative case and throwing an error / warning could be useful?

Reproduction

from openmm import NonbondedForce
from openff.toolkit import Molecule, Topology, ForceField
from openff.interchange.components._packmol import solvate_topology_nonwater
from openff.interchange import Interchange
import pytest

ligand = Molecule.from_smiles('C')
ligand.generate_conformers()
ligand.assign_partial_charges(partial_charge_method='am1bcc')

ff = ForceField('openff-2.2.0.offxml', 'opc.offxml')
inter = Interchange.from_smirnoff(topology=ligand.to_topology(), force_field=ff)

system = inter.to_openmm_system(hydrogen_mass=4.0*1.008)
print(system.getParticleMass(0))

Output

-0.08543200000000084 Da

Software versions

Interchange v0.3.29

mrshirts commented 3 days ago

Or perhaps the check should be that no mass is less than the specified hydrogen mass?

IAlibay commented 3 days ago

Or perhaps the check should be that no mass is less than the specified hydrogen mass?

In terms of "ensuring stable simulations", I think that's a great idea.

However, the 4 amu scaling has been used a lot in the past, where this happens for methyl groups. For those, I could see a case for someone saying "I should be able to generate these to repeat old simulations even though it's not very stable".

How about:

mattwthompson commented 2 days ago

I thought OpenMM forbid negative masses, guess it doesn't.

Erroring out with a negative mass seems appropriate. I don't feel strongly about how to handle the other case, leaning in favor of allowing it with some sort of warning/log message because users should generally be aware of the tradeoffs of 4 amu HMR if they opt in.