Merge pull request #244 from OpenBioSim/fix_241-243

Fix issues #241, #242, and #243

python/BioSimSpace/IO/_io.py

@@ -566,7 +566,7 @@ def readMolecules(
         prop = property_map.get("time", "time")
         time = system.property(prop)
         system.removeSharedProperty(prop)
-        system.setProperties(prop, time)
+        system.setProperty(prop, time)
     except:
         pass
 
@@ -1148,6 +1148,24 @@ def readPerturbableSystem(top0, coords0, top1, coords1, property_map={}):
     # Update the molecule in the original system.
     system0.updateMolecules(mol)
 
+    # Remove "space" and "time" shared properties since this causes incorrect
+    # behaviour when extracting molecules and recombining them to make other
+    # systems.
+    try:
+        # Space.
+        prop = property_map.get("space", "space")
+        space = system0._sire_object.property(prop)
+        system0._sire_object.removeSharedProperty(prop)
+        system0._sire_object.setProperty(prop, space)
+
+        # Time.
+        prop = property_map.get("time", "time")
+        time = system0._sire_object.property(prop)
+        system0._sire_object.removeSharedProperty(prop)
+        system0._sire_object.setProperty(prop, time)
+    except:
+        pass
+
     return system0
 
 

python/BioSimSpace/Parameters/_Protocol/_openforcefield.py

@@ -37,7 +37,6 @@
 
 import os as _os
 
-_parmed = _try_import("parmed")
 import queue as _queue
 import subprocess as _subprocess
 
@@ -189,10 +188,6 @@ def run(self, molecule, work_dir=None, queue=None):
         else:
             is_smiles = False
 
-        # The following is adapted from the Open Force Field examples, where an
-        # OpenFF system is converted to AMBER format files using ParmEd:
-        # https://github.com/openforcefield/openff-toolkit/blob/master/examples/using_smirnoff_in_amber_or_gromacs/convert_to_amber_gromacs.ipynb
-
         if is_smiles:
             # Convert SMILES string to an OpenFF molecule.
             try:
@@ -353,7 +348,7 @@ def run(self, molecule, work_dir=None, queue=None):
             if par_mol.nMolecules() == 1:
                 par_mol = par_mol.getMolecules()[0]
         except Exception as e:
-            msg = "Failed to read molecule from: 'parmed.prmtop', 'parmed.inpcrd'"
+            msg = "Failed to read molecule from: 'interchange.prmtop', 'interchange.inpcrd'"
             if _isVerbose():
                 msg += ": " + getattr(e, "message", repr(e))
                 raise IOError(msg) from e

python/BioSimSpace/Process/_amber.py

@@ -46,6 +46,7 @@
 from .._Config import Amber as _AmberConfig
 from .._Exceptions import IncompatibleError as _IncompatibleError
 from .._Exceptions import MissingSoftwareError as _MissingSoftwareError
+from ..Protocol._free_energy_mixin import _FreeEnergyMixin
 from ..Protocol._position_restraint_mixin import _PositionRestraintMixin
 from .._SireWrappers import System as _System
 from ..Types._type import Type as _Type
@@ -126,7 +127,7 @@ def __init__(
         )
 
         # Catch unsupported protocols.
-        if isinstance(protocol, _Protocol.FreeEnergy):
+        if isinstance(protocol, _FreeEnergyMixin):
             raise _IncompatibleError(
                 "Unsupported protocol: '%s'" % self._protocol.__class__.__name__
             )

python/BioSimSpace/Process/_gromacs.py

@@ -52,6 +52,7 @@
 from .. import _isVerbose
 from .._Config import Gromacs as _GromacsConfig
 from .._Exceptions import MissingSoftwareError as _MissingSoftwareError
+from ..Protocol._free_energy_mixin import _FreeEnergyMixin
 from ..Protocol._position_restraint_mixin import _PositionRestraintMixin
 from .._SireWrappers import System as _System
 from ..Types._type import Type as _Type
@@ -232,7 +233,7 @@ def _setup(self):
         # Create a copy of the system.
         system = self._system.copy()
 
-        if isinstance(self._protocol, _Protocol.FreeEnergy):
+        if isinstance(self._protocol, _FreeEnergyMixin):
             # Check that the system contains a perturbable molecule.
             if self._system.nPerturbableMolecules() == 0:
                 raise ValueError(
@@ -2544,10 +2545,15 @@ def _getFinalFrame(self):
                 space_prop in old_system._sire_object.propertyKeys()
                 and space_prop in new_system._sire_object.propertyKeys()
             ):
-                box = new_system._sire_object.property("space")
-                old_system._sire_object.setProperty(
-                    self._property_map.get("space", "space"), box
-                )
+                # Get the original space.
+                box = old_system._sire_object.property("space")
+
+                # Only update the box if the space is periodic.
+                if box.isPeriodic():
+                    box = new_system._sire_object.property("space")
+                    old_system._sire_object.setProperty(
+                        self._property_map.get("space", "space"), box
+                    )
 
             # If this is a vacuum simulation, then translate the centre of mass
             # of the system back to the origin.
@@ -2655,11 +2661,16 @@ def _getFrame(self, time):
                     space_prop in old_system._sire_object.propertyKeys()
                     and space_prop in new_system._sire_object.propertyKeys()
                 ):
