From 605e2d159cda3fd369928e2823ed254565c8d34a Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Mon, 3 Jul 2023 18:25:14 +0100 Subject: [PATCH 01/17] Backport fix from PR #125. [ci skip] --- .github/workflows/devel.yaml | 10 +++++----- .github/workflows/pr.yaml | 10 +++++----- python/BioSimSpace/Process/_gromacs.py | 13 +++++++++---- python/BioSimSpace/Process/_openmm.py | 10 +++++++--- .../Sandpit/Exscientia/Process/_openmm.py | 8 +++++++- .../Sandpit/Exscientia/Protocol/_config.py | 8 +++++++- python/BioSimSpace/_Config/_config.py | 12 ++++++++++-- 7 files changed, 50 insertions(+), 21 deletions(-) diff --git a/.github/workflows/devel.yaml b/.github/workflows/devel.yaml index 797741d97..c94a3cc27 100644 --- a/.github/workflows/devel.yaml +++ b/.github/workflows/devel.yaml @@ -13,7 +13,7 @@ jobs: max-parallel: 9 fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10"] + python-version: ["3.9", "3.10", "3.11"] platform: - { name: "windows", os: "windows-latest", shell: "pwsh" } - { name: "linux", os: "ubuntu-latest", shell: "bash -l {0}" } @@ -22,14 +22,14 @@ jobs: # Exclude all but the latest Python from all but Linux - platform: { name: "macos", os: "macos-latest", shell: "bash -l {0}" } - python-version: "3.8" + python-version: "3.9" - platform: { name: "windows", os: "windows-latest", shell: "pwsh" } - python-version: "3.8" + python-version: "3.9" - platform: { name: "macos", os: "macos-latest", shell: "bash -l {0}" } - python-version: "3.9" + python-version: "3.10" - platform: { name: "windows", os: "windows-latest", shell: "pwsh" } - python-version: "3.9" + python-version: "3.10" environment: name: biosimspace-build defaults: diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 6a4f15459..5e9268b58 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -12,7 +12,7 @@ jobs: max-parallel: 9 fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10"] + python-version: ["3.9", "3.10", "3.11"] platform: - { name: "windows", os: "windows-latest", shell: "pwsh" } - { name: "linux", os: "ubuntu-latest", shell: "bash -l {0}" } @@ -22,14 +22,14 @@ jobs: # but Linux - platform: { name: "macos", os: "macos-latest", shell: "bash -l {0}" } - python-version: "3.8" + python-version: "3.9" - platform: { name: "windows", os: "windows-latest", shell: "pwsh" } - python-version: "3.8" + python-version: "3.9" - platform: { name: "macos", os: "macos-latest", shell: "bash -l {0}" } - python-version: "3.9" + python-version: "3.10" - platform: { name: "windows", os: "windows-latest", shell: "pwsh" } - python-version: "3.9" + python-version: "3.10" environment: name: biosimspace-build defaults: diff --git a/python/BioSimSpace/Process/_gromacs.py b/python/BioSimSpace/Process/_gromacs.py index a3a3dd77a..5994f4e2e 100644 --- a/python/BioSimSpace/Process/_gromacs.py +++ b/python/BioSimSpace/Process/_gromacs.py @@ -286,10 +286,15 @@ def _generate_config(self): """Generate GROMACS configuration file strings.""" # Check whether the system contains periodic box information. - # For now, well not attempt to generate a box if the system property - # is missing. If no box is present, we'll assume a non-periodic simulation. - if "space" in self._system._sire_object.propertyKeys(): - has_box = True + space_prop = self._property_map.get("space", "space") + if space_prop in self._system._sire_object.propertyKeys(): + try: + # Make sure that we have a periodic box. The system will now have + # a default cartesian space. + box = self._system._sire_object.property(space_prop) + has_box = box.isPeriodic() + except: + has_box = False else: _warnings.warn("No simulation box found. Assuming gas phase simulation.") has_box = False diff --git a/python/BioSimSpace/Process/_openmm.py b/python/BioSimSpace/Process/_openmm.py index dc3bd8d2a..54af93ab6 100644 --- a/python/BioSimSpace/Process/_openmm.py +++ b/python/BioSimSpace/Process/_openmm.py @@ -283,10 +283,14 @@ def _generate_config(self): prop = self._property_map.get("space", "space") # Check whether the system contains periodic box information. - # For now, we'll not attempt to generate a box if the system property - # is missing. If no box is present, we'll assume a non-periodic simulation. if prop in self._system._sire_object.propertyKeys(): - has_box = True + try: + # Make sure that we have a periodic box. The system will now have + # a default cartesian space. + box = self._system._sire_object.property(prop) + has_box = box.isPeriodic() + except: + has_box = False else: _warnings.warn("No simulation box found. Assuming gas phase simulation.") has_box = False diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py index a440a46d9..dcea9a2b5 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py @@ -285,7 +285,13 @@ def _generate_config(self): # For now, we'll not attempt to generate a box if the system property # is missing. If no box is present, we'll assume a non-periodic simulation. if prop in self._system._sire_object.propertyKeys(): - has_box = True + try: + # Make sure that we have a periodic box. The system will now have + # a default cartesian space. + box = self._system._sire_object.property(prop) + has_box = box.isPeriodic() + except: + has_box = False else: _warnings.warn("No simulation box found. Assuming gas phase simulation.") has_box = False diff --git a/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py b/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py index 987cbb520..e71e1369d 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py @@ -39,7 +39,13 @@ def __init__(self, system, protocol, explicit_dummies=False): def _has_box(self): """Return whether the current system has a box.""" if "space" in self.system._sire_object.propertyKeys(): - has_box = True + try: + # Make sure that we have a periodic box. The system will now have + # a default cartesian space. + box = self.system._sire_object.property("space") + has_box = box.isPeriodic() + except: + has_box = False else: _warnings.warn("No simulation box found. Assuming gas phase simulation.") has_box = False diff --git a/python/BioSimSpace/_Config/_config.py b/python/BioSimSpace/_Config/_config.py index b372ca7e6..d8b5ce7ee 100644 --- a/python/BioSimSpace/_Config/_config.py +++ b/python/BioSimSpace/_Config/_config.py @@ -91,10 +91,18 @@ def hasBox(self): """ space_prop = self._property_map.get("space", "space") if space_prop in self._system._sire_object.propertyKeys(): - return True + try: + # Make sure that we have a periodic box. The system will now have + # a default cartesian space. + box = self._system._sire_object.property(space_prop) + has_box = box.isPeriodic() + except: + has_box = False else: + has_box = False _warnings.warn("No simulation box found. Assuming gas phase simulation.") - return False + + return has_box def hasWater(self): """ From cbfef2e759728802e1f5ee89bd8e797085e6300e Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Fri, 7 Jul 2023 13:59:47 +0100 Subject: [PATCH 02/17] Backport fixes from PR #128. [ci skip] --- demo/interactive_md.ipynb | 10 +++++----- python/BioSimSpace/Process/_amber.py | 15 +++++++++++++-- python/BioSimSpace/Process/_gromacs.py | 15 +++++++++++++-- python/BioSimSpace/Process/_namd.py | 16 ++++++++++++++-- python/BioSimSpace/Process/_openmm.py | 17 ++++++++++++++--- python/BioSimSpace/Process/_plumed.py | 8 +++++++- python/BioSimSpace/Process/_somd.py | 15 +++++++++++++-- .../Sandpit/Exscientia/Process/_amber.py | 15 +++++++++++++-- .../Sandpit/Exscientia/Process/_gromacs.py | 15 +++++++++++++-- .../Sandpit/Exscientia/Process/_namd.py | 16 ++++++++++++++-- .../Sandpit/Exscientia/Process/_openmm.py | 17 ++++++++++++++--- .../Sandpit/Exscientia/Process/_plumed.py | 8 +++++++- .../Sandpit/Exscientia/Process/_somd.py | 15 +++++++++++++-- 13 files changed, 153 insertions(+), 29 deletions(-) diff --git a/demo/interactive_md.ipynb b/demo/interactive_md.ipynb index 0b826f4bc..34f2293ff 100644 --- a/demo/interactive_md.ipynb +++ b/demo/interactive_md.ipynb @@ -336,14 +336,14 @@ "outputs": [], "source": [ "# Search the system for a residue named ALA. Since there is a single match, we take the first result.\n", - "residue = system.search(\"resname ALA\")[0]\n", + "atoms = system.search(\"resname ALA\")\n", "\n", "# Get the indices of the atoms in the molecule, relative to the original system.\n", - "indices = [system.getIndex(x) for x in residue.getAtoms()]\n", + "indices = [system.getIndex(atom) for atom in atoms]\n", "\n", "# Compute the RMSD for each frame and plot the result.\n", "BSS.Notebook.plot(\n", - " y=process.getTrajectory().rmsd(frame=0, atoms=indices),\n", + " y=process.getTrajectory(backend=\"mdtraj\").rmsd(frame=0, atoms=indices),\n", " xlabel=\"Frame\",\n", " ylabel=\"RMSD\",\n", ")" @@ -352,7 +352,7 @@ ], "metadata": { "kernelspec": { - "display_name": "Python 3", + "display_name": "Python 3 (ipykernel)", "language": "python", "name": "python3" }, @@ -366,7 +366,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.7.3" + "version": "3.10.11" } }, "nbformat": 4, diff --git a/python/BioSimSpace/Process/_amber.py b/python/BioSimSpace/Process/_amber.py index 8533d325d..e755cf16c 100644 --- a/python/BioSimSpace/Process/_amber.py +++ b/python/BioSimSpace/Process/_amber.py @@ -461,13 +461,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -478,6 +483,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -489,7 +500,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Process/_gromacs.py b/python/BioSimSpace/Process/_gromacs.py index 5994f4e2e..98626325a 100644 --- a/python/BioSimSpace/Process/_gromacs.py +++ b/python/BioSimSpace/Process/_gromacs.py @@ -659,13 +659,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -676,6 +681,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -695,7 +706,7 @@ def getTrajectory(self, block="AUTO"): else: self._traj_file = traj_file - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Process/_namd.py b/python/BioSimSpace/Process/_namd.py index bb2b76d6c..571051e5c 100644 --- a/python/BioSimSpace/Process/_namd.py +++ b/python/BioSimSpace/Process/_namd.py @@ -813,13 +813,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -829,6 +834,13 @@ def getTrajectory(self, block="AUTO"): trajectory : :class:`Trajectory ` The latest trajectory object. """ + + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -840,7 +852,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Process/_openmm.py b/python/BioSimSpace/Process/_openmm.py index 54af93ab6..7a3f80a4d 100644 --- a/python/BioSimSpace/Process/_openmm.py +++ b/python/BioSimSpace/Process/_openmm.py @@ -1109,7 +1109,7 @@ def _generate_config(self): # Create a dummy PLUMED input file so that we can bind PLUMED # analysis functions to this process. - self._plumed = _Plumed(self._work_dir) + self._plumed = _Plumed(str(self._work_dir)) plumed_config, auxillary_files = self._plumed.createConfig( self._system, self._protocol, self._property_map ) @@ -1350,13 +1350,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -1367,6 +1372,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -1380,7 +1391,7 @@ def getTrajectory(self, block="AUTO"): if not _os.path.isfile(self._traj_file): return None else: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) def getFrame(self, index): """ diff --git a/python/BioSimSpace/Process/_plumed.py b/python/BioSimSpace/Process/_plumed.py index d113f1c22..9f3dfd52e 100644 --- a/python/BioSimSpace/Process/_plumed.py +++ b/python/BioSimSpace/Process/_plumed.py @@ -106,6 +106,10 @@ def __init__(self, work_dir): "PLUMED version >= 2.5 is required." ) + # Store the major and minor versions. + self._plumed_major = major + self._plumed_minor = minor + else: raise _Exceptions.IncompatibleError("Could not determine PLUMED version!") @@ -377,7 +381,9 @@ def _createMetadynamicsConfig(self, system, protocol, property_map={}): # The funnel collective variable requires an auxiliary file for # PLUMED versions < 2.7. - if self._plumed_version < 2.7: + if self._plumed_major < 2 or ( + self._plumed_major < 3 and self._plumed_minor < 7 + ): aux_file = "ProjectionOnAxis.cpp" self._config.append(f"LOAD FILE={aux_file}") aux_file = ( diff --git a/python/BioSimSpace/Process/_somd.py b/python/BioSimSpace/Process/_somd.py index 4d9d5e5e8..27582b2e5 100644 --- a/python/BioSimSpace/Process/_somd.py +++ b/python/BioSimSpace/Process/_somd.py @@ -585,13 +585,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -602,6 +607,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -613,7 +624,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py index 10ea8de6f..7595ff33f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py @@ -708,13 +708,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -725,6 +730,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -736,7 +747,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index 2a53e58a9..4695d7af1 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -704,13 +704,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -721,6 +726,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -740,7 +751,7 @@ def getTrajectory(self, block="AUTO"): else: self._traj_file = traj_file - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_namd.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_namd.py index ac6dfaa7a..db585adaf 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_namd.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_namd.py @@ -811,13 +811,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -827,6 +832,13 @@ def getTrajectory(self, block="AUTO"): trajectory : :class:`Trajectory ` The latest trajectory object. """ + + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -838,7 +850,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py index dcea9a2b5..d6a1d42fd 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_openmm.py @@ -1109,7 +1109,7 @@ def _generate_config(self): # Create a dummy PLUMED input file so that we can bind PLUMED # analysis functions to this process. - self._plumed = _Plumed(self._work_dir) + self._plumed = _Plumed(str(self._work_dir)) plumed_config, auxillary_files = self._plumed.createConfig( self._system, self._protocol, self._property_map ) @@ -1350,13 +1350,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -1367,6 +1372,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -1380,7 +1391,7 @@ def getTrajectory(self, block="AUTO"): if not _os.path.isfile(self._traj_file): return None else: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) def getFrame(self, index): """ diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_plumed.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_plumed.py index d113f1c22..9f3dfd52e 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_plumed.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_plumed.py @@ -106,6 +106,10 @@ def __init__(self, work_dir): "PLUMED version >= 2.5 is required." ) + # Store the major and minor versions. + self._plumed_major = major + self._plumed_minor = minor + else: raise _Exceptions.IncompatibleError("Could not determine PLUMED version!") @@ -377,7 +381,9 @@ def _createMetadynamicsConfig(self, system, protocol, property_map={}): # The funnel collective variable requires an auxiliary file for # PLUMED versions < 2.7. - if self._plumed_version < 2.7: + if self._plumed_major < 2 or ( + self._plumed_major < 3 and self._plumed_minor < 7 + ): aux_file = "ProjectionOnAxis.cpp" self._config.append(f"LOAD FILE={aux_file}") aux_file = ( diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_somd.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_somd.py index fbc1351c1..97882c198 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_somd.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_somd.py @@ -600,13 +600,18 @@ def getCurrentSystem(self): """ return self.getSystem(block=False) - def getTrajectory(self, block="AUTO"): + def getTrajectory(self, backend="AUTO", block="AUTO"): """ Return a trajectory object. Parameters ---------- + backend : str + The backend to use for trajectory parsing. To see supported backends, + run BioSimSpace.Trajectory.backends(). Using "AUTO" will try each in + sequence. + block : bool Whether to block until the process has finished running. @@ -617,6 +622,12 @@ def getTrajectory(self, block="AUTO"): The latest trajectory object. """ + if not isinstance(backend, str): + raise TypeError("'backend' must be of type 'str'") + + if not isinstance(block, (bool, str)): + raise TypeError("'block' must be of type 'bool' or 'str'") + # Wait for the process to finish. if block is True: self.wait() @@ -628,7 +639,7 @@ def getTrajectory(self, block="AUTO"): _warnings.warn("The process exited with an error!") try: - return _Trajectory.Trajectory(process=self) + return _Trajectory.Trajectory(process=self, backend=backend) except: return None From d9c37e65f8b9706b16b0a7704e206b7a8025b76b Mon Sep 17 00:00:00 2001 From: "William (Zhiyi) Wu" Date: Thu, 13 Jul 2023 16:58:22 +0100 Subject: [PATCH 03/17] Fix the extract in the gmx.process Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu --- .github/workflows/Sandpit_exs.yml | 6 ++---- python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/Sandpit_exs.yml b/.github/workflows/Sandpit_exs.yml index 99616ce0e..1548f0d79 100644 --- a/.github/workflows/Sandpit_exs.yml +++ b/.github/workflows/Sandpit_exs.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: os: ["ubuntu-latest", "macOS-latest",] - python-version: ["3.8", ] + python-version: ["3.8",] steps: - uses: actions/checkout@v2 @@ -35,12 +35,10 @@ jobs: - name: Install dependency run: | - mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.8 ambertools gromacs "sire=2023.2.2" alchemlyb pytest pyarrow "typing-extensions!=4.6.0" openff-interchange pint=0.21 "rdkit<2023" "jaxlib>0.3.7" tqdm + mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.8 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm python -m pip install git+https://github.com/Exscientia/MDRestraintsGenerator.git # For the testing of BSS.FreeEnergy.AlchemicalFreeEnergy.analysis python -m pip install https://github.com/alchemistry/alchemtest/archive/master.zip - # Before the new alchemlyb is released - python -m pip install git+https://github.com/alchemistry/alchemlyb.git - name: Install the dev version run: | diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index 4695d7af1..a772e9090 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -57,7 +57,7 @@ _alchemlyb = _try_import("alchemlyb") if _have_imported(_alchemlyb): - from alchemlyb.parsing.amber import extract as _extract + from alchemlyb.parsing.gmx import extract as _extract from .. import _gmx_exe from .. import _isVerbose From 64183f8c8bcb45cc3d2be7bb435c2943247bc45a Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon <36005076+msuruzhon@users.noreply.github.com> Date: Mon, 31 Jul 2023 16:54:39 +0100 Subject: [PATCH 04/17] Fixed squash and unsquash in the case of explicit dummies (#18) --- .github/workflows/Sandpit_exs.yml | 4 +- .../Sandpit/Exscientia/Align/_squash.py | 87 +++++++++++++++---- tests/Sandpit/Exscientia/Align/test_squash.py | 64 ++++++++++++-- 3 files changed, 130 insertions(+), 25 deletions(-) diff --git a/.github/workflows/Sandpit_exs.yml b/.github/workflows/Sandpit_exs.yml index 1548f0d79..4f7f9fad2 100644 --- a/.github/workflows/Sandpit_exs.yml +++ b/.github/workflows/Sandpit_exs.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: os: ["ubuntu-latest", "macOS-latest",] - python-version: ["3.8",] + python-version: ["3.10",] steps: - uses: actions/checkout@v2 @@ -35,7 +35,7 @@ jobs: - name: Install dependency run: | - mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.8 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm + mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm python -m pip install git+https://github.com/Exscientia/MDRestraintsGenerator.git # For the testing of BSS.FreeEnergy.AlchemicalFreeEnergy.analysis python -m pip install https://github.com/alchemistry/alchemtest/archive/master.zip diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py index 6bc62793c..5193cfc83 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py @@ -113,9 +113,30 @@ def _squash_molecule(molecule, explicit_dummies=False): return molecule if explicit_dummies: - # We make sure we use the same coordinates at both endstates. + # Get the common core atoms + atom_mapping0_common = _squashed_atom_mapping( + molecule, + is_lambda1=False, + explicit_dummies=explicit_dummies, + environment=False, + dummies=False, + ) + atom_mapping1_common = _squashed_atom_mapping( + molecule, + is_lambda1=True, + explicit_dummies=explicit_dummies, + environment=False, + dummies=False, + ) + if set(atom_mapping0_common) != set(atom_mapping1_common): + raise RuntimeError("The MCS atoms don't match between the two endstates") + common_atoms = set(atom_mapping0_common) + + # We make sure we use the same coordinates for the common core at both endstates. c = molecule.copy()._sire_object.cursor() - c["coordinates1"] = c["coordinates0"] + for i, atom in enumerate(c.atoms()): + if i in common_atoms: + atom["coordinates1"] = atom["coordinates0"] molecule = _Molecule(c.commit()) # Generate a "system" from the molecule at lambda = 0 and another copy at lambda = 1. @@ -269,20 +290,38 @@ def _unsquash_molecule(molecule, squashed_molecules, explicit_dummies=False): molecule : BioSimSpace._SireWrappers.Molecule The output updated merged molecule. """ - # Get the atom mapping and combine it with the lambda=0 molecule being prioritised + # Get the common core atoms + atom_mapping0_common = _squashed_atom_mapping( + molecule, + is_lambda1=False, + explicit_dummies=explicit_dummies, + environment=False, + dummies=False, + ) + atom_mapping1_common = _squashed_atom_mapping( + molecule, + is_lambda1=True, + explicit_dummies=explicit_dummies, + environment=False, + dummies=False, + ) + if set(atom_mapping0_common) != set(atom_mapping1_common): + raise RuntimeError("The MCS atoms don't match between the two endstates") + common_atoms = set(atom_mapping0_common) + + # Get the atom mapping from both endstates atom_mapping0 = _squashed_atom_mapping( molecule, is_lambda1=False, explicit_dummies=explicit_dummies ) atom_mapping1 = _squashed_atom_mapping( molecule, is_lambda1=True, explicit_dummies=explicit_dummies ) - atom_mapping = {**atom_mapping1, **atom_mapping0} update_velocity = squashed_molecules[0]._sire_object.hasProperty("velocity") - # Even though the two molecules should have the same coordinates, they might be PBC wrapped differently. + # Even though the common core of the two molecules should have the same coordinates, + # they might be PBC wrapped differently. # Here we take the first common core atom and translate the second molecule. if len(squashed_molecules) == 2: - common_atoms = set(atom_mapping0.keys()) & set(atom_mapping1.keys()) first_common_atom = list(sorted(common_atoms))[0] pertatom0 = squashed_molecules.getAtom(atom_mapping0[first_common_atom]) pertatom1 = squashed_molecules.getAtom(atom_mapping1[first_common_atom]) @@ -292,25 +331,39 @@ def _unsquash_molecule(molecule, squashed_molecules, explicit_dummies=False): # Update the coordinates and velocities. siremol = molecule.copy()._sire_object.edit() - for merged_atom_idx, squashed_atom_idx in atom_mapping.items(): + for merged_atom_idx in range(molecule.nAtoms()): + # Get the relevant atom indices merged_atom = siremol.atom(_SireMol.AtomIdx(merged_atom_idx)) - squashed_atom = squashed_molecules.getAtom(squashed_atom_idx) + if merged_atom_idx in atom_mapping0: + squashed_atom_idx0 = atom_mapping0[merged_atom_idx] + else: + squashed_atom_idx0 = atom_mapping1[merged_atom_idx] + if merged_atom_idx in atom_mapping1: + squashed_atom_idx1 = atom_mapping1[merged_atom_idx] + apply_translation_vec = True + else: + squashed_atom_idx1 = atom_mapping0[merged_atom_idx] + apply_translation_vec = False - # Update the coordinates. - coordinates = squashed_atom._sire_object.property("coordinates") + # Get the coordinates. + squashed_atom0 = squashed_molecules.getAtom(squashed_atom_idx0) + squashed_atom1 = squashed_molecules.getAtom(squashed_atom_idx1) + coordinates0 = squashed_atom0._sire_object.property("coordinates") + coordinates1 = squashed_atom1._sire_object.property("coordinates") # Apply the translation if the atom is coming from the second molecule. - if len(squashed_molecules) == 2 and squashed_atom_idx in atom_mapping1.values(): - coordinates -= translation_vec + if len(squashed_molecules) == 2 and apply_translation_vec: + coordinates1 -= translation_vec - siremol = merged_atom.setProperty("coordinates0", coordinates).molecule() - siremol = merged_atom.setProperty("coordinates1", coordinates).molecule() + siremol = merged_atom.setProperty("coordinates0", coordinates0).molecule() + siremol = merged_atom.setProperty("coordinates1", coordinates1).molecule() # Update the velocities. if update_velocity: - velocities = squashed_atom._sire_object.property("velocity") - siremol = merged_atom.setProperty("velocity0", velocities).molecule() - siremol = merged_atom.setProperty("velocity1", velocities).molecule() + velocities0 = squashed_atom0._sire_object.property("velocity") + velocities1 = squashed_atom1._sire_object.property("velocity") + siremol = merged_atom.setProperty("velocity0", velocities0).molecule() + siremol = merged_atom.setProperty("velocity1", velocities1).molecule() return _Molecule(siremol.commit()) diff --git a/tests/Sandpit/Exscientia/Align/test_squash.py b/tests/Sandpit/Exscientia/Align/test_squash.py index 132cfdacf..bfcc29656 100644 --- a/tests/Sandpit/Exscientia/Align/test_squash.py +++ b/tests/Sandpit/Exscientia/Align/test_squash.py @@ -1,9 +1,22 @@ +import os import pickle + +import numpy as np import pytest +import sire +from sire.maths import Vector import BioSimSpace.Sandpit.Exscientia as BSS -from tests.Sandpit.Exscientia.conftest import has_amber +# 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 @pytest.fixture(scope="session") @@ -30,6 +43,20 @@ def perturbed_system(): return system +@pytest.fixture(scope="session") +def dual_topology_system(): + mol_smiles = ["c1ccccc1", "c1ccccc1C"] + mols = [BSS.Parameters.gaff(smi).getMolecule() for smi in mol_smiles] + pertmol = BSS.Align.merge(mols[0], mols[1], mapping={0: 0}) + c = pertmol._sire_object.cursor() + # Translate all atoms so that we have different coordinates between both endstates + for atom in c.atoms(): + atom["coordinates1"] = atom["coordinates0"] + Vector(1, 1, 1) + pertmol = BSS._SireWrappers.Molecule(c.commit()) + system = pertmol.toSystem() + return system + + @pytest.fixture def perturbed_tripeptide(): return pickle.load( @@ -118,21 +145,46 @@ def test_squashed_atom_mapping_explicit(perturbed_tripeptide, is_lambda1): @pytest.mark.skipif(has_amber is False, reason="Requires AMBER to be installed.") @pytest.mark.parametrize("explicit", [False, True]) -def test_unsquash(perturbed_system, explicit): +def test_unsquash(dual_topology_system, explicit): squashed_system, mapping = BSS.Align._squash._squash( - perturbed_system, explicit_dummies=explicit + dual_topology_system, explicit_dummies=explicit ) new_perturbed_system = BSS.Align._squash._unsquash( - perturbed_system, squashed_system, mapping, explicit_dummies=explicit + dual_topology_system, squashed_system, mapping, explicit_dummies=explicit ) assert [ mol0.nAtoms() == mol1.nAtoms() - for mol0, mol1 in zip(perturbed_system, new_perturbed_system) + for mol0, mol1 in zip(dual_topology_system, new_perturbed_system) ] assert [ mol0.isPerturbable() == mol1.isPerturbable() - for mol0, mol1 in zip(perturbed_system, new_perturbed_system) + for mol0, mol1 in zip(dual_topology_system, new_perturbed_system) ] + if explicit: + # Check that we have loaded the correct coordinates + coords0_before = sire.io.get_coords_array( + dual_topology_system[0]._sire_object, map={"coordinates": "coordinates0"} + ) + coords1_before = sire.io.get_coords_array( + dual_topology_system[0]._sire_object, map={"coordinates": "coordinates1"} + ) + coords0_after = sire.io.get_coords_array( + new_perturbed_system[0]._sire_object, map={"coordinates": "coordinates0"} + ) + coords1_after = sire.io.get_coords_array( + new_perturbed_system[0]._sire_object, map={"coordinates": "coordinates1"} + ) + + # The coordinates at the first endstate should be completely preserved + # Because in this case they are either common core, or separate dummies at lambda = 0 + assert np.allclose(coords0_before, coords0_after) + + # The coordinates at the first endstate should be partially preserved + # The common core must have the same coordinates as lambda = 0 + # Here this is just a single atom in the beginning + # The extra atoms which are dummies at lambda = 0 have separate coordinates here + assert np.allclose(coords0_before[:1, :], coords1_after[:1, :]) + assert np.allclose(coords1_before[1:, :], coords1_after[1:, :]) @pytest.mark.parametrize("explicit", [False, True]) From 1f1802a99b4d773c449f7ac59e4654e5418c002b Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Wed, 16 Aug 2023 09:13:17 +0100 Subject: [PATCH 05/17] Make the saveMetric method more robust (#19) Co-authored-by: William (Zhiyi) Wu --- .../Sandpit/Exscientia/Process/_process.py | 12 ++++++++- .../Sandpit/Exscientia/Process/test_amber.py | 26 ++++++++++++++----- .../Exscientia/Process/test_gromacs.py | 14 ++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py index fd9c5db95..7809ab9ee 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py @@ -29,6 +29,7 @@ import collections as _collections import glob as _glob import os as _os +import traceback import pandas as pd @@ -910,7 +911,16 @@ def wait(self, max_time=None): # The process isn't running. if not self.isRunning(): - self.saveMetric() + try: + self.saveMetric() + except Exception: + exception_info = traceback.format_exc() + with open(f"{self.workDir()}/{self._name}.err", 'a+') as f: + f.write("Exception Information during saveMetric():\n") + f.write("======================\n") + f.write(exception_info) + f.write("\n\n") + return if max_time is not None: diff --git a/tests/Sandpit/Exscientia/Process/test_amber.py b/tests/Sandpit/Exscientia/Process/test_amber.py index 86b0bb4ac..489f5bd84 100644 --- a/tests/Sandpit/Exscientia/Process/test_amber.py +++ b/tests/Sandpit/Exscientia/Process/test_amber.py @@ -320,14 +320,11 @@ def test_parse_fep_output(system, protocol): assert len(records_sc1) != 0 -@pytest.mark.skipif( - has_amber is False or has_pyarrow is False, - reason="Requires AMBER and pyarrow to be installed.", -) + class TestsaveMetric: @staticmethod @pytest.fixture() - def setup(system): + def alchemical_system(system): # Copy the system. system_copy = system.copy() @@ -335,10 +332,15 @@ def setup(system): mol = system_copy[0] mol = BSS.Align.decouple(mol) system_copy.updateMolecule(0, mol) + return system_copy + + @staticmethod + @pytest.fixture() + def setup(alchemical_system): # Create a process using any system and the protocol. process = BSS.Process.Amber( - system_copy, + alchemical_system, BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) shutil.copyfile( @@ -348,6 +350,18 @@ def setup(system): process.saveMetric() return process + def test_error_alchemlyb_extract(self, alchemical_system): + # Create a process using any system and the protocol. + process = BSS.Process.Amber( + alchemical_system, + BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), + ) + process.wait() + with open(process.workDir() + '/amber.err', 'r') as f: + text = f.read() + assert 'Exception Information' in text + + def test_metric_parquet_exist(self, setup): assert Path(f"{setup.workDir()}/metric.parquet").exists() diff --git a/tests/Sandpit/Exscientia/Process/test_gromacs.py b/tests/Sandpit/Exscientia/Process/test_gromacs.py index ea08bf52f..1a42967c5 100644 --- a/tests/Sandpit/Exscientia/Process/test_gromacs.py +++ b/tests/Sandpit/Exscientia/Process/test_gromacs.py @@ -341,6 +341,20 @@ def test_u_nk_parquet(self, setup): df = pd.read_parquet(f"{setup.workDir()}/u_nk.parquet") assert df.shape == (1001, 20) + def test_error_alchemlyb_extract(self, perturbable_system, monkeypatch): + def extract(*args): + raise ValueError('alchemlyb.parsing.gmx.extract failed.') + monkeypatch.setattr('alchemlyb.parsing.gmx.extract', extract) + # Create a process using any system and the protocol. + process = BSS.Process.Gromacs( + perturbable_system, + BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), + ) + process.wait() + with open(process.workDir() + '/gromacs.err', 'r') as f: + text = f.read() + assert 'Exception Information' in text + @pytest.mark.skipif( has_amber is False From f32838635e41ba8983b0a72ee00c7b1fee710cef Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon <36005076+msuruzhon@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:06:42 +0100 Subject: [PATCH 06/17] Merge some recent bugfixes (#24) --- .github/workflows/Sandpit_exs.yml | 2 +- python/BioSimSpace/Convert/_convert.py | 19 +++++++++++++-- python/BioSimSpace/IO/_io.py | 23 +++++++++++++++++++ .../Sandpit/Exscientia/Align/_squash.py | 3 +++ .../Sandpit/Exscientia/Convert/_convert.py | 19 +++++++++++++-- .../BioSimSpace/Sandpit/Exscientia/IO/_io.py | 23 +++++++++++++++++++ .../Sandpit/Exscientia/Process/_amber.py | 13 +++++++---- .../Exscientia/_SireWrappers/_system.py | 17 -------------- python/BioSimSpace/_SireWrappers/_system.py | 17 -------------- .../Exscientia/Process/test_gromacs.py | 9 ++++---- .../Process/test_position_restraint.py | 16 ++++++------- 11 files changed, 106 insertions(+), 55 deletions(-) diff --git a/.github/workflows/Sandpit_exs.yml b/.github/workflows/Sandpit_exs.yml index 4f7f9fad2..f55d08ace 100644 --- a/.github/workflows/Sandpit_exs.yml +++ b/.github/workflows/Sandpit_exs.yml @@ -35,7 +35,7 @@ jobs: - name: Install dependency run: | - mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm + mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm pyarrow=12.0.1 python -m pip install git+https://github.com/Exscientia/MDRestraintsGenerator.git # For the testing of BSS.FreeEnergy.AlchemicalFreeEnergy.analysis python -m pip install https://github.com/alchemistry/alchemtest/archive/master.zip diff --git a/python/BioSimSpace/Convert/_convert.py b/python/BioSimSpace/Convert/_convert.py index 68a2910b3..3025e165f 100644 --- a/python/BioSimSpace/Convert/_convert.py +++ b/python/BioSimSpace/Convert/_convert.py @@ -49,6 +49,7 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem +import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -205,14 +206,28 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule. + # Otherwise, convert residues/atoms to a molecule, then a system. try: - obj = obj.toMolecule() + obj = obj.toMolecule().toSystem() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) + # Get the user-defined space property name. + prop = property_map.get("space", "space") + + # Try to get the space property. Use an infinite cartesian space + # if none is present. + try: + space = obj._sire_object.property(prop) + except: + space = _SireVol.Cartesian() + + # Set a shared space property. + obj._sire_object.addSharedProperty(prop) + obj._sire_object.setSharedProperty(prop, space) + # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/IO/_io.py b/python/BioSimSpace/IO/_io.py index c36696051..36b4e548f 100644 --- a/python/BioSimSpace/IO/_io.py +++ b/python/BioSimSpace/IO/_io.py @@ -523,6 +523,29 @@ def readMolecules( else: raise IOError(msg) from None + # Add a file format shared property. + prop = property_map.get("fileformat", "fileformat") + system.addSharedProperty(prop) + system.setSharedProperty(prop, system.property(prop)) + + # 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 = system.property(prop) + system.removeSharedProperty(prop) + system.setProperty(prop, space) + + # Time. + prop = property_map.get("time", "time") + time = system.property(prop) + system.removeSharedProperty(prop) + system.setProperties(prop, time) + except: + pass + return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py index 5193cfc83..a7377f7b1 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py @@ -353,6 +353,9 @@ def _unsquash_molecule(molecule, squashed_molecules, explicit_dummies=False): # Apply the translation if the atom is coming from the second molecule. if len(squashed_molecules) == 2 and apply_translation_vec: + # This is a dummy atom so we need to translate coordinates0 as well + if squashed_atom_idx0 == squashed_atom_idx1: + coordinates0 -= translation_vec coordinates1 -= translation_vec siremol = merged_atom.setProperty("coordinates0", coordinates0).molecule() diff --git a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py index 68a2910b3..3025e165f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py @@ -49,6 +49,7 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem +import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -205,14 +206,28 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule. + # Otherwise, convert residues/atoms to a molecule, then a system. try: - obj = obj.toMolecule() + obj = obj.toMolecule().toSystem() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) + # Get the user-defined space property name. + prop = property_map.get("space", "space") + + # Try to get the space property. Use an infinite cartesian space + # if none is present. + try: + space = obj._sire_object.property(prop) + except: + space = _SireVol.Cartesian() + + # Set a shared space property. + obj._sire_object.addSharedProperty(prop) + obj._sire_object.setSharedProperty(prop, space) + # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py index c36696051..36b4e548f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py +++ b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py @@ -523,6 +523,29 @@ def readMolecules( else: raise IOError(msg) from None + # Add a file format shared property. + prop = property_map.get("fileformat", "fileformat") + system.addSharedProperty(prop) + system.setSharedProperty(prop, system.property(prop)) + + # 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 = system.property(prop) + system.removeSharedProperty(prop) + system.setProperty(prop, space) + + # Time. + prop = property_map.get("time", "time") + time = system.property(prop) + system.removeSharedProperty(prop) + system.setProperties(prop, time) + except: + pass + return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py index 7595ff33f..742ad8459 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py @@ -27,6 +27,7 @@ __all__ = ["Amber"] import os +import shutil from pathlib import Path as _Path from .._Utils import _try_import @@ -303,9 +304,11 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if coord_file is not None: try: file = _os.path.splitext(coord_file)[0] - _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) + _IO.saveMolecules(file, system, "rst", property_map=self._property_map) + # To keep the file extension the same. + shutil.move(f"{file}.rst", f"{file}.rst7") except Exception as e: - msg = "Failed to write system to 'RST7' format." + msg = "Failed to write system to 'rst7' format." if _isVerbose(): raise IOError(msg) from e else: @@ -315,9 +318,11 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if ref_file is not None: try: file = _os.path.splitext(ref_file)[0] - _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) + _IO.saveMolecules(file, system, "rst", property_map=self._property_map) + # To keep the file extension the same. + shutil.move(f"{file}.rst", f"{file}.rst7") except Exception as e: - msg = "Failed to write system to 'RST7' format." + msg = "Failed to write system to 'rst7' format." if _isVerbose(): raise IOError(msg) from e else: diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py index 701cf81d2..8793472ef 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py @@ -136,23 +136,6 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 - # Copy any fileformat property to each molecule. - if "fileformat" in self._sire_object.propertyKeys(): - fileformat = self._sire_object.property("fileformat") - for num in self._mol_nums: - edit_mol = self._sire_object[num].edit() - edit_mol = edit_mol.setProperty("fileformat", fileformat) - self._sire_object.update(edit_mol.commit()) - else: - # If a molecule has a fileformat property, use the first - # that we find. - for mol in self: - if mol._sire_object.hasProperty("fileformat"): - self._sire_object.setProperty( - "fileformat", mol._sire_object.property("fileformat") - ) - break - # Reset the index mappings. self._reset_mappings() diff --git a/python/BioSimSpace/_SireWrappers/_system.py b/python/BioSimSpace/_SireWrappers/_system.py index 140a66f5b..14ef3ba65 100644 --- a/python/BioSimSpace/_SireWrappers/_system.py +++ b/python/BioSimSpace/_SireWrappers/_system.py @@ -136,23 +136,6 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 - # Copy any fileformat property to each molecule. - if "fileformat" in self._sire_object.propertyKeys(): - fileformat = self._sire_object.property("fileformat") - for num in self._mol_nums: - edit_mol = self._sire_object[num].edit() - edit_mol = edit_mol.setProperty("fileformat", fileformat) - self._sire_object.update(edit_mol.commit()) - else: - # If a molecule has a fileformat property, use the first - # that we find. - for mol in self: - if mol._sire_object.hasProperty("fileformat"): - self._sire_object.setProperty( - "fileformat", mol._sire_object.property("fileformat") - ) - break - # Reset the index mappings. self._reset_mappings() diff --git a/tests/Sandpit/Exscientia/Process/test_gromacs.py b/tests/Sandpit/Exscientia/Process/test_gromacs.py index 1a42967c5..8d7f6c4fd 100644 --- a/tests/Sandpit/Exscientia/Process/test_gromacs.py +++ b/tests/Sandpit/Exscientia/Process/test_gromacs.py @@ -343,17 +343,18 @@ def test_u_nk_parquet(self, setup): def test_error_alchemlyb_extract(self, perturbable_system, monkeypatch): def extract(*args): - raise ValueError('alchemlyb.parsing.gmx.extract failed.') - monkeypatch.setattr('alchemlyb.parsing.gmx.extract', extract) + raise ValueError("alchemlyb.parsing.gmx.extract failed.") + + monkeypatch.setattr("alchemlyb.parsing.gmx.extract", extract) # Create a process using any system and the protocol. process = BSS.Process.Gromacs( perturbable_system, BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) process.wait() - with open(process.workDir() + '/gromacs.err', 'r') as f: + with open(process.workDir() + "/gromacs.err", "r") as f: text = f.read() - assert 'Exception Information' in text + assert "Exception Information" in text @pytest.mark.skipif( diff --git a/tests/Sandpit/Exscientia/Process/test_position_restraint.py b/tests/Sandpit/Exscientia/Process/test_position_restraint.py index 45697b9ed..aa720c470 100644 --- a/tests/Sandpit/Exscientia/Process/test_position_restraint.py +++ b/tests/Sandpit/Exscientia/Process/test_position_restraint.py @@ -5,6 +5,7 @@ import pandas as pd import pytest +from sire.legacy.IO import AmberRst import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Units.Energy import kj_per_mol @@ -121,8 +122,8 @@ def test_gromacs(protocol, system, ref_system, tmp_path): @pytest.mark.skipif( - has_gromacs is False or has_openff is False, - reason="Requires AMBER and openff to be installed", + has_openff is False, + reason="Requires openff to be installed", ) def test_amber(protocol, system, ref_system, tmp_path): proc = BSS.Process.Amber( @@ -137,12 +138,11 @@ def test_amber(protocol, system, ref_system, tmp_path): # We have generated a separate restraint reference assert os.path.exists(proc._ref_file) - contents_ref, contents_rst = ( - open(proc._ref_file).readlines(), - open(proc._rst_file).readlines(), - ) - diff = list(unified_diff(contents_ref, contents_rst)) - assert len(diff) + + ref = AmberRst(proc._ref_file).getFrame(0) + rst = AmberRst(proc._rst_file).getFrame(0) + + assert ref != rst # We are pointing the reference to the correct file assert f"{proc._work_dir}/{proc.getArgs()['-ref']}" == proc._ref_file From 24bbaa05d3241635d6946d589a64b68f366cb327 Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon Date: Mon, 4 Sep 2023 15:03:10 +0100 Subject: [PATCH 07/17] Revert "Merge some recent bugfixes (#24)" This reverts commit f32838635e41ba8983b0a72ee00c7b1fee710cef. --- .github/workflows/Sandpit_exs.yml | 2 +- python/BioSimSpace/Convert/_convert.py | 19 ++------------- python/BioSimSpace/IO/_io.py | 23 ------------------- .../Sandpit/Exscientia/Align/_squash.py | 3 --- .../Sandpit/Exscientia/Convert/_convert.py | 19 ++------------- .../BioSimSpace/Sandpit/Exscientia/IO/_io.py | 23 ------------------- .../Sandpit/Exscientia/Process/_amber.py | 13 ++++------- .../Exscientia/_SireWrappers/_system.py | 17 ++++++++++++++ python/BioSimSpace/_SireWrappers/_system.py | 17 ++++++++++++++ .../Exscientia/Process/test_gromacs.py | 9 ++++---- .../Process/test_position_restraint.py | 16 ++++++------- 11 files changed, 55 insertions(+), 106 deletions(-) diff --git a/.github/workflows/Sandpit_exs.yml b/.github/workflows/Sandpit_exs.yml index f55d08ace..4f7f9fad2 100644 --- a/.github/workflows/Sandpit_exs.yml +++ b/.github/workflows/Sandpit_exs.yml @@ -35,7 +35,7 @@ jobs: - name: Install dependency run: | - mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm pyarrow=12.0.1 + mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm python -m pip install git+https://github.com/Exscientia/MDRestraintsGenerator.git # For the testing of BSS.FreeEnergy.AlchemicalFreeEnergy.analysis python -m pip install https://github.com/alchemistry/alchemtest/archive/master.zip diff --git a/python/BioSimSpace/Convert/_convert.py b/python/BioSimSpace/Convert/_convert.py index 3025e165f..68a2910b3 100644 --- a/python/BioSimSpace/Convert/_convert.py +++ b/python/BioSimSpace/Convert/_convert.py @@ -49,7 +49,6 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem -import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -206,28 +205,14 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule, then a system. + # Otherwise, convert residues/atoms to a molecule. try: - obj = obj.toMolecule().toSystem() + obj = obj.toMolecule() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) - # Get the user-defined space property name. - prop = property_map.get("space", "space") - - # Try to get the space property. Use an infinite cartesian space - # if none is present. - try: - space = obj._sire_object.property(prop) - except: - space = _SireVol.Cartesian() - - # Set a shared space property. - obj._sire_object.addSharedProperty(prop) - obj._sire_object.setSharedProperty(prop, space) - # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/IO/_io.py b/python/BioSimSpace/IO/_io.py index 36b4e548f..c36696051 100644 --- a/python/BioSimSpace/IO/_io.py +++ b/python/BioSimSpace/IO/_io.py @@ -523,29 +523,6 @@ def readMolecules( else: raise IOError(msg) from None - # Add a file format shared property. - prop = property_map.get("fileformat", "fileformat") - system.addSharedProperty(prop) - system.setSharedProperty(prop, system.property(prop)) - - # 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 = system.property(prop) - system.removeSharedProperty(prop) - system.setProperty(prop, space) - - # Time. - prop = property_map.get("time", "time") - time = system.property(prop) - system.removeSharedProperty(prop) - system.setProperties(prop, time) - except: - pass - return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py index a7377f7b1..5193cfc83 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py @@ -353,9 +353,6 @@ def _unsquash_molecule(molecule, squashed_molecules, explicit_dummies=False): # Apply the translation if the atom is coming from the second molecule. if len(squashed_molecules) == 2 and apply_translation_vec: - # This is a dummy atom so we need to translate coordinates0 as well - if squashed_atom_idx0 == squashed_atom_idx1: - coordinates0 -= translation_vec coordinates1 -= translation_vec siremol = merged_atom.setProperty("coordinates0", coordinates0).molecule() diff --git a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py index 3025e165f..68a2910b3 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py @@ -49,7 +49,6 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem -import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -206,28 +205,14 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule, then a system. + # Otherwise, convert residues/atoms to a molecule. try: - obj = obj.toMolecule().toSystem() + obj = obj.toMolecule() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) - # Get the user-defined space property name. - prop = property_map.get("space", "space") - - # Try to get the space property. Use an infinite cartesian space - # if none is present. - try: - space = obj._sire_object.property(prop) - except: - space = _SireVol.Cartesian() - - # Set a shared space property. - obj._sire_object.addSharedProperty(prop) - obj._sire_object.setSharedProperty(prop, space) - # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py index 36b4e548f..c36696051 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py +++ b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py @@ -523,29 +523,6 @@ def readMolecules( else: raise IOError(msg) from None - # Add a file format shared property. - prop = property_map.get("fileformat", "fileformat") - system.addSharedProperty(prop) - system.setSharedProperty(prop, system.property(prop)) - - # 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 = system.property(prop) - system.removeSharedProperty(prop) - system.setProperty(prop, space) - - # Time. - prop = property_map.get("time", "time") - time = system.property(prop) - system.removeSharedProperty(prop) - system.setProperties(prop, time) - except: - pass - return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py index 742ad8459..7595ff33f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py @@ -27,7 +27,6 @@ __all__ = ["Amber"] import os -import shutil from pathlib import Path as _Path from .._Utils import _try_import @@ -304,11 +303,9 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if coord_file is not None: try: file = _os.path.splitext(coord_file)[0] - _IO.saveMolecules(file, system, "rst", property_map=self._property_map) - # To keep the file extension the same. - shutil.move(f"{file}.rst", f"{file}.rst7") + _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) except Exception as e: - msg = "Failed to write system to 'rst7' format." + msg = "Failed to write system to 'RST7' format." if _isVerbose(): raise IOError(msg) from e else: @@ -318,11 +315,9 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if ref_file is not None: try: file = _os.path.splitext(ref_file)[0] - _IO.saveMolecules(file, system, "rst", property_map=self._property_map) - # To keep the file extension the same. - shutil.move(f"{file}.rst", f"{file}.rst7") + _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) except Exception as e: - msg = "Failed to write system to 'rst7' format." + msg = "Failed to write system to 'RST7' format." if _isVerbose(): raise IOError(msg) from e else: diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py index 8793472ef..701cf81d2 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py @@ -136,6 +136,23 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 + # Copy any fileformat property to each molecule. + if "fileformat" in self._sire_object.propertyKeys(): + fileformat = self._sire_object.property("fileformat") + for num in self._mol_nums: + edit_mol = self._sire_object[num].edit() + edit_mol = edit_mol.setProperty("fileformat", fileformat) + self._sire_object.update(edit_mol.commit()) + else: + # If a molecule has a fileformat property, use the first + # that we find. + for mol in self: + if mol._sire_object.hasProperty("fileformat"): + self._sire_object.setProperty( + "fileformat", mol._sire_object.property("fileformat") + ) + break + # Reset the index mappings. self._reset_mappings() diff --git a/python/BioSimSpace/_SireWrappers/_system.py b/python/BioSimSpace/_SireWrappers/_system.py index 14ef3ba65..140a66f5b 100644 --- a/python/BioSimSpace/_SireWrappers/_system.py +++ b/python/BioSimSpace/_SireWrappers/_system.py @@ -136,6 +136,23 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 + # Copy any fileformat property to each molecule. + if "fileformat" in self._sire_object.propertyKeys(): + fileformat = self._sire_object.property("fileformat") + for num in self._mol_nums: + edit_mol = self._sire_object[num].edit() + edit_mol = edit_mol.setProperty("fileformat", fileformat) + self._sire_object.update(edit_mol.commit()) + else: + # If a molecule has a fileformat property, use the first + # that we find. + for mol in self: + if mol._sire_object.hasProperty("fileformat"): + self._sire_object.setProperty( + "fileformat", mol._sire_object.property("fileformat") + ) + break + # Reset the index mappings. self._reset_mappings() diff --git a/tests/Sandpit/Exscientia/Process/test_gromacs.py b/tests/Sandpit/Exscientia/Process/test_gromacs.py index 8d7f6c4fd..1a42967c5 100644 --- a/tests/Sandpit/Exscientia/Process/test_gromacs.py +++ b/tests/Sandpit/Exscientia/Process/test_gromacs.py @@ -343,18 +343,17 @@ def test_u_nk_parquet(self, setup): def test_error_alchemlyb_extract(self, perturbable_system, monkeypatch): def extract(*args): - raise ValueError("alchemlyb.parsing.gmx.extract failed.") - - monkeypatch.setattr("alchemlyb.parsing.gmx.extract", extract) + raise ValueError('alchemlyb.parsing.gmx.extract failed.') + monkeypatch.setattr('alchemlyb.parsing.gmx.extract', extract) # Create a process using any system and the protocol. process = BSS.Process.Gromacs( perturbable_system, BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) process.wait() - with open(process.workDir() + "/gromacs.err", "r") as f: + with open(process.workDir() + '/gromacs.err', 'r') as f: text = f.read() - assert "Exception Information" in text + assert 'Exception Information' in text @pytest.mark.skipif( diff --git a/tests/Sandpit/Exscientia/Process/test_position_restraint.py b/tests/Sandpit/Exscientia/Process/test_position_restraint.py index aa720c470..45697b9ed 100644 --- a/tests/Sandpit/Exscientia/Process/test_position_restraint.py +++ b/tests/Sandpit/Exscientia/Process/test_position_restraint.py @@ -5,7 +5,6 @@ import pandas as pd import pytest -from sire.legacy.IO import AmberRst import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Units.Energy import kj_per_mol @@ -122,8 +121,8 @@ def test_gromacs(protocol, system, ref_system, tmp_path): @pytest.mark.skipif( - has_openff is False, - reason="Requires openff to be installed", + has_gromacs is False or has_openff is False, + reason="Requires AMBER and openff to be installed", ) def test_amber(protocol, system, ref_system, tmp_path): proc = BSS.Process.Amber( @@ -138,11 +137,12 @@ def test_amber(protocol, system, ref_system, tmp_path): # We have generated a separate restraint reference assert os.path.exists(proc._ref_file) - - ref = AmberRst(proc._ref_file).getFrame(0) - rst = AmberRst(proc._rst_file).getFrame(0) - - assert ref != rst + contents_ref, contents_rst = ( + open(proc._ref_file).readlines(), + open(proc._rst_file).readlines(), + ) + diff = list(unified_diff(contents_ref, contents_rst)) + assert len(diff) # We are pointing the reference to the correct file assert f"{proc._work_dir}/{proc.getArgs()['-ref']}" == proc._ref_file From f9bed330ff5cfa2850b0a5865052b3ccd9ecfb53 Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon <36005076+msuruzhon@users.noreply.github.com> Date: Mon, 18 Sep 2023 11:10:01 +0100 Subject: [PATCH 08/17] Added some fixes to the solvation (#25) Co-authored-by: Lester Hedges --- python/BioSimSpace/Convert/_convert.py | 19 +++++++++++++-- python/BioSimSpace/IO/_io.py | 23 +++++++++++++++++++ .../Sandpit/Exscientia/Convert/_convert.py | 19 +++++++++++++-- .../BioSimSpace/Sandpit/Exscientia/IO/_io.py | 23 +++++++++++++++++++ .../Exscientia/_SireWrappers/_system.py | 17 -------------- python/BioSimSpace/_SireWrappers/_system.py | 17 -------------- 6 files changed, 80 insertions(+), 38 deletions(-) diff --git a/python/BioSimSpace/Convert/_convert.py b/python/BioSimSpace/Convert/_convert.py index 68a2910b3..3025e165f 100644 --- a/python/BioSimSpace/Convert/_convert.py +++ b/python/BioSimSpace/Convert/_convert.py @@ -49,6 +49,7 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem +import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -205,14 +206,28 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule. + # Otherwise, convert residues/atoms to a molecule, then a system. try: - obj = obj.toMolecule() + obj = obj.toMolecule().toSystem() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) + # Get the user-defined space property name. + prop = property_map.get("space", "space") + + # Try to get the space property. Use an infinite cartesian space + # if none is present. + try: + space = obj._sire_object.property(prop) + except: + space = _SireVol.Cartesian() + + # Set a shared space property. + obj._sire_object.addSharedProperty(prop) + obj._sire_object.setSharedProperty(prop, space) + # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/IO/_io.py b/python/BioSimSpace/IO/_io.py index c36696051..36b4e548f 100644 --- a/python/BioSimSpace/IO/_io.py +++ b/python/BioSimSpace/IO/_io.py @@ -523,6 +523,29 @@ def readMolecules( else: raise IOError(msg) from None + # Add a file format shared property. + prop = property_map.get("fileformat", "fileformat") + system.addSharedProperty(prop) + system.setSharedProperty(prop, system.property(prop)) + + # 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 = system.property(prop) + system.removeSharedProperty(prop) + system.setProperty(prop, space) + + # Time. + prop = property_map.get("time", "time") + time = system.property(prop) + system.removeSharedProperty(prop) + system.setProperties(prop, time) + except: + pass + return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py index 68a2910b3..3025e165f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Convert/_convert.py @@ -49,6 +49,7 @@ import sire.legacy.Mol as _SireMol import sire.legacy.System as _SireSystem +import sire.legacy.Vol as _SireVol import sire.system as _NewSireSystem @@ -205,14 +206,28 @@ def to(obj, format="biosimspace", property_map={}): try: obj = obj.toSystem() except: - # Otherwise, convert residues/atoms to a molecule. + # Otherwise, convert residues/atoms to a molecule, then a system. try: - obj = obj.toMolecule() + obj = obj.toMolecule().toSystem() except: raise _ConversionError( "Unable to convert object to OpenMM format!" ) + # Get the user-defined space property name. + prop = property_map.get("space", "space") + + # Try to get the space property. Use an infinite cartesian space + # if none is present. + try: + space = obj._sire_object.property(prop) + except: + space = _SireVol.Cartesian() + + # Set a shared space property. + obj._sire_object.addSharedProperty(prop) + obj._sire_object.setSharedProperty(prop, space) + # Now try to convert the object to OpenMM format. try: return _sire_convert.to( diff --git a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py index c36696051..36b4e548f 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py +++ b/python/BioSimSpace/Sandpit/Exscientia/IO/_io.py @@ -523,6 +523,29 @@ def readMolecules( else: raise IOError(msg) from None + # Add a file format shared property. + prop = property_map.get("fileformat", "fileformat") + system.addSharedProperty(prop) + system.setSharedProperty(prop, system.property(prop)) + + # 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 = system.property(prop) + system.removeSharedProperty(prop) + system.setProperty(prop, space) + + # Time. + prop = property_map.get("time", "time") + time = system.property(prop) + system.removeSharedProperty(prop) + system.setProperties(prop, time) + except: + pass + return _System(system) diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py index 701cf81d2..8793472ef 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py @@ -136,23 +136,6 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 - # Copy any fileformat property to each molecule. - if "fileformat" in self._sire_object.propertyKeys(): - fileformat = self._sire_object.property("fileformat") - for num in self._mol_nums: - edit_mol = self._sire_object[num].edit() - edit_mol = edit_mol.setProperty("fileformat", fileformat) - self._sire_object.update(edit_mol.commit()) - else: - # If a molecule has a fileformat property, use the first - # that we find. - for mol in self: - if mol._sire_object.hasProperty("fileformat"): - self._sire_object.setProperty( - "fileformat", mol._sire_object.property("fileformat") - ) - break - # Reset the index mappings. self._reset_mappings() diff --git a/python/BioSimSpace/_SireWrappers/_system.py b/python/BioSimSpace/_SireWrappers/_system.py index 140a66f5b..14ef3ba65 100644 --- a/python/BioSimSpace/_SireWrappers/_system.py +++ b/python/BioSimSpace/_SireWrappers/_system.py @@ -136,23 +136,6 @@ def __init__(self, system): # Initialise the iterator counter. self._iter_count = 0 - # Copy any fileformat property to each molecule. - if "fileformat" in self._sire_object.propertyKeys(): - fileformat = self._sire_object.property("fileformat") - for num in self._mol_nums: - edit_mol = self._sire_object[num].edit() - edit_mol = edit_mol.setProperty("fileformat", fileformat) - self._sire_object.update(edit_mol.commit()) - else: - # If a molecule has a fileformat property, use the first - # that we find. - for mol in self: - if mol._sire_object.hasProperty("fileformat"): - self._sire_object.setProperty( - "fileformat", mol._sire_object.property("fileformat") - ) - break - # Reset the index mappings. self._reset_mappings() From f1f52769fe43e6654caec03f537a1fe8aee69bdd Mon Sep 17 00:00:00 2001 From: "William (Zhiyi) Wu" Date: Mon, 18 Sep 2023 18:22:35 +0800 Subject: [PATCH 09/17] Fixed PBC fixing when unsquashing for dummy atoms (#20) --- python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py | 3 +++ tests/Sandpit/Exscientia/_SireWrappers/test_system.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py index 5193cfc83..a7377f7b1 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_squash.py @@ -353,6 +353,9 @@ def _unsquash_molecule(molecule, squashed_molecules, explicit_dummies=False): # Apply the translation if the atom is coming from the second molecule. if len(squashed_molecules) == 2 and apply_translation_vec: + # This is a dummy atom so we need to translate coordinates0 as well + if squashed_atom_idx0 == squashed_atom_idx1: + coordinates0 -= translation_vec coordinates1 -= translation_vec siremol = merged_atom.setProperty("coordinates0", coordinates0).molecule() diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py index 86d4e0f85..c5e6978c6 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py @@ -1,5 +1,6 @@ import math import pytest +import platform import BioSimSpace.Sandpit.Exscientia as BSS @@ -262,6 +263,10 @@ def test_get_box(system): assert math.isclose(a0.value(), a1.value(), rel_tol=1e-4) +@pytest.mark.skipif( + platform != "Linux", + reason="Temporarily skipping test for macOS because the fixed sire 2023.2.3 has not been built for it", +) def test_set_box(system): # Generate box dimensions and angles for a truncated octahedron. box, angles = BSS.Box.truncatedOctahedron(30 * BSS.Units.Length.angstrom) From 55bb6fcae20ab7dae5a4e8aa8f8914cec4dab121 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Mon, 21 Aug 2023 18:33:43 +0800 Subject: [PATCH 10/17] Have amber process write binary RST file instead of text rst7 file (#22) Co-authored-by: William (Zhiyi) Wu --- .../Sandpit/Exscientia/Process/_amber.py | 13 +++++++++---- .../Process/test_position_restraint.py | 16 ++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py index 7595ff33f..742ad8459 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_amber.py @@ -27,6 +27,7 @@ __all__ = ["Amber"] import os +import shutil from pathlib import Path as _Path from .._Utils import _try_import @@ -303,9 +304,11 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if coord_file is not None: try: file = _os.path.splitext(coord_file)[0] - _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) + _IO.saveMolecules(file, system, "rst", property_map=self._property_map) + # To keep the file extension the same. + shutil.move(f"{file}.rst", f"{file}.rst7") except Exception as e: - msg = "Failed to write system to 'RST7' format." + msg = "Failed to write system to 'rst7' format." if _isVerbose(): raise IOError(msg) from e else: @@ -315,9 +318,11 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) if ref_file is not None: try: file = _os.path.splitext(ref_file)[0] - _IO.saveMolecules(file, system, "rst7", property_map=self._property_map) + _IO.saveMolecules(file, system, "rst", property_map=self._property_map) + # To keep the file extension the same. + shutil.move(f"{file}.rst", f"{file}.rst7") except Exception as e: - msg = "Failed to write system to 'RST7' format." + msg = "Failed to write system to 'rst7' format." if _isVerbose(): raise IOError(msg) from e else: diff --git a/tests/Sandpit/Exscientia/Process/test_position_restraint.py b/tests/Sandpit/Exscientia/Process/test_position_restraint.py index 45697b9ed..aa720c470 100644 --- a/tests/Sandpit/Exscientia/Process/test_position_restraint.py +++ b/tests/Sandpit/Exscientia/Process/test_position_restraint.py @@ -5,6 +5,7 @@ import pandas as pd import pytest +from sire.legacy.IO import AmberRst import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Units.Energy import kj_per_mol @@ -121,8 +122,8 @@ def test_gromacs(protocol, system, ref_system, tmp_path): @pytest.mark.skipif( - has_gromacs is False or has_openff is False, - reason="Requires AMBER and openff to be installed", + has_openff is False, + reason="Requires openff to be installed", ) def test_amber(protocol, system, ref_system, tmp_path): proc = BSS.Process.Amber( @@ -137,12 +138,11 @@ def test_amber(protocol, system, ref_system, tmp_path): # We have generated a separate restraint reference assert os.path.exists(proc._ref_file) - contents_ref, contents_rst = ( - open(proc._ref_file).readlines(), - open(proc._rst_file).readlines(), - ) - diff = list(unified_diff(contents_ref, contents_rst)) - assert len(diff) + + ref = AmberRst(proc._ref_file).getFrame(0) + rst = AmberRst(proc._rst_file).getFrame(0) + + assert ref != rst # We are pointing the reference to the correct file assert f"{proc._work_dir}/{proc.getArgs()['-ref']}" == proc._ref_file From 591a535542b7d9a83326c8b0f5b7eda07de29b16 Mon Sep 17 00:00:00 2001 From: "William (Zhiyi) Wu" Date: Mon, 18 Sep 2023 21:47:50 +0800 Subject: [PATCH 11/17] remove skip --- tests/Sandpit/Exscientia/_SireWrappers/test_system.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py index c5e6978c6..55e7a96d9 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py @@ -263,10 +263,6 @@ def test_get_box(system): assert math.isclose(a0.value(), a1.value(), rel_tol=1e-4) -@pytest.mark.skipif( - platform != "Linux", - reason="Temporarily skipping test for macOS because the fixed sire 2023.2.3 has not been built for it", -) def test_set_box(system): # Generate box dimensions and angles for a truncated octahedron. box, angles = BSS.Box.truncatedOctahedron(30 * BSS.Units.Length.angstrom) From e82f367d0108f2d8dd6e83f755fe850b92540f9b Mon Sep 17 00:00:00 2001 From: "William (Zhiyi) Wu" Date: Mon, 18 Sep 2023 21:48:26 +0800 Subject: [PATCH 12/17] fix import --- tests/Sandpit/Exscientia/_SireWrappers/test_system.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py index 55e7a96d9..86d4e0f85 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py @@ -1,6 +1,5 @@ import math import pytest -import platform import BioSimSpace.Sandpit.Exscientia as BSS From d5e28e8afea9a3a7e1b9ab3830cb505768fc0397 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Tue, 19 Sep 2023 17:36:43 +0800 Subject: [PATCH 13/17] Fix test in devel (#27) Co-authored-by: William (Zhiyi) Wu --- tests/Sandpit/Exscientia/_SireWrappers/test_system.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py index 86d4e0f85..f715ee05a 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py @@ -274,7 +274,7 @@ def test_set_box(system): # Store the expected box dimensions and angles. expected_box = [30, 30, 30] * BSS.Units.Length.angstrom - expected_angles = [70.5288, 109.4712, 70.5288] * BSS.Units.Angle.degree + expected_angles = [109.4712, 109.4712, 109.4712] * BSS.Units.Angle.degree # Check that the box dimensions match. for b0, b1 in zip(box, expected_box): From 20da673bfd6b7ecf0a359106b06293a654a3dcd1 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Tue, 19 Sep 2023 21:14:44 +0800 Subject: [PATCH 14/17] Fix the tests such that it no longer require the working directory to be the root directory (#28) Co-authored-by: William (Zhiyi) Wu --- .../Sandpit/Exscientia/Align/test_decouple.py | 5 ++++- .../Sandpit/Exscientia/Align/test_make_ml.py | 5 ++++- tests/Sandpit/Exscientia/Align/test_squash.py | 3 ++- .../Exscientia/Convert/test_convert.py | 5 ++++- .../FreeEnergy/test_alchemical_free_energy.py | 9 ++++++-- tests/Sandpit/Exscientia/Gateway/test_file.py | 5 ++++- .../Exscientia/Gateway/test_fileset.py | 11 +++++----- tests/Sandpit/Exscientia/Gateway/test_node.py | 3 ++- .../Sandpit/Exscientia/IO/test_file_cache.py | 3 ++- tests/Sandpit/Exscientia/IO/test_tuple.py | 3 ++- tests/Sandpit/Exscientia/MD/test_md.py | 11 ++++++---- .../Sandpit/Exscientia/Process/test_amber.py | 18 ++++++++-------- .../Exscientia/Process/test_dummy_protocol.py | 5 ++++- .../Exscientia/Process/test_gromacs.py | 16 ++++++++------ tests/Sandpit/Exscientia/Process/test_namd.py | 7 ++++--- .../Sandpit/Exscientia/Process/test_openmm.py | 5 ++++- tests/Sandpit/Exscientia/Process/test_somd.py | 7 +++++-- .../Sandpit/Exscientia/Stream/test_stream.py | 5 ++++- .../Exscientia/Trajectory/test_trajectory.py | 21 +++++++++++-------- .../Exscientia/_SireWrappers/test_bond.py | 5 ++++- .../Exscientia/_SireWrappers/test_molecule.py | 5 ++++- .../_SireWrappers/test_properties.py | 3 ++- .../_SireWrappers/test_search_result.py | 5 ++++- .../Exscientia/_SireWrappers/test_system.py | 5 ++++- tests/conftest.py | 4 ++++ 25 files changed, 118 insertions(+), 56 deletions(-) diff --git a/tests/Sandpit/Exscientia/Align/test_decouple.py b/tests/Sandpit/Exscientia/Align/test_decouple.py index 210bdd816..150938ddd 100644 --- a/tests/Sandpit/Exscientia/Align/test_decouple.py +++ b/tests/Sandpit/Exscientia/Align/test_decouple.py @@ -6,6 +6,7 @@ from BioSimSpace.Sandpit.Exscientia.Align._decouple import decouple import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp # Store the tutorial URL. url = BSS.tutorialUrl() @@ -14,7 +15,9 @@ @pytest.fixture(scope="session") def mol(): # Alanin-dipeptide. - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"])[0] + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + )[0] def test_sanity(mol): diff --git a/tests/Sandpit/Exscientia/Align/test_make_ml.py b/tests/Sandpit/Exscientia/Align/test_make_ml.py index 908fd3ee5..b2c2a54e9 100644 --- a/tests/Sandpit/Exscientia/Align/test_make_ml.py +++ b/tests/Sandpit/Exscientia/Align/test_make_ml.py @@ -2,6 +2,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Align import make_ml +from tests.conftest import root_fp # Store the tutorial URL. url = BSS.tutorialUrl() @@ -9,7 +10,9 @@ @pytest.fixture def mol(): - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"])[0] + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + )[0] @pytest.fixture diff --git a/tests/Sandpit/Exscientia/Align/test_squash.py b/tests/Sandpit/Exscientia/Align/test_squash.py index bfcc29656..0f01bf088 100644 --- a/tests/Sandpit/Exscientia/Align/test_squash.py +++ b/tests/Sandpit/Exscientia/Align/test_squash.py @@ -7,6 +7,7 @@ from sire.maths import Vector import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp # Make sure AMBER is installed. if BSS._amber_home is not None: @@ -60,7 +61,7 @@ def dual_topology_system(): @pytest.fixture def perturbed_tripeptide(): return pickle.load( - open("tests/Sandpit/Exscientia/input/merged_tripeptide.pickle", "rb") + open(f"{root_fp}/Sandpit/Exscientia/input/merged_tripeptide.pickle", "rb") ) diff --git a/tests/Sandpit/Exscientia/Convert/test_convert.py b/tests/Sandpit/Exscientia/Convert/test_convert.py index 220f48b51..e444c3144 100644 --- a/tests/Sandpit/Exscientia/Convert/test_convert.py +++ b/tests/Sandpit/Exscientia/Convert/test_convert.py @@ -3,11 +3,14 @@ from sire.legacy import Mol as SireMol import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): - return BSS.IO.readMolecules(["tests/input/ala.crd", "tests/input/ala.top"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"] + ) def test_system(system): diff --git a/tests/Sandpit/Exscientia/FreeEnergy/test_alchemical_free_energy.py b/tests/Sandpit/Exscientia/FreeEnergy/test_alchemical_free_energy.py index 5c2b8f952..873c654d5 100644 --- a/tests/Sandpit/Exscientia/FreeEnergy/test_alchemical_free_energy.py +++ b/tests/Sandpit/Exscientia/FreeEnergy/test_alchemical_free_energy.py @@ -14,6 +14,7 @@ has_gromacs, url, ) +from tests.conftest import root_fp import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Protocol import FreeEnergyEquilibration @@ -133,7 +134,9 @@ class TestAnalysePARQUET: @pytest.fixture(scope="class") def data(tmp_path_factory): outdir = tmp_path_factory.mktemp("out") - shutil.copytree("tests/Sandpit/Exscientia/input/parquet", outdir / "parquet") + shutil.copytree( + f"{root_fp}/Sandpit/Exscientia/input/parquet", outdir / "parquet" + ) return str(outdir / "parquet") def test_analyse(self, data): @@ -153,7 +156,9 @@ class Test_gmx_ABFE: @staticmethod @pytest.fixture(scope="class") def freenrg(): - m = BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"])[0] + m = BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + )[0] decouple_m = decouple(m) solvated = BSS.Solvent.tip3p( molecule=decouple_m, box=3 * [3 * BSS.Units.Length.nanometer] diff --git a/tests/Sandpit/Exscientia/Gateway/test_file.py b/tests/Sandpit/Exscientia/Gateway/test_file.py index e36faf36f..ea098822d 100644 --- a/tests/Sandpit/Exscientia/Gateway/test_file.py +++ b/tests/Sandpit/Exscientia/Gateway/test_file.py @@ -1,6 +1,7 @@ import pytest from BioSimSpace.Sandpit.Exscientia.Gateway import File +from tests.conftest import root_fp def test_no_arguments(): @@ -10,7 +11,9 @@ def test_no_arguments(): f = File() -@pytest.mark.parametrize("value", ["tests/input/ala.crd", "tests/input/ala.top"]) +@pytest.mark.parametrize( + "value", [f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"] +) def test_value(value): """Test whether object is initialised correctly and value is set.""" diff --git a/tests/Sandpit/Exscientia/Gateway/test_fileset.py b/tests/Sandpit/Exscientia/Gateway/test_fileset.py index d2e1b0216..9b4bdf9e5 100644 --- a/tests/Sandpit/Exscientia/Gateway/test_fileset.py +++ b/tests/Sandpit/Exscientia/Gateway/test_fileset.py @@ -1,6 +1,7 @@ import pytest from BioSimSpace.Sandpit.Exscientia.Gateway import FileSet +from tests.conftest import root_fp def test_no_arguments(): @@ -13,11 +14,11 @@ def test_no_arguments(): @pytest.mark.parametrize( "value", [ - ["tests/input/ala.crd", "tests/input/ala.top"], + [f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"], [ - "tests/input/alanin.pdb", - "tests/input/alanin.psf", - "tests/input/alanin.params", + f"{root_fp}/input/alanin.pdb", + f"{root_fp}/input/alanin.psf", + f"{root_fp}/input/alanin.params", ], ], ) @@ -58,7 +59,7 @@ def test_missing_files(): # One file missing. with pytest.raises(IOError): f = FileSet(help="Help!") - f.setValue(["tests/input/amber/ala/ala.crd", "missing2.txt"]) + f.setValue([f"{root_fp}/input/amber/ala/ala.crd", "missing2.txt"]) @pytest.mark.parametrize("optional", [True, False]) diff --git a/tests/Sandpit/Exscientia/Gateway/test_node.py b/tests/Sandpit/Exscientia/Gateway/test_node.py index 3d47cbd3a..61e9ff047 100644 --- a/tests/Sandpit/Exscientia/Gateway/test_node.py +++ b/tests/Sandpit/Exscientia/Gateway/test_node.py @@ -4,9 +4,10 @@ import sys import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp # Store the name of the test script. -script_name = "tests/Gateway/node.py" +script_name = f"{root_fp}/Gateway/node.py" # Store the name of the python interpreter. exe = sys.executable diff --git a/tests/Sandpit/Exscientia/IO/test_file_cache.py b/tests/Sandpit/Exscientia/IO/test_file_cache.py index cce3c62a8..92e0a8d91 100644 --- a/tests/Sandpit/Exscientia/IO/test_file_cache.py +++ b/tests/Sandpit/Exscientia/IO/test_file_cache.py @@ -6,6 +6,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import has_amber, has_openff +from tests.conftest import root_fp def test_file_cache(): @@ -18,7 +19,7 @@ def test_file_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"]) + s = BSS.IO.readMolecules([f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"]) # Create a temporary working directory. tmp_dir = tempfile.TemporaryDirectory() diff --git a/tests/Sandpit/Exscientia/IO/test_tuple.py b/tests/Sandpit/Exscientia/IO/test_tuple.py index f4fc601bb..1f8700888 100644 --- a/tests/Sandpit/Exscientia/IO/test_tuple.py +++ b/tests/Sandpit/Exscientia/IO/test_tuple.py @@ -1,6 +1,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp def test_tuple(): """Check that we can read from a tuple of files.""" - BSS.IO.readMolecules(("tests/input/ala.crd", "tests/input/ala.top")) + BSS.IO.readMolecules((f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top")) diff --git a/tests/Sandpit/Exscientia/MD/test_md.py b/tests/Sandpit/Exscientia/MD/test_md.py index f479d9ab1..f2afe28db 100644 --- a/tests/Sandpit/Exscientia/MD/test_md.py +++ b/tests/Sandpit/Exscientia/MD/test_md.py @@ -3,6 +3,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_amber, has_gromacs, has_namd +from tests.conftest import root_fp @pytest.mark.skipif(has_amber is False, reason="Requires AMBER to be installed.") @@ -13,7 +14,9 @@ def test_amber(): protocol = BSS.Protocol.Minimisation(steps=100) # Load the molecular system. - system = BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + system = BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) # Initialise the AMBER process. process = BSS.MD.run(system, protocol, name="test") @@ -55,9 +58,9 @@ def test_namd(): # Load the molecular system. system = BSS.IO.readMolecules( [ - "tests/input/alanin.psf", - "tests/input/alanin.pdb", - "tests/input/alanin.params", + f"{root_fp}/input/alanin.psf", + f"{root_fp}/input/alanin.pdb", + f"{root_fp}/input/alanin.params", ] ) diff --git a/tests/Sandpit/Exscientia/Process/test_amber.py b/tests/Sandpit/Exscientia/Process/test_amber.py index 489f5bd84..60168a707 100644 --- a/tests/Sandpit/Exscientia/Process/test_amber.py +++ b/tests/Sandpit/Exscientia/Process/test_amber.py @@ -10,12 +10,15 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_amber, has_pyarrow +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.mark.skipif( @@ -280,9 +283,9 @@ def test_parse_fep_output(system, protocol): # Assign the path to the output file. if isinstance(protocol, BSS.Protocol.FreeEnergy): - out_file = "tests/Sandpit/Exscientia/output/amber_fep.out" + out_file = f"{root_fp}/Sandpit/Exscientia/output/amber_fep.out" else: - out_file = "tests/Sandpit/Exscientia/output/amber_fep_min.out" + out_file = f"{root_fp}/Sandpit/Exscientia/output/amber_fep_min.out" # Copy the existing output file into the working directory. shutil.copyfile(out_file, process.workDir() + "/amber.out") @@ -320,7 +323,6 @@ def test_parse_fep_output(system, protocol): assert len(records_sc1) != 0 - class TestsaveMetric: @staticmethod @pytest.fixture() @@ -334,7 +336,6 @@ def alchemical_system(system): system_copy.updateMolecule(0, mol) return system_copy - @staticmethod @pytest.fixture() def setup(alchemical_system): @@ -344,7 +345,7 @@ def setup(alchemical_system): BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) shutil.copyfile( - "tests/Sandpit/Exscientia/output/amber_fep.out", + f"{root_fp}/Sandpit/Exscientia/output/amber_fep.out", process.workDir() + "/amber.out", ) process.saveMetric() @@ -357,10 +358,9 @@ def test_error_alchemlyb_extract(self, alchemical_system): BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) process.wait() - with open(process.workDir() + '/amber.err', 'r') as f: + with open(process.workDir() + "/amber.err", "r") as f: text = f.read() - assert 'Exception Information' in text - + assert "Exception Information" in text def test_metric_parquet_exist(self, setup): assert Path(f"{setup.workDir()}/metric.parquet").exists() diff --git a/tests/Sandpit/Exscientia/Process/test_dummy_protocol.py b/tests/Sandpit/Exscientia/Process/test_dummy_protocol.py index a50c5e533..fa063d427 100644 --- a/tests/Sandpit/Exscientia/Process/test_dummy_protocol.py +++ b/tests/Sandpit/Exscientia/Process/test_dummy_protocol.py @@ -1,6 +1,7 @@ import pytest from tests.Sandpit.Exscientia.conftest import has_amber, has_gromacs +from tests.conftest import root_fp import BioSimSpace.Sandpit.Exscientia as BSS from BioSimSpace.Sandpit.Exscientia.Process._process import Process @@ -9,7 +10,9 @@ @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.mark.skipif( diff --git a/tests/Sandpit/Exscientia/Process/test_gromacs.py b/tests/Sandpit/Exscientia/Process/test_gromacs.py index 1a42967c5..7d4e57c49 100644 --- a/tests/Sandpit/Exscientia/Process/test_gromacs.py +++ b/tests/Sandpit/Exscientia/Process/test_gromacs.py @@ -27,12 +27,15 @@ has_openff, has_pyarrow, ) +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.fixture(scope="session") @@ -254,7 +257,7 @@ def setup(perturbable_system): ) process = BSS.Process.Gromacs(perturbable_system, protocol) shutil.copyfile( - "tests/Sandpit/Exscientia/output/gromacs.edr", + f"{root_fp}/Sandpit/Exscientia/output/gromacs.edr", process.workDir() + "/gromacs.edr", ) shutil.copyfile( @@ -343,17 +346,18 @@ def test_u_nk_parquet(self, setup): def test_error_alchemlyb_extract(self, perturbable_system, monkeypatch): def extract(*args): - raise ValueError('alchemlyb.parsing.gmx.extract failed.') - monkeypatch.setattr('alchemlyb.parsing.gmx.extract', extract) + raise ValueError("alchemlyb.parsing.gmx.extract failed.") + + monkeypatch.setattr("alchemlyb.parsing.gmx.extract", extract) # Create a process using any system and the protocol. process = BSS.Process.Gromacs( perturbable_system, BSS.Protocol.FreeEnergy(temperature=298 * BSS.Units.Temperature.kelvin), ) process.wait() - with open(process.workDir() + '/gromacs.err', 'r') as f: + with open(process.workDir() + "/gromacs.err", "r") as f: text = f.read() - assert 'Exception Information' in text + assert "Exception Information" in text @pytest.mark.skipif( diff --git a/tests/Sandpit/Exscientia/Process/test_namd.py b/tests/Sandpit/Exscientia/Process/test_namd.py index 5327a4a28..3ad97f742 100644 --- a/tests/Sandpit/Exscientia/Process/test_namd.py +++ b/tests/Sandpit/Exscientia/Process/test_namd.py @@ -3,6 +3,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_namd +from tests.conftest import root_fp @pytest.fixture(scope="session") @@ -10,9 +11,9 @@ def system(): """Re-use the same molecuar system for each test.""" return BSS.IO.readMolecules( [ - "tests/input/alanin.psf", - f"tests/input/alanin.pdb", - f"tests/input/alanin.params", + f"{root_fp}/input/alanin.psf", + f"{root_fp}/input/alanin.pdb", + f"{root_fp}/input/alanin.params", ] ) diff --git a/tests/Sandpit/Exscientia/Process/test_openmm.py b/tests/Sandpit/Exscientia/Process/test_openmm.py index f4a55342b..10c9f0045 100644 --- a/tests/Sandpit/Exscientia/Process/test_openmm.py +++ b/tests/Sandpit/Exscientia/Process/test_openmm.py @@ -4,12 +4,15 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_amber, has_gromacs, has_openff +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.mark.parametrize("restraint", ["backbone", "heavy", "all", "none"]) diff --git a/tests/Sandpit/Exscientia/Process/test_somd.py b/tests/Sandpit/Exscientia/Process/test_somd.py index 11ecc6ba0..ca81a539c 100644 --- a/tests/Sandpit/Exscientia/Process/test_somd.py +++ b/tests/Sandpit/Exscientia/Process/test_somd.py @@ -12,6 +12,7 @@ import warnings import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp # Store the tutorial URL. url = BSS.tutorialUrl() @@ -20,7 +21,9 @@ @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.fixture(scope="session") @@ -91,7 +94,7 @@ def test_pert_file(): BSS.Process._somd._to_pert_file(mol) # Check that the files are the same. - assert filecmp.cmp("MORPH.pert", "tests/input/morph.pert") + assert filecmp.cmp("MORPH.pert", f"{root_fp}/input/morph.pert") # Remove the temporary perturbation file. os.remove("MORPH.pert") diff --git a/tests/Sandpit/Exscientia/Stream/test_stream.py b/tests/Sandpit/Exscientia/Stream/test_stream.py index bf40c3993..432110cbc 100644 --- a/tests/Sandpit/Exscientia/Stream/test_stream.py +++ b/tests/Sandpit/Exscientia/Stream/test_stream.py @@ -2,11 +2,14 @@ import pytest import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp @pytest.fixture def system(scope="session"): - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.fixture(autouse=True) diff --git a/tests/Sandpit/Exscientia/Trajectory/test_trajectory.py b/tests/Sandpit/Exscientia/Trajectory/test_trajectory.py index 667c0fea7..6711d806e 100644 --- a/tests/Sandpit/Exscientia/Trajectory/test_trajectory.py +++ b/tests/Sandpit/Exscientia/Trajectory/test_trajectory.py @@ -11,20 +11,23 @@ def wrap(arg): from tests.Sandpit.Exscientia.conftest import has_mdanalysis, has_mdtraj +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): """A system object with the same topology as the trajectories.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.fixture(scope="session") def traj_sire(system): """A trajectory object using the Sire backend.""" return BSS.Trajectory.Trajectory( - trajectory="tests/input/ala.trr", - topology="tests/input/ala.gro", + trajectory=f"{root_fp}/input/ala.trr", + topology=f"{root_fp}/input/ala.gro", system=system, backend="SIRE", ) @@ -34,8 +37,8 @@ def traj_sire(system): def traj_mdtraj(system): """A trajectory object using the MDTraj backend.""" return BSS.Trajectory.Trajectory( - trajectory="tests/input/ala.trr", - topology="tests/input/ala.gro", + trajectory=f"{root_fp}/input/ala.trr", + topology=f"{root_fp}/input/ala.gro", system=system, backend="MDTRAJ", ) @@ -45,8 +48,8 @@ def traj_mdtraj(system): def traj_mdanalysis(system): """A trajectory object using the MDAnalysis backend.""" return BSS.Trajectory.Trajectory( - trajectory="tests/input/ala.trr", - topology="tests/input/ala.tpr", + trajectory=f"{root_fp}/input/ala.trr", + topology=f"{root_fp}/input/ala.tpr", system=system, backend="MDANALYSIS", ) @@ -58,8 +61,8 @@ def traj_mdanalysis_pdb(system): new_system = system.copy() new_system._sire_object.setProperty("fileformat", wrap("PDB")) return BSS.Trajectory.Trajectory( - trajectory="tests/input/ala.trr", - topology="tests/input/ala.tpr", + trajectory=f"{root_fp}/input/ala.trr", + topology=f"{root_fp}/input/ala.tpr", system=new_system, backend="MDANALYSIS", ) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_bond.py b/tests/Sandpit/Exscientia/_SireWrappers/test_bond.py index bab5be6d8..861e49241 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_bond.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_bond.py @@ -1,10 +1,13 @@ import pytest import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp def test_bonds(): - system = BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + system = BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) bonds = system.search("bonds") diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py index f66f70db4..d19b385f8 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py @@ -3,11 +3,14 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_amber, has_pyarrow +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.mark.skipif( diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_properties.py b/tests/Sandpit/Exscientia/_SireWrappers/test_properties.py index 545bf249c..47bf616f1 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_properties.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_properties.py @@ -1,8 +1,9 @@ import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp def test_sire_properties(): - s = BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + s = BSS.IO.readMolecules([f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"]) m = s[0]._sire_object diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_search_result.py b/tests/Sandpit/Exscientia/_SireWrappers/test_search_result.py index 45903da4f..aae552377 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_search_result.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_search_result.py @@ -1,6 +1,7 @@ import pytest import BioSimSpace.Sandpit.Exscientia as BSS +from tests.conftest import root_fp # Store the tutorial URL. url = BSS.tutorialUrl() @@ -9,7 +10,9 @@ @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) @pytest.fixture(scope="session") diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py index f715ee05a..1d8948520 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_system.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_system.py @@ -4,12 +4,15 @@ import BioSimSpace.Sandpit.Exscientia as BSS from tests.Sandpit.Exscientia.conftest import url, has_amber, has_openff +from tests.conftest import root_fp @pytest.fixture(scope="session") def system(): """Re-use the same molecuar system for each test.""" - return BSS.IO.readMolecules(["tests/input/ala.top", "tests/input/ala.crd"]) + return BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + ) # Parameterise the function with a set of molecule indices. diff --git a/tests/conftest.py b/tests/conftest.py index 9c547f7c3..b303e6d82 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,5 @@ +from pathlib import Path + collect_ignore_glob = ["*/out_test*.py"] import os @@ -55,3 +57,5 @@ ) has_mdrestraints_generator = _have_imported(MDRestraintsGenerator) + +root_fp = Path(__file__).parent.resolve() From 3c4a23ddeb9b8d6e587612253eaf033dcc9acdf8 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Tue, 19 Sep 2023 23:22:30 +0800 Subject: [PATCH 15/17] Add a function for marking the alchemical ion (#30) Co-authored-by: William (Zhiyi) Wu --- .../Sandpit/Exscientia/Align/_alch_ion.py | 52 +++++++++++++++++++ .../Exscientia/_SireWrappers/_molecule.py | 12 +++++ .../Exscientia/_SireWrappers/_system.py | 27 ++++++++++ .../Exscientia/Align/test_alchemical_ion.py | 49 +++++++++++++++++ 4 files changed, 140 insertions(+) create mode 100644 python/BioSimSpace/Sandpit/Exscientia/Align/_alch_ion.py create mode 100644 tests/Sandpit/Exscientia/Align/test_alchemical_ion.py diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_alch_ion.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_alch_ion.py new file mode 100644 index 000000000..14773adf6 --- /dev/null +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_alch_ion.py @@ -0,0 +1,52 @@ +import warnings + +from .._SireWrappers import Molecule as _Molecule + + +def _mark_alchemical_ion(molecule): + """ + Mark the ion molecule as being alchemical ion. + + This enables one to use + + * :meth:`~BioSimSpace.Sandpit.Exscientia._SireWrappers._system.System.getAlchemicalIon` to get the alchemical ion. + * :meth:`~BioSimSpace.Sandpit.Exscientia._SireWrappers._system.System.getAlchemicalIonIdx` to get the index of alchemical ion. + * :meth:`~BioSimSpace.Sandpit.Exscientia._SireWrappers._molecule.Molecule.isAlchemicalIon` to check if a molecule is an alchemical ion. + + + Parameters + ---------- + + molecule : BioSimSpace._SireWrappers.Molecule + The molecule to be marked as alchemical ion. + + Returns + ------- + + alchemical_ion : BSS._SireWrappers.Molecule + The molecule marked as being alchemical ion. + """ + # Validate input. + + if not isinstance(molecule, _Molecule): + raise TypeError( + "'molecule' must be of type 'BioSimSpace._SireWrappers.Molecule'" + ) + + # Cannot decouple a perturbable molecule. + if molecule.isAlchemicalIon(): + warnings.warn("'molecule' has already been marked as alchemical ion!") + + # Create a copy of this molecule. + mol = _Molecule(molecule) + mol_sire = mol._sire_object + + # Edit the molecule + mol_edit = mol_sire.edit() + + mol_edit.setProperty("AlchemicalIon", True) + + # Update the Sire molecule object of the new molecule. + mol._sire_object = mol_edit.commit() + + return mol diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py index 005a68d63..05ca21681 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py @@ -518,6 +518,18 @@ def isML(self): else: return False + def isAlchemicalIon(self): + """ + Whether this molecule is marked as Alchemical Ion. + + Returns + ------- + + isAlchemicalIon : bool + Whether the molecule is marked as Alchemical Ion. + """ + return self._sire_object.hasProperty("AlchemicalIon") + def isWater(self, property_map={}): """ Whether this is a water molecule. diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py index 8793472ef..280e01327 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_system.py @@ -1115,6 +1115,33 @@ def nMLMolecules(self): """ return len(self.getMLMolecules()) + def getAlchemicalIon(self): + """ + Return the Alchemical Ion in the system. + + Returns + ------- + + molecule : [:class:`Molecule `] + The Alchemical Ion or None if there isn't any. + """ + try: + return self.search("mols with property AlchemicalIon").molecules()[0] + except: + return None + + def getAlchemicalIonIdx(self): + """ + Return the index of Alchemical Ion in the system. + + Returns + ------- + + index : int + The index of Alchemical Ion in the system. + """ + return self.getIndex(self.getAlchemicalIon()) + def repartitionHydrogenMass( self, factor=4, water="no", use_coordinates=False, property_map={} ): diff --git a/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py b/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py new file mode 100644 index 000000000..3b56be919 --- /dev/null +++ b/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py @@ -0,0 +1,49 @@ +import pytest + +import BioSimSpace.Sandpit.Exscientia as BSS +from BioSimSpace.Sandpit.Exscientia.Align._alch_ion import _mark_alchemical_ion +from BioSimSpace.Sandpit.Exscientia._SireWrappers import Molecule + +from tests.conftest import root_fp + + +@pytest.fixture +def system(): + mol = BSS.IO.readMolecules( + [f"{root_fp}/input/ala.top", f"{root_fp}/input/ala.crd"] + )[0] + system = BSS.Solvent.tip3p(mol, ion_conc=0.15, shell=2 * BSS.Units.Length.nanometer) + return system + + +@pytest.fixture +def alchemical_ion_system(system): + ion = system[-1] + ion = _mark_alchemical_ion(ion) + system.updateMolecules(ion) + return system + + +@pytest.mark.parametrize( + "input_system,isalchem", [("system", False), ("alchemical_ion_system", True)] +) +def test_isAlchemicalIon(input_system, isalchem, request): + system = request.getfixturevalue(input_system) + assert system[-1].isAlchemicalIon() is isalchem + + +@pytest.mark.parametrize( + "input_system,isalchem", [("system", None), ("alchemical_ion_system", True)] +) +def test_getAlchemicalIon(input_system, isalchem, request): + system = request.getfixturevalue(input_system) + ion = system.getAlchemicalIon() + if isalchem is None: + assert ion is None + else: + assert isinstance(ion, Molecule) + + +def test_getAlchemicalIonIdx(alchemical_ion_system): + index = alchemical_ion_system.getAlchemicalIonIdx() + assert index == 680 From b9f1c84abe30e8bef5d55194565b5709b1919c56 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 12 Oct 2023 19:28:42 +0100 Subject: [PATCH 16/17] Increase distance tolerance. This is not reproducibly satisfied on all platforms and Python variants. --- tests/Sandpit/Exscientia/Process/test_openmm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Sandpit/Exscientia/Process/test_openmm.py b/tests/Sandpit/Exscientia/Process/test_openmm.py index 3c45e7fb9..aec8406af 100644 --- a/tests/Sandpit/Exscientia/Process/test_openmm.py +++ b/tests/Sandpit/Exscientia/Process/test_openmm.py @@ -70,7 +70,7 @@ def test_minimise(system, restraint): ) # Run the process, check that it finished without error, and returns a system. - run_process(system, protocol, restraint=restraint, tolerance=0.05) + run_process(system, protocol, restraint=restraint, tolerance=0.1) @pytest.mark.parametrize("restraint", ["backbone", "heavy", "all", "none"]) From 4cceb23955057e204d64f19bcb084e699275a099 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 12 Oct 2023 19:29:29 +0100 Subject: [PATCH 17/17] Guard tests against missing packages. --- tests/Sandpit/Exscientia/Align/test_alchemical_ion.py | 5 ++++- tests/Sandpit/Exscientia/Process/test_position_restraint.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py b/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py index 3b56be919..fa2a0aeb6 100644 --- a/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py +++ b/tests/Sandpit/Exscientia/Align/test_alchemical_ion.py @@ -4,7 +4,7 @@ from BioSimSpace.Sandpit.Exscientia.Align._alch_ion import _mark_alchemical_ion from BioSimSpace.Sandpit.Exscientia._SireWrappers import Molecule -from tests.conftest import root_fp +from tests.conftest import root_fp, has_gromacs @pytest.fixture @@ -24,6 +24,7 @@ def alchemical_ion_system(system): return system +@pytest.mark.skipif(has_gromacs is False, reason="Requires GROMACS to be installed.") @pytest.mark.parametrize( "input_system,isalchem", [("system", False), ("alchemical_ion_system", True)] ) @@ -32,6 +33,7 @@ def test_isAlchemicalIon(input_system, isalchem, request): assert system[-1].isAlchemicalIon() is isalchem +@pytest.mark.skipif(has_gromacs is False, reason="Requires GROMACS to be installed.") @pytest.mark.parametrize( "input_system,isalchem", [("system", None), ("alchemical_ion_system", True)] ) @@ -44,6 +46,7 @@ def test_getAlchemicalIon(input_system, isalchem, request): assert isinstance(ion, Molecule) +@pytest.mark.skipif(has_gromacs is False, reason="Requires GROMACS to be installed.") def test_getAlchemicalIonIdx(alchemical_ion_system): index = alchemical_ion_system.getAlchemicalIonIdx() assert index == 680 diff --git a/tests/Sandpit/Exscientia/Process/test_position_restraint.py b/tests/Sandpit/Exscientia/Process/test_position_restraint.py index aa720c470..7355adc70 100644 --- a/tests/Sandpit/Exscientia/Process/test_position_restraint.py +++ b/tests/Sandpit/Exscientia/Process/test_position_restraint.py @@ -122,8 +122,8 @@ def test_gromacs(protocol, system, ref_system, tmp_path): @pytest.mark.skipif( - has_openff is False, - reason="Requires openff to be installed", + has_amber is False or has_openff is False, + reason="Requires AMBER and openff to be installed", ) def test_amber(protocol, system, ref_system, tmp_path): proc = BSS.Process.Amber(