Skip to content

Add vdWHandler.long_range_dispersion #1351

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.

Sign up for GitHub

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

Closed
mattwthompson wants to merge 1 commit into from
Closed

Add vdWHandler.long_range_dispersion #1351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions openff/toolkit/tests/test_parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,30 @@ def test_set_invalid_scale_factor(self):
handler.scale15 = 0.1


class TestvdwHandlerUpconversion:
"""
Test a proposed implementation of OFF-EP-0007:

https://github.com/openforcefield/standards/pull/40
"""

def test_upconversion(self):
handler = vdWHandler(version=0.3)
assert handler.version == Version("0.4")

assert handler.long_range_dispersion == "isotropic"

@pytest.mark.parametrize(
"version,exception_type", [(0.3, NotImplementedError), (0.4, SMIRNOFFSpecError)]
)
def test_invalid_dispersion(self, version, exception_type):
with pytest.raises(
exception_type,
match="long_range_dispersion.*magic",
):
vdWHandler(version=version, long_range_dispersion="magic")


class TestvdWType:
"""
Test the behavior of vdWType
Expand Down
31 changes: 31 additions & 0 deletions openff/toolkit/typing/engines/smirnoff/parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,7 @@ def to_dict(

_TAGNAME = "vdW" # SMIRNOFF tag name to process
_INFOTYPE = vdWType # info type to store
_MAX_SUPPORTED_SECTION_VERSION = Version("0.4")

potential = ParameterAttribute(
default="Lennard-Jones-12-6", converter=_allow_only(["Lennard-Jones-12-6"])
Expand All @@ -2828,6 +2829,9 @@ def to_dict(
method = ParameterAttribute(
default="cutoff", converter=_allow_only(["cutoff", "PME"])
)
long_range_dispersion = ParameterAttribute(
default="isotropic", converter=_allow_only(["isotropic", "none"])
)

# TODO: Use _allow_only when ParameterAttribute will support multiple converters
# (it'll be easy when we switch to use the attrs library)
Expand Down Expand Up @@ -2861,6 +2865,33 @@ def scale15(self, attrs, new_scale15):
# Tolerance when comparing float attributes for handler compatibility.
_SCALETOL = 1e-5

def __init__(self, **kwargs):
if kwargs.get("version") == 0.3:

logger.info("Attempting to up-convert vdW section from 0.3 to 0.4")

if kwargs.get("long_range_dispersion") in ["isotropic", None]:
kwargs["long_range_dispersion"] = "isotropic"
logger.info(
"Successfully up-converted vdW section from 0.3 to 0.4. "
'`long_range_dispersion="isotropic"` is now set.'
)
kwargs["version"] = 0.4
elif kwargs.get("long_range_dispersion") == "none":
# store as "none" or None?
kwargs["long_range_dispersion"] = "none"
logger.info(
"Successfully up-converted vdW section from 0.3 to 0.4. "
'`long_range_dispersion="isotropic"` is now set.'
)
kwargs["version"] = 0.4
else:
raise NotImplementedError(
"Failed to up-convert Electrostatics section from 0.3 to 0.4. Did not know "
f'how to process `long_range_dispersion="{kwargs["long_range_dispersion"]}"`.'
)
super().__init__(**kwargs)

def check_handler_compatibility(self, other_handler):
"""
Checks whether this ParameterHandler encodes compatible physics as another ParameterHandler. This is
Expand Down