-                    box = new_system._sire_object.property("space")
+                    # Get the original space.
+                    box = old_system._sire_object.property("space")
+
+                    # Only update the box if the space is periodic.
                     if box.isPeriodic():
-                        old_system._sire_object.setProperty(
-                            self._property_map.get("space", "space"), box
-                        )
+                        box = new_system._sire_object.property("space")
+                        if box.isPeriodic():
+                            old_system._sire_object.setProperty(
+                                self._property_map.get("space", "space"), box
+                            )
 
                 # If this is a vacuum simulation, then translate the centre of mass
                 # of the system back to the origin.

python/BioSimSpace/Sandpit/Exscientia/IO/_io.py

@@ -566,7 +566,7 @@ def readMolecules(
         prop = property_map.get("time", "time")
         time = system.property(prop)
         system.removeSharedProperty(prop)
-        system.setProperties(prop, time)
+        system.setProperty(prop, time)
     except:
         pass
 
@@ -1148,6 +1148,24 @@ def readPerturbableSystem(top0, coords0, top1, coords1, property_map={}):
     # Update the molecule in the original system.
     system0.updateMolecules(mol)
 
+    # Remove "space" and "time" shared properties since this causes incorrect
+    # behaviour when extracting molecules and recombining them to make other
+    # systems.
+    try:
+        # Space.
+        prop = property_map.get("space", "space")
+        space = system0._sire_object.property(prop)
+        system0._sire_object.removeSharedProperty(prop)
+        system0._sire_object.setProperty(prop, space)
+
+        # Time.
+        prop = property_map.get("time", "time")
+        time = system0._sire_object.property(prop)
+        system0._sire_object.removeSharedProperty(prop)
+        system0._sire_object.setPropery(prop, time)
+    except:
+        pass
+
     return system0
 
 

python/BioSimSpace/Sandpit/Exscientia/Parameters/_Protocol/_openforcefield.py

@@ -37,7 +37,6 @@
 
 import os as _os
 
-_parmed = _try_import("parmed")
 import queue as _queue
 import subprocess as _subprocess
 
@@ -189,10 +188,6 @@ def run(self, molecule, work_dir=None, queue=None):
         else:
             is_smiles = False
 
-        # The following is adapted from the Open Force Field examples, where an
-        # OpenFF system is converted to AMBER format files using ParmEd:
-        # https://github.com/openforcefield/openff-toolkit/blob/master/examples/using_smirnoff_in_amber_or_gromacs/convert_to_amber_gromacs.ipynb
-
         if is_smiles:
             # Convert SMILES string to an OpenFF molecule.
             try:
@@ -353,7 +348,7 @@ def run(self, molecule, work_dir=None, queue=None):
             if par_mol.nMolecules() == 1:
                 par_mol = par_mol.getMolecules()[0]
         except Exception as e:
-            msg = "Failed to read molecule from: 'parmed.prmtop', 'parmed.inpcrd'"
+            msg = "Failed to read molecule from: 'interchange.prmtop', 'interchange.inpcrd'"
             if _isVerbose():
                 msg += ": " + getattr(e, "message", repr(e))
                 raise IOError(msg) from e

python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py

@@ -2638,10 +2638,15 @@ def _getFinalFrame(self):
                 space_prop in old_system._sire_object.propertyKeys()
                 and space_prop in new_system._sire_object.propertyKeys()
             ):
-                box = new_system._sire_object.property("space")
-                old_system._sire_object.setProperty(
-                    self._property_map.get("space", "space"), box
-                )
+                # Get the original space.
+                box = old_system._sire_object.property("space")
+
+                # Only update the box if the space is periodic.
+                if box.isPeriodic():
+                    box = new_system._sire_object.property("space")
+                    old_system._sire_object.setProperty(
+                        self._property_map.get("space", "space"), box
+                    )
 
             # If this is a vacuum simulation, then translate the centre of mass
             # of the system back to the origin.
@@ -2749,11 +2754,16 @@ def _getFrame(self, time):
                     space_prop in old_system._sire_object.propertyKeys()
                     and space_prop in new_system._sire_object.propertyKeys()
                 ):
-                    box = new_system._sire_object.property("space")
+                    # Get the original space.
+                    box = old_system._sire_object.property("space")
+
+                    # Only update the box if the space is periodic.
                     if box.isPeriodic():
-                        old_system._sire_object.setProperty(
-                            self._property_map.get("space", "space"), box
-                        )
+                        box = new_system._sire_object.property("space")
+                        if box.isPeriodic():
+                            old_system._sire_object.setProperty(
+                                self._property_map.get("space", "space"), box
+                            )
 
                 # If this is a vacuum simulation, then translate the centre of mass
                 # of the system back to the origin.