From 3ebc80f269b9cce294d438cafa6086f46495da7e Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 12 Jun 2025 11:08:30 +0100 Subject: [PATCH 1/3] Backport fix from #403. [ci skip] --- python/BioSimSpace/Process/_amber.py | 69 +++++++++++++++++++--------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/python/BioSimSpace/Process/_amber.py b/python/BioSimSpace/Process/_amber.py index c585c670..59d14b36 100644 --- a/python/BioSimSpace/Process/_amber.py +++ b/python/BioSimSpace/Process/_amber.py @@ -732,31 +732,58 @@ def getFrame(self, index): # Create a copy of the existing system object. old_system = self._system.copy() - # Update the coordinates and velocities and return a mapping between - # the molecule indices in the two systems. - sire_system, mapping = _SireIO.updateCoordinatesAndVelocities( - old_system._sire_object, - new_system._sire_object, - self._mapping, - is_lambda1, - self._property_map, - self._property_map, - ) + if isinstance(self._protocol, _Protocol._FreeEnergyMixin): + # Udpate the coordinates and velocities and return a mapping between + # the molecule indices in the two systems. + mapping = { + _SireMol.MolIdx(x): _SireMol.MolIdx(x) + for x in range(0, self._squashed_system.nMolecules()) + } + ( + self._squashed_system._sire_object, + _, + ) = _SireIO.updateCoordinatesAndVelocities( + self._squashed_system._sire_object, + new_system._sire_object, + mapping, + is_lambda1, + self._property_map, + self._property_map, + ) + + # Update the unsquashed system based on the updated squashed system. + old_system = _unsquash( + old_system, + self._squashed_system, + self._mapping, + explicit_dummies=self._explicit_dummies, + ) + + else: + # Update the coordinates and velocities and return a mapping between + # the molecule indices in the two systems. + sire_system, mapping = _SireIO.updateCoordinatesAndVelocities( + old_system._sire_object, + new_system._sire_object, + self._mapping, + is_lambda1, + self._property_map, + self._property_map, + ) - # Update the underlying Sire object. - old_system._sire_object = sire_system + # Update the underlying Sire object. + old_system._sire_object = sire_system - # Store the mapping between the MolIdx in both systems so we don't - # need to recompute it next time. - self._mapping = mapping + # Store the mapping between the MolIdx in both systems so we don't + # need to recompute it next time. + self._mapping = mapping # Update the box information in the original system. - if self._has_box: - if "space" in new_system._sire_object.propertyKeys(): - box = new_system._sire_object.property("space") - old_system._sire_object.setProperty( - self._property_map.get("space", "space"), box - ) + if "space" in new_system._sire_object.propertyKeys(): + box = new_system._sire_object.property("space") + old_system._sire_object.setProperty( + self._property_map.get("space", "space"), box + ) return old_system From 12ca1bd7e56be74b54fdaefd3039a6522a7720b1 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 12 Jun 2025 11:11:56 +0100 Subject: [PATCH 2/3] Backport fixes from PR #400. [ci skip] --- python/BioSimSpace/Parameters/_parameters.py | 51 ------------------- .../Exscientia/Parameters/_parameters.py | 51 ------------------- tests/Parameters/test_parameters.py | 5 +- .../Exscientia/Parameters/test_parameters.py | 5 +- 4 files changed, 8 insertions(+), 104 deletions(-) diff --git a/python/BioSimSpace/Parameters/_parameters.py b/python/BioSimSpace/Parameters/_parameters.py index 9e2bc5d8..dade5ac5 100644 --- a/python/BioSimSpace/Parameters/_parameters.py +++ b/python/BioSimSpace/Parameters/_parameters.py @@ -534,57 +534,6 @@ def _parameterise_openff( "must be in your PATH." ) from None - # Check the Antechamber version. Open Force Field requires Antechamber >= 22.0. - try: - # Antechamber returns an exit code of 1 when requesting version information. - # As such, we wrap the call within a try-except block in case it fails. - - import shlex as _shlex - import subprocess as _subprocess - - # Generate the command-line string. (Antechamber must be in the PATH, - # so no need to use AMBERHOME. - command = "antechamber -v" - - # Run the command as a subprocess. - proc = _subprocess.run( - _Utils.command_split(command), - shell=False, - text=True, - stdout=_subprocess.PIPE, - stderr=_subprocess.STDOUT, - ) - - # Get stdout and split into lines. - lines = proc.stdout.split("\n") - - # If present, version information is on line 1. - string = lines[1] - - # Delete the welcome message. - string = string.replace("Welcome to antechamber", "") - - # Extract the version and convert to float. - version = float(string.split(":")[0]) - - # The version is okay, enable Open Force Field support. - if version >= 22: - is_compatible = True - # Disable Open Force Field support. - else: - is_compatible = False - - del _shlex - del _subprocess - - # Something went wrong, disable Open Force Field support. - except: - is_compatible = False - raise - - if not is_compatible: - raise _IncompatibleError(f"'{forcefield}' requires Antechamber >= 22.0") - # Validate arguments. if not isinstance(molecule, (_Molecule, str)): diff --git a/python/BioSimSpace/Sandpit/Exscientia/Parameters/_parameters.py b/python/BioSimSpace/Sandpit/Exscientia/Parameters/_parameters.py index 9e2bc5d8..dade5ac5 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Parameters/_parameters.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Parameters/_parameters.py @@ -534,57 +534,6 @@ def _parameterise_openff( "must be in your PATH." ) from None - # Check the Antechamber version. Open Force Field requires Antechamber >= 22.0. - try: - # Antechamber returns an exit code of 1 when requesting version information. - # As such, we wrap the call within a try-except block in case it fails. - - import shlex as _shlex - import subprocess as _subprocess - - # Generate the command-line string. (Antechamber must be in the PATH, - # so no need to use AMBERHOME. - command = "antechamber -v" - - # Run the command as a subprocess. - proc = _subprocess.run( - _Utils.command_split(command), - shell=False, - text=True, - stdout=_subprocess.PIPE, - stderr=_subprocess.STDOUT, - ) - - # Get stdout and split into lines. - lines = proc.stdout.split("\n") - - # If present, version information is on line 1. - string = lines[1] - - # Delete the welcome message. - string = string.replace("Welcome to antechamber", "") - - # Extract the version and convert to float. - version = float(string.split(":")[0]) - - # The version is okay, enable Open Force Field support. - if version >= 22: - is_compatible = True - # Disable Open Force Field support. - else: - is_compatible = False - - del _shlex - del _subprocess - - # Something went wrong, disable Open Force Field support. - except: - is_compatible = False - raise - - if not is_compatible: - raise _IncompatibleError(f"'{forcefield}' requires Antechamber >= 22.0") - # Validate arguments. if not isinstance(molecule, (_Molecule, str)): diff --git a/tests/Parameters/test_parameters.py b/tests/Parameters/test_parameters.py index 61061d56..9b2d68ed 100644 --- a/tests/Parameters/test_parameters.py +++ b/tests/Parameters/test_parameters.py @@ -169,8 +169,11 @@ def test_smiles_stereo(): assert rdmol0_smiles == rdmol1_smiles +# This test is currently skipped since it fails with AnteChamber verssion +# 24.0 and above and there is no way to query the version number from +# the command-line. (The version output has been removed.) @pytest.mark.skipif( - has_antechamber is False or has_tleap is False, + True or has_antechamber is False or has_tleap is False, reason="Requires AmberTools/antechamber and tLEaP to be installed.", ) def test_acdoctor(): diff --git a/tests/Sandpit/Exscientia/Parameters/test_parameters.py b/tests/Sandpit/Exscientia/Parameters/test_parameters.py index d6699346..e23f7f4e 100644 --- a/tests/Sandpit/Exscientia/Parameters/test_parameters.py +++ b/tests/Sandpit/Exscientia/Parameters/test_parameters.py @@ -174,8 +174,11 @@ def test_smiles_stereo(): assert rdmol0_smiles == rdmol1_smiles +# This test is currently skipped since it fails with AnteChamber verssion +# 24.0 and above and there is no way to query the version number from +# the command-line. (The version output has been removed.) @pytest.mark.skipif( - has_antechamber is False or has_tleap is False, + True or has_antechamber is False or has_tleap is False, reason="Requires AmberTools/antechamber and tLEaP to be installed.", ) def test_acdoctor(): From f26feb13d4d62e29b0a92457309755d1213722e2 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 12 Jun 2025 11:15:02 +0100 Subject: [PATCH 3/3] Backport Recursion sandpit synchronisation updates. [ci skip] --- .../Sandpit/Exscientia/Process/_gromacs.py | 30 +++++++++- .../Sandpit/Exscientia/Process/_process.py | 53 ++++++++++++++++-- .../Exscientia/Units/Volume/__init__.py | 3 +- .../Exscientia/Process/test_gromacs.py | 6 +- .../Exscientia/Process/test_process_wait.py | 55 +++++++++++++++++++ 5 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 tests/Sandpit/Exscientia/Process/test_process_wait.py diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index 78f48f4e..357f26c4 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -1930,6 +1930,29 @@ def getCurrentPressure(self, time_series=False): """ return self.getPressure(time_series, block=False) + def getDensity(self, time_series=False, block="AUTO"): + """ + Get the Density. + + Parameters + ---------- + + time_series : bool + Whether to return a list of time series records. + + block : bool + Whether to block until the process has finished running. + + Returns + ------- + + density : :class:`GeneralUnit ` + The Density. + """ + return self.getRecord( + "DENSITY", time_series, _Units.Mass.kilogram / _Units.Volume.meter3, block + ) + def getPressureDC(self, time_series=False, block="AUTO"): """ Get the DC pressure. @@ -2489,7 +2512,7 @@ def _parse_energy_units(text): elif unit == "nm/ps": units.append(_Units.Length.nanometer / _Units.Time.picosecond) elif unit == "kg/m^3": - units.append(_Types._GeneralUnit("kg/m3")) + units.append(_Units.Mass.kilogram / _Units.Volume.meter3) else: units.append(1.0) _warnings.warn( @@ -2935,6 +2958,11 @@ def _saveMetric( _Units.Temperature.kelvin, "getTemperature", ), + ( + "Density (g/cm^3)", + _Units.Mass.gram / _Units.Volume.centimeter3, + "getDensity", + ), ] ) df = self._convert_datadict_keys(datadict_keys) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py index bc77e0aa..fdcca27c 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_process.py @@ -32,6 +32,7 @@ import traceback import pandas as pd +from loguru import logger from .._Utils import _try_import @@ -53,6 +54,7 @@ from ..Protocol._protocol import Protocol as _Protocol from .._SireWrappers import System as _System from ..Types._type import Type as _Type +from ..Types import Time as _Time from .. import Units as _Units from .. import _Utils from ..FreeEnergy._restraint import Restraint as _Restraint @@ -898,7 +900,7 @@ def setSeed(self, seed): else: self._seed = seed - def wait(self, max_time=None): + def wait(self, max_time=None, inactivity_timeout: None | _Time = None): """ Wait for the process to finish. @@ -939,11 +941,52 @@ def wait(self, max_time=None): self._process.wait(max_time) else: - # Wait for the process to finish. - self._process.wait() + if inactivity_timeout is None: + # Wait for the process to finish. + self._process.wait() - # Store the final run time. - self.runTime() + # Store the final run time. + self.runTime() + else: + inactivity_timeout = int(inactivity_timeout.milliseconds().value()) + last_time = self._getLastTime() + if last_time is None: + # Wait for the process to finish. + self._process.wait() + + # Store the final run time. + self.runTime() + else: + while self.isRunning(): + self._process.wait(inactivity_timeout) + if self.isRunning(): + current_time = self._getLastTime() + if current_time > last_time: + logger.info( + f"Current simulation time ({current_time})." + ) + last_time = current_time + else: + logger.warning( + f"Current simulation time ({current_time}) has not advanced compared " + f"to the last time ({last_time}). The process " + f"might have hung and will be killed." + ) + with open( + f"{self.workDir()}/{self._name}.out", "a+" + ) as f: + f.write("Process Hung. Killed.") + self.kill() + + def _getLastTime(self) -> float | None: + """This is the base method in the Process base class. + Each subclass, such as AMBER or GROMACS, is expected to override this method + to provide their own implementation for returning the current time. + + If this method is not overridden, it will return None, + and the `inactivity_timeout` feature will be skipped. + """ + return None def isQueued(self): """ diff --git a/python/BioSimSpace/Sandpit/Exscientia/Units/Volume/__init__.py b/python/BioSimSpace/Sandpit/Exscientia/Units/Volume/__init__.py index 76e2864b..6b5911e2 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Units/Volume/__init__.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Units/Volume/__init__.py @@ -24,7 +24,7 @@ __author__ = "Lester Hedges" __email__ = "lester.hedges@gmail.com" -__all__ = ["meter3", "nanometer3", "angstrom3", "picometer3"] +__all__ = ["meter3", "nanometer3", "angstrom3", "picometer3", "centimeter3"] from ...Types import Volume as _Volume @@ -32,3 +32,4 @@ nanometer3 = _Volume(1, "nanometer3") angstrom3 = _Volume(1, "angstrom3") picometer3 = _Volume(1, "picometer3") +centimeter3 = _Volume(1e-6, "meter3") diff --git a/tests/Sandpit/Exscientia/Process/test_gromacs.py b/tests/Sandpit/Exscientia/Process/test_gromacs.py index 70250a84..78ccfe72 100644 --- a/tests/Sandpit/Exscientia/Process/test_gromacs.py +++ b/tests/Sandpit/Exscientia/Process/test_gromacs.py @@ -12,10 +12,11 @@ from BioSimSpace.Sandpit.Exscientia.Units.Angle import radian from BioSimSpace.Sandpit.Exscientia.Units.Energy import kcal_per_mol, kj_per_mol from BioSimSpace.Sandpit.Exscientia.Units.Length import angstrom +from BioSimSpace.Sandpit.Exscientia.Units.Mass import gram from BioSimSpace.Sandpit.Exscientia.Units.Pressure import bar from BioSimSpace.Sandpit.Exscientia.Units.Temperature import kelvin from BioSimSpace.Sandpit.Exscientia.Units.Time import picosecond -from BioSimSpace.Sandpit.Exscientia.Units.Volume import nanometer3 +from BioSimSpace.Sandpit.Exscientia.Units.Volume import centimeter3, nanometer3 from tests.Sandpit.Exscientia.conftest import ( has_alchemtest, has_amber, @@ -359,6 +360,8 @@ def setup(perturbable_system): ("getPressureDC", False, -215.590363, bar), ("getVolume", True, 44.679958, nanometer3), ("getVolume", False, 44.523510, nanometer3), + ("getDensity", False, 1.027221558, gram / centimeter3), + ("getDensity", True, 1.023624695, gram / centimeter3), ], ) def test_get(self, setup, func, time_series, value, unit): @@ -378,6 +381,7 @@ def test_metric_parquet(self, setup): assert np.isclose(df["Volume (nm^3)"][0.0], 44.679958) assert np.isclose(df["Pressure (bar)"][0.0], 119.490417) assert np.isclose(df["Temperature (kelvin)"][0.0], 306.766907) + assert np.isclose(df["Density (g/cm^3)"][0.0], 1.023624695) def test_dhdl_parquet_exist(self, setup): assert Path(f"{setup.workDir()}/dHdl.parquet").exists() diff --git a/tests/Sandpit/Exscientia/Process/test_process_wait.py b/tests/Sandpit/Exscientia/Process/test_process_wait.py new file mode 100644 index 00000000..7f80316d --- /dev/null +++ b/tests/Sandpit/Exscientia/Process/test_process_wait.py @@ -0,0 +1,55 @@ +from unittest.mock import MagicMock + +import BioSimSpace.Sandpit.Exscientia as BSS +from BioSimSpace.Sandpit.Exscientia.Process._process import Process + + +def test_max_time(): + process = MagicMock() + Process.wait(process, max_time=1) + process._process.wait.assert_called_once_with(60000) + + +def test_None_inactivity_timeout(): + process = MagicMock() + Process.wait(process, max_time=None, inactivity_timeout=None) + process._process.wait.assert_called_once() + + +def test_inactivity_timeout_no_getLastTime(): + process = MagicMock() + process._getLastTime.return_value = None + Process.wait(process, max_time=None, inactivity_timeout=BSS.Units.Time.nanosecond) + process._process.wait.assert_called_once() + + +def test_hang(tmp_path): + process = MagicMock() + process.workDir.return_value = str(tmp_path) + process._name = "test" + # Using TEST_HANG_COUNTER to mimic simulation progress + global TEST_HANG_COUNTER + TEST_HANG_COUNTER = 0 + process.isRunning.return_value = True + + def _getLastTime(): + global TEST_HANG_COUNTER + TEST_HANG_COUNTER += 1 + # Mimic simulation hang after 10 calls + return min(TEST_HANG_COUNTER, 10) + + process._getLastTime = _getLastTime + + def mock_kill(): + # Mock kill to stop the simulation + process.isRunning.return_value = False + + process.kill.side_effect = mock_kill + + Process.wait(process, max_time=None, inactivity_timeout=BSS.Units.Time.nanosecond) + + assert process._process.wait.call_count == 10 + process.kill.assert_called_once() + + with open(f"{tmp_path}/test.out", "r") as f: + assert f.read() == "Process Hung. Killed."