Failure with SMIRNOFF-style impropers

mattwthompson commented 2 years ago

Using https://github.com/openforcefield/openff-toolkit/pull/951/commits/7fb46c7ff6b7a49a6c04ef10956c66f790f7673e

In:

from simtk.openmm.app import PDBFile
from openff.toolkit.utils import get_data_file_path
from openff.toolkit.topology import Molecule, Topology
from openff.toolkit.typing.engines.smirnoff import ForceField

t4 = Molecule.from_file(get_data_file_path('proteins/T4-protein.sdf'))
ff = ForceField('ff14sb_off_impropers_0.0.1.offxml')
ff.create_openmm_system(t4.to_topology())

Out:

---------------------------------------------------------------------------
NotBondedError                            Traceback (most recent call last)
<ipython-input-4-211f4dcc3ca1> in <module>
----> 1 ff.create_openmm_system(t4.to_topology())

~/miniconda3/envs/openff-system/lib/python3.8/site-packages/openff/toolkit/typing/engines/smirnoff/forcefield.py in create_openmm_system(self, topology, **kwargs)
   1341         # Add forces and parameters to the System
   1342         for parameter_handler in parameter_handlers:
-> 1343             parameter_handler.create_force(system, topology, **kwargs)
   1344
   1345         # Let force Handlers do postprocessing

~/miniconda3/envs/openff-system/lib/python3.8/site-packages/openff/toolkit/typing/engines/smirnoff/parameters.py in create_force(self, system, topology, **kwargs)
   3379             # for (i, j) in [(0, 1), (1, 2), (1, 3)]:
   3380             #     topology.assert_bonded(atom_indices[i], atom_indices[j])
-> 3381             self._assert_correct_connectivity(improper_match, [(0, 1), (1, 2), (1, 3)])
   3382
   3383             improper = improper_match.parameter_type

~/miniconda3/envs/openff-system/lib/python3.8/site-packages/openff/toolkit/typing/engines/smirnoff/parameters.py in _assert_correct_connectivity(match, expected_connectivity)
   2324             atom_j = match.environment_match.reference_atom_indices[connectivity[1]]
   2325
-> 2326             reference_molecule.get_bond_between(atom_i, atom_j)
   2327
   2328     def assign_parameters(self, topology, system):

~/miniconda3/envs/openff-system/lib/python3.8/site-packages/openff/toolkit/topology/molecule.py in get_bond_between(self, i, j)
   5878         from openff.toolkit.topology import NotBondedError
   5879
-> 5880         raise NotBondedError("No bond between atom {} and {}".format(i, j))
   5881
   5882     @property

NotBondedError: No bond between atom 4 and 19

The source of the error is an improper:

ipdb> improper_match.parameter_type.smirks
'[H][C@@:1]([C])([N+])[C:3](=[O:4])[N:2]([H])[C]'
ipdb> improper_match.parameter_type.id
'A14SB-NTerminal-PRO_ALA-CX_N_C_O'

ff.get_parameter_handler('ImproperTorsions')._INFOTYPE._VALENCE_TYPE = None did not fix it

@j-wags has suggested it might be due to where in the SMIRKS patterns the central atoms are specified. I tried swapping indices 2 and 3, but that produces the same error. I could have screwed up the editing of the file; I don't really want to check hundreds of lines of XMLs by hand.

test.offxml.txt

lilyminium commented 2 years ago

I had more success exploiting the Python API; no error, although I also did not check the output system.

ff14sb_off_remapped_impropers.txt

#!/usr/bin/env python
# coding: utf-8

# In[1]:

from simtk.openmm.app import PDBFile
from openff.toolkit.utils import get_data_file_path
from openff.toolkit.topology import Molecule, Topology
from openff.toolkit.typing.engines.smirnoff import ForceField

t4 = Molecule.from_file(get_data_file_path('proteins/T4-protein.sdf'))
ff = ForceField('ff14sb_off_impropers_0.0.1.offxml')

# In[2]:

import re

impropers = ff.get_parameter_handler("ImproperTorsions")
for imp in impropers.parameters:
    imp.smirks = re.sub("(:[23])(.+)(:[23])", r"\3\2\1", imp.smirks)

# In[3]:

ff.create_openmm_system(t4.to_topology())

# Out[3]:
<simtk.openmm.openmm.System; proxy of <Swig Object of type 'OpenMM::System *' at 0x7f8263ede1e0> >

j-wags commented 2 years ago

Yeah, I was getting this locally too. I think @lilyminium's solution is the right one (switching :2s with :3s throughout the impropers section, though I'm not sure that we can safely change parameter SMIRKS in-place). Sorry about the sloppiness of that file in the initial release.

lilyminium commented 2 years ago

though I'm not sure that we can safely change parameter SMIRKS in-place

You could give loading the forcefield from the remapped file a try -- if that passes any tests you have in place, it could just get released as a fix.

j-wags commented 2 years ago

This has been fixed in the ff14sb_off_impropers_0.0.2.offxml file in release 0.0.2

j-wags commented 2 years ago

(And the improper-fixing and validation code was piggybacked onto the parameter deduplication notebook in #34 )

mattwthompson commented 2 years ago

Thanks ! Conda packages coming soon ...

openforcefield / amber-ff-porting

Failure with SMIRNOFF-style impropers #36