-
Notifications
You must be signed in to change notification settings - Fork 91
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
vdW switch width not used #882
Comments
Is the spec being followed at the moment?
i.e. is a switching function even being applied? |
Also see linked issue for some confusion about what |
I don't have specific feedback on details of this because I haven't worked with the relevant part of OpenMM enough. Generally, though, I'd expect there to be a VDW switch region over which VDW interactions are switched smoothly to zero (switching function) and the switch width ought to define this. If it's not being done we should fix it, I think. |
Did this get cleared up by addressing OFF-EP 000 so now we know what to implement? If we're going to have switch-width in the API it has to do something, otherwise we need to remove it, I think. I also just commented on #896 in case that removes the blocker. |
I don't think would be fixed by an OFF-EP since the problem is the implementation not following the spec. The linked PR should close this, though, provided it's accepted and we've determined the behavior change is acceptable. |
This has been resolved, and actually a new error will be raised when trying to use a nonsense switch width: In [3]: from openff.toolkit.topology import Molecule
...: from openff.toolkit.typing.engines.smirnoff import ForceField
...: from openff.units import unit
...:
...: molecule = Molecule.from_smiles("CCC")
...:
...: # Create a system from a default FF.
...: force_field = ForceField("openff-1.3.0.offxml")
...:
...: system_original = force_field.create_openmm_system(molecule.to_topology())
...: system_original_str = openmm.XmlSerializer.serialize(system_original)
...:
...: # Create a system from a modified version of the FF
...: force_field = ForceField("openff-1.3.0.offxml")
...: force_field.get_parameter_handler("vdW").switch_width = 1000.0 * unit.angstroms
...:
...: system_modified = force_field.create_openmm_system(molecule.to_topology())
...: system_modified_str = openmm.XmlSerializer.serialize(system_modified)
...:
...: assert system_original_str != system_modified_str
/Users/mattthompson/mambaforge/envs/test/lib/python3.10/site-packages/openff/interchange/components/interchange.py:339: UserWarning: Automatically up-converting BondHandler from version 0.3 to 0.4. Consider manually upgrading this BondHandler (or <Bonds> section in an OFFXML file) to 0.4 or newer. For more details, see https://openforcefield.github.io/standards/standards/smirnoff/#bonds.
warnings.warn(
---------------------------------------------------------------------------
UnsupportedCutoffMethodError Traceback (**I'm cutting out some lines of this**)
File ~/mambaforge/envs/test/lib/python3.10/site-packages/openff/interchange/interop/openmm/_nonbonded.py:763, in _apply_switching_function(vdw_handler, force)
758 switching_distance = to_openmm_quantity(
759 vdw_handler.cutoff - vdw_handler.switch_width,
760 )
762 if switching_distance._value < 0:
--> 763 raise UnsupportedCutoffMethodError(
764 "Found a `switch_width` greater than the cutoff distance. It's not clear "
765 "what this means and it's probably invalid. Found "
766 f"switch_width{vdw_handler.switch_width} and cutoff {vdw_handler.cutoff}",
767 )
769 force.setUseSwitchingFunction(True)
770 force.setSwitchingDistance(switching_distance)
UnsupportedCutoffMethodError: Found a `switch_width` greater than the cutoff distance. It's not clear what this means and it's probably invalid. Found switch_width1000.0 angstrom and cutoff 9.0 angstrom Slightly modifying this to use a sensible but non-default switch width triggers a difference in the produced In [4]: from openff.toolkit.topology import Molecule
...: from openff.toolkit.typing.engines.smirnoff import ForceField
...: from openff.units import unit
...:
...: molecule = Molecule.from_smiles("CCC")
...:
...: # Create a system from a default FF.
...: force_field = ForceField("openff-1.3.0.offxml")
...:
...: system_original = force_field.create_openmm_system(molecule.to_topology())
...: system_original_str = openmm.XmlSerializer.serialize(system_original)
...:
...: # Create a system from a modified version of the FF
...: force_field = ForceField("openff-1.3.0.offxml")
...: force_field.get_parameter_handler("vdW").switch_width = 0.4444 * unit.angstroms
...:
...: system_modified = force_field.create_openmm_system(molecule.to_topology())
...: system_modified_str = openmm.XmlSerializer.serialize(system_modified)
...:
...: assert system_original_str != system_modified_str
/Users/mattthompson/mambaforge/envs/test/lib/python3.10/site-packages/openff/interchange/components/interchange.py:339: UserWarning: Automatically up-converting BondHandler from version 0.3 to 0.4. Consider manually upgrading this BondHandler (or <Bonds> section in an OFFXML file) to 0.4 or newer. For more details, see https://openforcefield.github.io/standards/standards/smirnoff/#bonds.
warnings.warn(
# No errors For good measure, the control case fails as expected - so the differences must be due to modifying the switch width in the force field! In [5]: from openff.toolkit.topology import Molecule
...: from openff.toolkit.typing.engines.smirnoff import ForceField
...: from openff.units import unit
...:
...: molecule = Molecule.from_smiles("CCC")
...:
...: # Create a system from a default FF.
...: force_field = ForceField("openff-1.3.0.offxml")
...:
...: system_original = force_field.create_openmm_system(molecule.to_topology())
...: system_original_str = openmm.XmlSerializer.serialize(system_original)
...:
...: # Create a system from a modified version of the FF
...: force_field = ForceField("openff-1.3.0.offxml")
...: # force_field.get_parameter_handler("vdW").switch_width = 0.4444 * unit.angstroms
...:
...: system_modified = force_field.create_openmm_system(molecule.to_topology())
...: system_modified_str = openmm.XmlSerializer.serialize(system_modified)
...:
...: assert system_original_str != system_modified_str
/Users/mattthompson/mambaforge/envs/test/lib/python3.10/site-packages/openff/interchange/components/interchange.py:339: UserWarning: Automatically up-converting BondHandler from version 0.3 to 0.4. Consider manually upgrading this BondHandler (or <Bonds> section in an OFFXML file) to 0.4 or newer. For more details, see https://openforcefield.github.io/standards/standards/smirnoff/#bonds.
warnings.warn(
---------------------------------------------------------------------------
AssertionError Traceback (most recent call last)
Cell In[5], line 20
17 system_modified = force_field.create_openmm_system(molecule.to_topology())
18 system_modified_str = openmm.XmlSerializer.serialize(system_modified)
---> 20 assert system_original_str != system_modified_str
AssertionError: |
Describe the bug
This
vdWHandler
exposes aswitch_width
property but as far as I can tell this doesn't get used anywhere in the code.To Reproduce
Output
The full error message (may be large, that's fine. Better to paste too much than too little.)
Computing environment (please complete the following information):
conda list
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: