Skip to content

Commit

Permalink
Merge pull request #909 from openforcefield/json-roundtrip-bugfixes
Browse files Browse the repository at this point in the history
Do not ignore float/int in JSON input
  • Loading branch information
mattwthompson authored Feb 21, 2024
2 parents b516c99 + 8b66143 commit 5db8bce
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 2 deletions.
4 changes: 4 additions & 0 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Dates are given in YYYY-MM-DD format.

Please note that all releases prior to a version 1.0.0 are considered pre-releases and many API changes will come before a stable release.

## Current development

* #909 Fixes a bug in which numerical values such as `scale_14` were lost when parsing JSON dumps.

## 0.3.21 - 2023-02-20

* #906 Fixes a bug in which intramolecular interactions between virtual sites were not properly excluded with OpenMM.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Energy tests directly related to the SMIRNOFF submodule."""
27 changes: 27 additions & 0 deletions openff/interchange/_tests/energy_tests/smirnoff/test_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from openff.toolkit import Quantity
from openff.utilities.testing import skip_if_missing

from openff.interchange import Interchange
from openff.interchange._tests import MoleculeWithConformer, needs_gmx
from openff.interchange.drivers import get_gromacs_energies, get_openmm_energies


@needs_gmx
@skip_if_missing("openmm")
def test_issue_908(sage_unconstrained):
molecule = MoleculeWithConformer.from_smiles("ClCCl")
topology = molecule.to_topology()
topology.box_vectors = Quantity([4, 4, 4], "nanometer")

state1 = sage_unconstrained.create_interchange(topology)

with open("test.json", "w") as f:
f.write(state1.json())

state2 = Interchange.parse_file("test.json")

get_gromacs_energies(state1).compare(get_gromacs_energies(state2))
get_openmm_energies(
state1,
combine_nonbonded_forces=False,
).compare(get_openmm_energies(state2, combine_nonbonded_forces=False))
21 changes: 20 additions & 1 deletion openff/interchange/_tests/unit_tests/smirnoff/test_base.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import random

import pytest
from openff.toolkit.topology import Topology
from openff.toolkit.typing.engines.smirnoff.parameters import (
Expand All @@ -6,7 +8,11 @@
)

from openff.interchange.exceptions import InvalidParameterHandlerError
from openff.interchange.smirnoff import SMIRNOFFAngleCollection, SMIRNOFFCollection
from openff.interchange.smirnoff import (
SMIRNOFFAngleCollection,
SMIRNOFFCollection,
SMIRNOFFElectrostaticsCollection,
)


class TestSMIRNOFFCollection:
Expand Down Expand Up @@ -49,3 +55,16 @@ def supported_parameters(cls):
parameter_handler=angle_Handler,
topology=Topology(),
)


def test_json_roundtrip_preserves_float_values():
"""Reproduce issue #908."""
scale_factor = 0.5 + random.random() * 0.5

collection = SMIRNOFFElectrostaticsCollection(scale_14=scale_factor)

assert collection.scale_14 == scale_factor

roundtripped = SMIRNOFFElectrostaticsCollection.parse_raw(collection.json())

assert roundtripped.scale_14 == scale_factor
6 changes: 5 additions & 1 deletion openff/interchange/smirnoff/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ def collection_loader(data: str) -> dict:
tmp: dict[str, Optional[Union[int, bool, str]]] = {}

for key, val in json.loads(data).items():
if isinstance(val, (str, bool, type(None))):
if val is None:
tmp[key] = val
elif isinstance(val, (int, float, bool)):
tmp[key] = val
elif isinstance(val, (str)):
# These are stored as string but must be parsed into `Quantity`
if key in ("cutoff", "switch_width"):
tmp[key] = unit.Quantity(*json.loads(val).values()) # type: ignore[arg-type]
Expand Down

0 comments on commit 5db8bce

Please sign in to comment.