Skip to content

Commit

Permalink
BUG: Clear charge cache before (re-?)building
Browse files Browse the repository at this point in the history
  • Loading branch information
mattwthompson committed Oct 4, 2024
1 parent 4099a1e commit 25b144e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 18 deletions.
27 changes: 9 additions & 18 deletions openff/interchange/_tests/unit_tests/smirnoff/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,11 @@ def test_virtual_site_charge_increments_applied_after_preset_charges_water(
)

with pytest.warns(PresetChargesAndVirtualSitesWarning):
out = tip4p.create_interchange(
# This dict is probably ordered O H H VS
charges = tip4p.create_interchange(
water.to_topology(),
charge_from_molecules=[water],
)

# This dict is probably ordered O H H VS
charges = out["Electrostatics"].charges
)["Electrostatics"].charges

# tip4p_fb.offxml is meant to result in charges of -1.0517362213526 (VS) 0 (O) and 0.5258681106763 (H)
# the force field has 0.0 for all library charges (skipping AM1-BCC), so preset charges screw this up.
Expand All @@ -300,9 +298,6 @@ def test_virtual_site_charge_increments_applied_after_preset_charges_water(
[-2.0, 1.5258681106763001, 1.5258681106763001, -1.0517362213526],
)

# The cache seems to leak charges between tests
out["Electrostatics"]._charges.clear()

def test_virtual_site_charge_increments_applied_after_preset_charges_ligand(
self,
sage_with_sigma_hole,
Expand All @@ -314,13 +309,12 @@ def test_virtual_site_charge_increments_applied_after_preset_charges_ligand(
"elementary_charge",
)

out = sage_with_sigma_hole.create_interchange(
ligand.to_topology(),
charge_from_molecules=[ligand],
)

# This dict is probably ordered C Cl Cl Cl Cl VS VS VS VS
charges = out["Electrostatics"].charges
with pytest.warns(PresetChargesAndVirtualSitesWarning):
# This dict is probably ordered C Cl Cl Cl Cl VS VS VS VS
charges = sage_with_sigma_hole.create_interchange(
ligand.to_topology(),
charge_from_molecules=[ligand],
)["Electrostatics"].charges

# this fake sigma hole parameter shifts 0.1 e from the carbon and 0.2 e from the chlorine, so the
# resulting charges should be like
Expand All @@ -332,9 +326,6 @@ def test_virtual_site_charge_increments_applied_after_preset_charges_ligand(
[1.6, -0.1, -0.1, -0.1, -0.1, -0.3, -0.3, -0.3, -0.3],
)

# The cache seems to leak charges between tests
out["Electrostatics"]._charges.clear()


@skip_if_missing("openmm")
class TestPartialBondOrdersFromMolecules:
Expand Down
13 changes: 13 additions & 0 deletions openff/interchange/smirnoff/_nonbonded.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,19 @@ def charges(
) -> dict[TopologyKey | LibraryChargeTopologyKey | VirtualSiteKey, _ElementaryChargeQuantity]:
"""Get the total partial charge on each atom, including virtual sites."""
if len(self._charges) == 0 or self._charges_cached is False:
# TODO: Clearing this dict **should not be necessary** but in some cases in appears
# that this class attribute persists in some cases, possibly only on a single
# thread. Ideas to try include
# * Drop @computed_field ?
# * Don't handle caching ourselves and let Pydantic do it (i.e. have this
# function simply retrun ._get_charges(...)
#
# Hopefully this isn't a major issue - caches for large systems should still
# only be build once
#
# Some more context: https://github.com/openforcefield/openff-interchange/issues/842#issuecomment-2394211357
self._charges.clear()

self._charges.update(self._get_charges(include_virtual_sites=True))
self._charges_cached = True

Expand Down

0 comments on commit 25b144e

Please sign in to comment.