Backport fixes from PR #89.

michellab · Jun 7, 2023 · 128fbe2 · 128fbe2
1 parent f7fad68
commit 128fbe2
Show file tree

Hide file tree

Showing 10 changed files with 288 additions and 8 deletions.
diff --git a/python/BioSimSpace/IO/_file_cache.py b/python/BioSimSpace/IO/_file_cache.py
@@ -68,6 +68,11 @@ def __setitem__(self, key, value):
                 key, value = self.popitem(False)
                 self._num_atoms -= value[0].nAtoms()
 
+    def __delitem__(self, key):
+        value = self[key]
+        self._num_atoms -= value[0].nAtoms()
+        _collections.OrderedDict.__delitem__(self, key)
+
 
 # Initialise a "cache" dictionary. This maps a key of the system UID, file format
 # and excluded properties a value of the system and file path. When saving to a
@@ -146,6 +151,7 @@ def check_cache(
     key = (
         system._sire_object.uid().toString(),
         format,
+        _compress_molnum_key(str(system._mol_nums)),
         str(set(excluded_properties)),
         str(skip_water),
     )
@@ -262,12 +268,13 @@ def update_cache(
     key = (
         system._sire_object.uid().toString(),
         format,
+        _compress_molnum_key(str(system._mol_nums)),
         str(set(excluded_properties)),
         str(skip_water),
     )
 
     # Update the cache.
-    _cache[key] = (system, path, hash)
+    _cache[key] = (system.copy(), path, hash)
 
 
 def _get_md5_hash(path):
@@ -287,3 +294,10 @@ def _get_md5_hash(path):
             hash.update(chunk)
 
     return hash.hexdigest()
+
+
+def _compress_molnum_key(str):
+    """
+    Internal helper function to compress the MolNum list section of the key.
+    """
+    return str.replace("MolNum(", "").replace(")", "")
diff --git a/python/BioSimSpace/Sandpit/Exscientia/IO/_file_cache.py b/python/BioSimSpace/Sandpit/Exscientia/IO/_file_cache.py
@@ -68,6 +68,11 @@ def __setitem__(self, key, value):
                 key, value = self.popitem(False)
                 self._num_atoms -= value[0].nAtoms()
 
+    def __delitem__(self, key):
+        value = self[key]
+        self._num_atoms -= value[0].nAtoms()
+        _collections.OrderedDict.__delitem__(self, key)
+
 
 # Initialise a "cache" dictionary. This maps a key of the system UID, file format
 # and excluded properties a value of the system and file path. When saving to a
@@ -146,6 +151,7 @@ def check_cache(
     key = (
         system._sire_object.uid().toString(),
         format,
+        _compress_molnum_key(str(system._mol_nums)),
         str(set(excluded_properties)),
         str(skip_water),
     )
@@ -262,12 +268,13 @@ def update_cache(
     key = (
         system._sire_object.uid().toString(),
         format,
+        _compress_molnum_key(str(system._mol_nums)),
         str(set(excluded_properties)),
         str(skip_water),
     )
 
     # Update the cache.
-    _cache[key] = (system, path, hash)
+    _cache[key] = (system.copy(), path, hash)
 
 
 def _get_md5_hash(path):
@@ -287,3 +294,10 @@ def _get_md5_hash(path):
             hash.update(chunk)
 
     return hash.hexdigest()
+
+
+def _compress_molnum_key(str):
+    """
+    Internal helper function to compress the MolNum list section of the key.
+    """
+    return str.replace("MolNum(", "").replace(")", "")
diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py
@@ -471,6 +471,10 @@ def isSame(
         ):
             return False
 
+        # Make sure that the molecule numbers in the system match.
+        if self._mol_nums != other._mol_nums:
+            return False
+
         # Invert the property maps.
         inv_prop_map0 = {v: k for k, v in property_map0.items()}
         inv_prop_map1 = {v: k for k, v in property_map1.items()}

diff --git a/python/BioSimSpace/_SireWrappers/_system.py b/python/BioSimSpace/_SireWrappers/_system.py
@@ -471,6 +471,10 @@ def isSame(
         ):
             return False
 
+        # Make sure that the molecule numbers in the system match.
+        if self._mol_nums != other._mol_nums:
+            return False
+
         # Invert the property maps.
         inv_prop_map0 = {v: k for k, v in property_map0.items()}
         inv_prop_map1 = {v: k for k, v in property_map1.items()}

diff --git a/tests/IO/test_file_cache.py b/tests/IO/test_file_cache.py
@@ -1,10 +1,26 @@
 import BioSimSpace as BSS
 
+from BioSimSpace._Utils import _try_import, _have_imported
+
 import glob
 import os
 import pytest
 import tempfile
 
+# Make sure AMBER is installed.
+if BSS._amber_home is not None:
+    exe = "%s/bin/sander" % BSS._amber_home
+    if os.path.isfile(exe):
+        has_amber = True
+    else:
+        has_amber = False
+else:
+    has_amber = False
+
+# Make sure openff is installed.
+_openff = _try_import("openff")
+has_openff = _have_imported(_openff)
+
 
 def test_file_cache():
     """
@@ -13,7 +29,7 @@ def test_file_cache():
     """
 
     # Clear the file cache.
-    BSS.IO._file_cache._cache = {}
+    BSS.IO._file_cache._cache = BSS.IO._file_cache._FixedSizeOrderedDict()
 
     # Load the molecular system.
     s = BSS.IO.readMolecules(["tests/input/ala.crd", "tests/input/ala.top"])
@@ -62,3 +78,57 @@ def test_file_cache():
 
     # The directory should now contain 7 files.
     assert len(glob.glob(f"{tmp_path}/*")) == 7
+
+    # Now delete a key and check that the number of atoms is decremented.
+
+    # Get the first key.
+    key = list(BSS.IO._file_cache._cache.keys())[0]
+
+    # Store the number of atoms for this key.
+    num_atoms = BSS.IO._file_cache._cache[key][0].nAtoms()
+
+    # Store the total number of atoms in the cache.
+    total_atoms = BSS.IO._file_cache._cache._num_atoms
+
+    # Delete the key.
+    del BSS.IO._file_cache._cache[key]
+
+    # Make sure the number of atoms in the cache was decremented.
+    assert BSS.IO._file_cache._cache._num_atoms == total_atoms - num_atoms
+
+
+@pytest.mark.skipif(
+    has_amber is False or has_openff is False,
+    reason="Requires AMBER and OpenFF to be installed",
+)
+def test_file_cache_mol_nuums():
+    """
+    Make sure that systems can be cached if they have the same UID, but
+    contain different MolNUms.
+    """
+
+    # Clear the file cache.
+    BSS.IO._file_cache._cache = BSS.IO._file_cache._FixedSizeOrderedDict()
+
+    # Create an initial system.
+    system = BSS.Parameters.openff_unconstrained_2_0_0("CO").getMolecule().toSystem()
+
+    # Create two different 5 atom molecules.
+    mol0 = BSS.Parameters.openff_unconstrained_2_0_0("C").getMolecule()
+    mol1 = BSS.Parameters.openff_unconstrained_2_0_0("CF").getMolecule()
+
+    # Create two new systems by adding the different molecules to the original
+    # system. These will have the same UID, but different molecule numbers.
+    system0 = system + mol0
+    system1 = system + mol1
+
+    # Create a temporary working directory.
+    tmp_dir = tempfile.TemporaryDirectory()
+    tmp_path = tmp_dir.name
+
+    # Write the two systems to PDB format.
+    BSS.IO.saveMolecules(f"{tmp_path}/tmp0", system0, "pdb")
+    BSS.IO.saveMolecules(f"{tmp_path}/tmp1", system1, "pdb")
+
+    # The cache shold have two entries.
+    assert len(BSS.IO._file_cache._cache) == 2
diff --git a/tests/Process/test_openmm.py b/tests/Process/test_openmm.py
@@ -2,8 +2,19 @@
 
 from BioSimSpace._Utils import _try_import, _have_imported
 
+import os
 import pytest
 
+# Make sure AMBER is installed.
+if BSS._amber_home is not None:
+    exe = "%s/bin/sander" % BSS._amber_home
+    if os.path.isfile(exe):
+        has_amber = True
+    else:
+        has_amber = False
+else:
+    has_amber = False
+
 # Make sure GROMACS is installed.
 has_gromacs = BSS._gmx_exe is not None
 
@@ -90,8 +101,8 @@ def test_production(system, restraint):
 
 
 @pytest.mark.skipif(
-    has_gromacs is False or has_openff is False,
-    reason="Requires GROMACS and OpenFF to be installed",
+    has_amber is False or has_gromacs is False or has_openff is False,
+    reason="Requires AMBER, GROMACS, and OpenFF to be installed",
 )
 def test_rhombic_dodecahedron():
     """Test that OpenMM can load and run rhombic dodecahedral triclinic spaces."""

diff --git a/tests/Sandpit/Exscientia/IO/test_file_cache.py b/tests/Sandpit/Exscientia/IO/test_file_cache.py
@@ -1,10 +1,26 @@
 import BioSimSpace.Sandpit.Exscientia as BSS
 
+from BioSimSpace.Sandpit.Exscientia._Utils import _try_import, _have_imported
+
 import glob
 import os
 import pytest
 import tempfile
 
+# Make sure AMBER is installed.
+if BSS._amber_home is not None:
+    exe = "%s/bin/sander" % BSS._amber_home
+    if os.path.isfile(exe):
+        has_amber = True
+    else:
+        has_amber = False
+else:
+    has_amber = False
+
+# Make sure openff is installed.
+_openff = _try_import("openff")
+has_openff = _have_imported(_openff)
+
 
 def test_file_cache():
     """
@@ -13,7 +29,7 @@ def test_file_cache():
     """
 
     # Clear the file cache.
-    BSS.IO._file_cache._cache = {}
+    BSS.IO._file_cache._cache = BSS.IO._file_cache._FixedSizeOrderedDict()
 
     # Load the molecular system.
     s = BSS.IO.readMolecules(["tests/input/ala.crd", "tests/input/ala.top"])
@@ -62,3 +78,57 @@ def test_file_cache():
 
     # The directory should now contain 7 files.
     assert len(glob.glob(f"{tmp_path}/*")) == 7
+
+    # Now delete a key and check that the number of atoms is decremented.
+
+    # Get the first key.
+    key = list(BSS.IO._file_cache._cache.keys())[0]
+
+    # Store the number of atoms for this key.
+    num_atoms = BSS.IO._file_cache._cache[key][0].nAtoms()
+
+    # Store the total number of atoms in the cache.
+    total_atoms = BSS.IO._file_cache._cache._num_atoms
+
+    # Delete the key.
+    del BSS.IO._file_cache._cache[key]
+
+    # Make sure the number of atoms in the cache was decremented.
+    assert BSS.IO._file_cache._cache._num_atoms == total_atoms - num_atoms
+
+
+@pytest.mark.skipif(
+    has_amber is False or has_openff is False,
+    reason="Requires AMBER and OpenFF to be installed",
+)
+def test_file_cache_mol_nuums():
+    """
+    Make sure that systems can be cached if they have the same UID, but
+    contain different MolNUms.
+    """
+
+    # Clear the file cache.
+    BSS.IO._file_cache._cache = BSS.IO._file_cache._FixedSizeOrderedDict()
+
+    # Create an initial system.
+    system = BSS.Parameters.openff_unconstrained_2_0_0("CO").getMolecule().toSystem()
+
+    # Create two different 5 atom molecules.
+    mol0 = BSS.Parameters.openff_unconstrained_2_0_0("C").getMolecule()
+    mol1 = BSS.Parameters.openff_unconstrained_2_0_0("CF").getMolecule()
+
+    # Create two new systems by adding the different molecules to the original
+    # system. These will have the same UID, but different molecule numbers.
+    system0 = system + mol0
+    system1 = system + mol1
+
+    # Create a temporary working directory.
+    tmp_dir = tempfile.TemporaryDirectory()
+    tmp_path = tmp_dir.name
+
+    # Write the two systems to PDB format.
+    BSS.IO.saveMolecules(f"{tmp_path}/tmp0", system0, "pdb")
+    BSS.IO.saveMolecules(f"{tmp_path}/tmp1", system1, "pdb")
+
+    # The cache shold have two entries.
+    assert len(BSS.IO._file_cache._cache) == 2
diff --git a/tests/Sandpit/Exscientia/Process/test_openmm.py b/tests/Sandpit/Exscientia/Process/test_openmm.py
@@ -2,8 +2,19 @@
 
 from BioSimSpace.Sandpit.Exscientia._Utils import _try_import, _have_imported
 
+import os
 import pytest
 
+# Make sure AMBER is installed.
+if BSS._amber_home is not None:
+    exe = "%s/bin/sander" % BSS._amber_home
+    if os.path.isfile(exe):
+        has_amber = True
+    else:
+        has_amber = False
+else:
+    has_amber = False
+
 # Make sure GROMACS is installed.
 has_gromacs = BSS._gmx_exe is not None
 
@@ -83,8 +94,8 @@ def test_production(system):
 
 
 @pytest.mark.skipif(
-    has_gromacs is False or has_openff is False,
-    reason="Requires GROMACS and OpenFF to be installed",
+    has_amber is False or has_gromacs is False or has_openff is False,
+    reason="Requires AMBER, GROMACS, and OpenFF to be installed",
 )
 def test_rhombic_dodecahedron():
     """Test that OpenMM can load and run rhombic dodecahedral triclinic spaces."""