From 4194ffa9994a833359ab750c2c167f09b900f27b Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Wed, 10 Jul 2024 09:51:18 +0100 Subject: [PATCH 01/14] Backport fix from PR #310. [ci skip] --- doc/source/tutorials/protein_mutations.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/tutorials/protein_mutations.rst b/doc/source/tutorials/protein_mutations.rst index 50ed78ce2..b75202552 100644 --- a/doc/source/tutorials/protein_mutations.rst +++ b/doc/source/tutorials/protein_mutations.rst @@ -268,7 +268,7 @@ Apo System Now we are going to focus on the aldose reductase system and set up an alchemical transformation in both apo and holo forms of the protein. The input files (2PDG_8.0) were taken from the SI of a `paper by Aldeghi et. -al `__, +al `__, residue 47 mutated via PyMol (V47I), and standardised via *pdb4amber*. .. code:: ipython3 From 9acedaa8b6810f6d6745556ac9e9c8ceb64ba425 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Wed, 17 Jul 2024 11:20:20 +0100 Subject: [PATCH 02/14] Backport fixes from PR #313. [ci skip] --- python/BioSimSpace/Align/_align.py | 272 +++++++++++------- python/BioSimSpace/Process/_amber.py | 8 +- .../Sandpit/Exscientia/Align/_align.py | 272 +++++++++++------- tests/Align/test_align.py | 22 ++ tests/Sandpit/Exscientia/Align/test_align.py | 25 ++ 5 files changed, 374 insertions(+), 225 deletions(-) diff --git a/python/BioSimSpace/Align/_align.py b/python/BioSimSpace/Align/_align.py index 1db09c51a..90839464f 100644 --- a/python/BioSimSpace/Align/_align.py +++ b/python/BioSimSpace/Align/_align.py @@ -1547,22 +1547,47 @@ def _rmsdAlign(molecule0, molecule1, mapping=None, property_map0={}, property_ma mol0 = molecule0._getSireObject() mol1 = molecule1._getSireObject() + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Convert the mapping to AtomIdx key:value pairs. sire_mapping = _to_sire_mapping(mapping) # Perform the alignment, mol0 to mol1. - try: + if len(mapping) == 1 and is_ion: + idx0 = list(mapping.keys())[0] + idx1 = list(mapping.values())[0] + # Replace the coordinates of the mapped atom with those of the reference. mol0 = ( - mol0.move().align(mol1, _SireMol.AtomResultMatcher(sire_mapping)).molecule() + mol0.edit() + .atom(idx0) + .setProperty( + property_map0.get("coordinates", "coordinates"), + mol1.atom(idx1).property( + property_map1.get("coordinates", "coordinates") + ), + ) + .molecule() + .commit() ) - except Exception as e: - msg = "Failed to align molecules based on mapping: %r" % mapping - if "Could not calculate the single value decomposition" in str(e): - msg += ". Try minimising your molecular coordinates prior to alignment." - if _isVerbose(): - raise _AlignmentError(msg) from e - else: - raise _AlignmentError(msg) from None + else: + try: + mol0 = ( + mol0.move() + .align(mol1, _SireMol.AtomResultMatcher(sire_mapping)) + .molecule() + ) + except Exception as e: + msg = "Failed to align molecules based on mapping: %r" % mapping + if "Could not calculate the single value decomposition" in str(e): + msg += ". Try minimising your molecular coordinates prior to alignment." + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None # Return the aligned molecule. return _Molecule(mol0) @@ -2509,6 +2534,12 @@ def _score_rdkit_mappings( .commit() ) + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Get the set of matching substructures in each molecule. For some reason # setting uniquify to True removes valid matches, in some cases even the # best match! As such, we set uniquify to False and account for duplicate @@ -2575,61 +2606,68 @@ def _score_rdkit_mappings( break if is_valid: - # Rigidly align molecule0 to molecule1 based on the mapping. - if scoring_function == "RMSDALIGN": - try: - molecule0 = ( - molecule0.move() - .align( - molecule1, _SireMol.AtomResultMatcher(sire_mapping) - ) - .molecule() - ) - except Exception as e: - if ( - "Could not calculate the single value decomposition" - in str(e) - ): - is_gsl_error = True - gsl_exception = e - else: - msg = ( - "Failed to align molecules when scoring based on mapping: %r" - % mapping + # If there is only a single atom in the mapping and one molecule + # has one atom, e.g. an ion, then skip the alignment. + if len(mapping) == 1 and is_ion: + mappings.append(mapping) + scores.append(0.0) + else: + # Rigidly align molecule0 to molecule1 based on the mapping. + if scoring_function == "RMSDALIGN": + try: + molecule0 = ( + molecule0.move() + .align( + molecule1, + _SireMol.AtomResultMatcher(sire_mapping), + ) + .molecule() ) - if _isVerbose(): - raise _AlignmentError(msg) from e + except Exception as e: + if ( + "Could not calculate the single value decomposition" + in str(e) + ): + is_gsl_error = True + gsl_exception = e else: - raise _AlignmentError(msg) from None - # Flexibly align molecule0 to molecule1 based on the mapping. - elif scoring_function == "RMSDFLEXALIGN": - molecule0 = flexAlign( - _Molecule(molecule0), - _Molecule(molecule1), - mapping, - property_map0=property_map0, - property_map1=property_map1, - )._sire_object - - # Append the mapping to the list. - mappings.append(mapping) - - # We now compute the RMSD between the coordinates of the matched atoms - # in molecule0 and molecule1. - - # Initialise lists to hold the coordinates. - c0 = [] - c1 = [] - - # Loop over each atom index in the map. - for idx0, idx1 in sire_mapping.items(): - # Append the coordinates of the matched atom in molecule0. - c0.append(molecule0.atom(idx0).property("coordinates")) - # Append the coordinates of atom in molecule1 to which it maps. - c1.append(molecule1.atom(idx1).property("coordinates")) - - # Compute the RMSD between the two sets of coordinates. - scores.append(_SireMaths.getRMSD(c0, c1)) + msg = ( + "Failed to align molecules when scoring based on mapping: %r" + % mapping + ) + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None + # Flexibly align molecule0 to molecule1 based on the mapping. + elif scoring_function == "RMSDFLEXALIGN": + molecule0 = flexAlign( + _Molecule(molecule0), + _Molecule(molecule1), + mapping, + property_map0=property_map0, + property_map1=property_map1, + )._sire_object + + # Append the mapping to the list. + mappings.append(mapping) + + # We now compute the RMSD between the coordinates of the matched atoms + # in molecule0 and molecule1. + + # Initialise lists to hold the coordinates. + c0 = [] + c1 = [] + + # Loop over each atom index in the map. + for idx0, idx1 in sire_mapping.items(): + # Append the coordinates of the matched atom in molecule0. + c0.append(molecule0.atom(idx0).property("coordinates")) + # Append the coordinates of atom in molecule1 to which it maps. + c1.append(molecule1.atom(idx1).property("coordinates")) + + # Compute the RMSD between the two sets of coordinates. + scores.append(_SireMaths.getRMSD(c0, c1)) # No mappings were found. if len(mappings) == 0: @@ -2732,6 +2770,12 @@ def _score_sire_mappings( .commit() ) + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Initialise a list to hold the mappings. mappings = [] @@ -2751,54 +2795,60 @@ def _score_sire_mappings( break if is_valid: - # Rigidly align molecule0 to molecule1 based on the mapping. - if scoring_function == "RMSDALIGN": - try: - molecule0 = ( - molecule0.move() - .align(molecule1, _SireMol.AtomResultMatcher(mapping)) - .molecule() - ) - except Exception as e: - msg = ( - "Failed to align molecules when scoring based on mapping: %r" - % mapping - ) - if _isVerbose(): - raise _AlignmentError(msg) from e - else: - raise _AlignmentError(msg) from None - # Flexibly align molecule0 to molecule1 based on the mapping. - elif scoring_function == "RMSDFLEXALIGN": - molecule0 = flexAlign( - _Molecule(molecule0), - _Molecule(molecule1), - _from_sire_mapping(mapping), - property_map0=property_map0, - property_map1=property_map1, - )._sire_object - - # Append the mapping to the list. - mapping = _from_sire_mapping(mapping) - mapping = dict(sorted(mapping.items())) - mappings.append(mapping) - - # We now compute the RMSD between the coordinates of the matched atoms - # in molecule0 and molecule1. - - # Initialise lists to hold the coordinates. - c0 = [] - c1 = [] - - # Loop over each atom index in the map. - for idx0, idx1 in mapping.items(): - # Append the coordinates of the matched atom in molecule0. - c0.append(molecule0.atom(idx0).property("coordinates")) - # Append the coordinates of atom in molecule1 to which it maps. - c1.append(molecule1.atom(idx1).property("coordinates")) - - # Compute the RMSD between the two sets of coordinates. - scores.append(_SireMaths.getRMSD(c0, c1)) + # If there is only a single atom in the mapping and one molecule + # has one atom, e.g. an ion, then skip the alignment. + if len(mapping) == 1 and is_ion: + mappings.append(mapping) + scores.append(0.0) + else: + # Rigidly align molecule0 to molecule1 based on the mapping. + if scoring_function == "RMSDALIGN": + try: + molecule0 = ( + molecule0.move() + .align(molecule1, _SireMol.AtomResultMatcher(mapping)) + .molecule() + ) + except Exception as e: + msg = ( + "Failed to align molecules when scoring based on mapping: %r" + % mapping + ) + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None + # Flexibly align molecule0 to molecule1 based on the mapping. + elif scoring_function == "RMSDFLEXALIGN": + molecule0 = flexAlign( + _Molecule(molecule0), + _Molecule(molecule1), + _from_sire_mapping(mapping), + property_map0=property_map0, + property_map1=property_map1, + )._sire_object + + # Append the mapping to the list. + mapping = _from_sire_mapping(mapping) + mapping = dict(sorted(mapping.items())) + mappings.append(mapping) + + # We now compute the RMSD between the coordinates of the matched atoms + # in molecule0 and molecule1. + + # Initialise lists to hold the coordinates. + c0 = [] + c1 = [] + + # Loop over each atom index in the map. + for idx0, idx1 in mapping.items(): + # Append the coordinates of the matched atom in molecule0. + c0.append(molecule0.atom(idx0).property("coordinates")) + # Append the coordinates of atom in molecule1 to which it maps. + c1.append(molecule1.atom(idx1).property("coordinates")) + + # Compute the RMSD between the two sets of coordinates. + scores.append(_SireMaths.getRMSD(c0, c1)) # No mappings were found. if len(mappings) == 0: diff --git a/python/BioSimSpace/Process/_amber.py b/python/BioSimSpace/Process/_amber.py index 8140729a4..f9b5e21ca 100644 --- a/python/BioSimSpace/Process/_amber.py +++ b/python/BioSimSpace/Process/_amber.py @@ -2856,9 +2856,11 @@ def is_exe(fpath): # order their path accordingly, or use the exe keyword argument. if results: for exe in results: - exe = _pathlib.Path(p) / exe - if is_exe(exe): - return str(exe) + # Exclude "locally enhanced sampling" executables. + if "LES" not in exe: + exe = _pathlib.Path(p) / exe + if is_exe(exe): + return str(exe) msg = ( "'BioSimSpace.Process.Amber' is not supported. " diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py index 27731e95d..6b3522c39 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py @@ -1151,19 +1151,44 @@ def rmsdAlign(molecule0, molecule1, mapping=None, property_map0={}, property_map # Convert the mapping to AtomIdx key:value pairs. sire_mapping = _to_sire_mapping(mapping) + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Perform the alignment, mol0 to mol1. - try: + if len(mapping) == 1 and is_ion: + idx0 = list(mapping.keys())[0] + idx1 = list(mapping.values())[0] + # Replace the coordinates of the mapped atom with those of the reference. mol0 = ( - mol0.move().align(mol1, _SireMol.AtomResultMatcher(sire_mapping)).molecule() + mol0.edit() + .atom(idx0) + .setProperty( + property_map0.get("coordinates", "coordinates"), + mol1.atom(idx1).property( + property_map1.get("coordinates", "coordinates") + ), + ) + .molecule() + .commit() ) - except Exception as e: - msg = "Failed to align molecules based on mapping: %r" % mapping - if "Could not calculate the single value decomposition" in str(e): - msg += ". Try minimising your molecular coordinates prior to alignment." - if _isVerbose(): - raise _AlignmentError(msg) from e - else: - raise _AlignmentError(msg) from None + else: + try: + mol0 = ( + mol0.move() + .align(mol1, _SireMol.AtomResultMatcher(sire_mapping)) + .molecule() + ) + except Exception as e: + msg = "Failed to align molecules based on mapping: %r" % mapping + if "Could not calculate the single value decomposition" in str(e): + msg += ". Try minimising your molecular coordinates prior to alignment." + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None # Return the aligned molecule. return _Molecule(mol0) @@ -1760,6 +1785,12 @@ def _score_rdkit_mappings( .commit() ) + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Get the set of matching substructures in each molecule. For some reason # setting uniquify to True removes valid matches, in some cases even the # best match! As such, we set uniquify to False and account for duplicate @@ -1826,61 +1857,68 @@ def _score_rdkit_mappings( break if is_valid: - # Rigidly align molecule0 to molecule1 based on the mapping. - if scoring_function == "RMSDALIGN": - try: - molecule0 = ( - molecule0.move() - .align( - molecule1, _SireMol.AtomResultMatcher(sire_mapping) - ) - .molecule() - ) - except Exception as e: - if ( - "Could not calculate the single value decomposition" - in str(e) - ): - is_gsl_error = True - gsl_exception = e - else: - msg = ( - "Failed to align molecules when scoring based on mapping: %r" - % mapping + # If there is only a single atom in the mapping and one molecule + # has one atom, e.g. an ion, then skip the alignment. + if len(mapping) == 1 and is_ion: + mappings.append(mapping) + scores.append(0.0) + else: + # Rigidly align molecule0 to molecule1 based on the mapping. + if scoring_function == "RMSDALIGN": + try: + molecule0 = ( + molecule0.move() + .align( + molecule1, + _SireMol.AtomResultMatcher(sire_mapping), + ) + .molecule() ) - if _isVerbose(): - raise _AlignmentError(msg) from e + except Exception as e: + if ( + "Could not calculate the single value decomposition" + in str(e) + ): + is_gsl_error = True + gsl_exception = e else: - raise _AlignmentError(msg) from None - # Flexibly align molecule0 to molecule1 based on the mapping. - elif scoring_function == "RMSDFLEXALIGN": - molecule0 = flexAlign( - _Molecule(molecule0), - _Molecule(molecule1), - mapping, - property_map0=property_map0, - property_map1=property_map1, - )._sire_object - - # Append the mapping to the list. - mappings.append(mapping) - - # We now compute the RMSD between the coordinates of the matched atoms - # in molecule0 and molecule1. - - # Initialise lists to hold the coordinates. - c0 = [] - c1 = [] - - # Loop over each atom index in the map. - for idx0, idx1 in sire_mapping.items(): - # Append the coordinates of the matched atom in molecule0. - c0.append(molecule0.atom(idx0).property("coordinates")) - # Append the coordinates of atom in molecule1 to which it maps. - c1.append(molecule1.atom(idx1).property("coordinates")) - - # Compute the RMSD between the two sets of coordinates. - scores.append(_SireMaths.getRMSD(c0, c1)) + msg = ( + "Failed to align molecules when scoring based on mapping: %r" + % mapping + ) + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None + # Flexibly align molecule0 to molecule1 based on the mapping. + elif scoring_function == "RMSDFLEXALIGN": + molecule0 = flexAlign( + _Molecule(molecule0), + _Molecule(molecule1), + mapping, + property_map0=property_map0, + property_map1=property_map1, + )._sire_object + + # Append the mapping to the list. + mappings.append(mapping) + + # We now compute the RMSD between the coordinates of the matched atoms + # in molecule0 and molecule1. + + # Initialise lists to hold the coordinates. + c0 = [] + c1 = [] + + # Loop over each atom index in the map. + for idx0, idx1 in sire_mapping.items(): + # Append the coordinates of the matched atom in molecule0. + c0.append(molecule0.atom(idx0).property("coordinates")) + # Append the coordinates of atom in molecule1 to which it maps. + c1.append(molecule1.atom(idx1).property("coordinates")) + + # Compute the RMSD between the two sets of coordinates. + scores.append(_SireMaths.getRMSD(c0, c1)) # No mappings were found. if len(mappings) == 0: @@ -1983,6 +2021,12 @@ def _score_sire_mappings( .commit() ) + # Do we have a monatomic ion? + if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): + is_ion = True + else: + is_ion = False + # Initialise a list to hold the mappings. mappings = [] @@ -2002,54 +2046,60 @@ def _score_sire_mappings( break if is_valid: - # Rigidly align molecule0 to molecule1 based on the mapping. - if scoring_function == "RMSDALIGN": - try: - molecule0 = ( - molecule0.move() - .align(molecule1, _SireMol.AtomResultMatcher(mapping)) - .molecule() - ) - except Exception as e: - msg = ( - "Failed to align molecules when scoring based on mapping: %r" - % mapping - ) - if _isVerbose(): - raise _AlignmentError(msg) from e - else: - raise _AlignmentError(msg) from None - # Flexibly align molecule0 to molecule1 based on the mapping. - elif scoring_function == "RMSDFLEXALIGN": - molecule0 = flexAlign( - _Molecule(molecule0), - _Molecule(molecule1), - _from_sire_mapping(mapping), - property_map0=property_map0, - property_map1=property_map1, - )._sire_object - - # Append the mapping to the list. - mapping = _from_sire_mapping(mapping) - mapping = dict(sorted(mapping.items())) - mappings.append(mapping) - - # We now compute the RMSD between the coordinates of the matched atoms - # in molecule0 and molecule1. - - # Initialise lists to hold the coordinates. - c0 = [] - c1 = [] - - # Loop over each atom index in the map. - for idx0, idx1 in mapping.items(): - # Append the coordinates of the matched atom in molecule0. - c0.append(molecule0.atom(idx0).property("coordinates")) - # Append the coordinates of atom in molecule1 to which it maps. - c1.append(molecule1.atom(idx1).property("coordinates")) - - # Compute the RMSD between the two sets of coordinates. - scores.append(_SireMaths.getRMSD(c0, c1)) + # If there is only a single atom in the mapping and one molecule + # has one atom, e.g. an ion, then skip the alignment. + if len(mapping) == 1 and is_ion: + mappings.append(mapping) + scores.append(0.0) + else: + # Rigidly align molecule0 to molecule1 based on the mapping. + if scoring_function == "RMSDALIGN": + try: + molecule0 = ( + molecule0.move() + .align(molecule1, _SireMol.AtomResultMatcher(mapping)) + .molecule() + ) + except Exception as e: + msg = ( + "Failed to align molecules when scoring based on mapping: %r" + % mapping + ) + if _isVerbose(): + raise _AlignmentError(msg) from e + else: + raise _AlignmentError(msg) from None + # Flexibly align molecule0 to molecule1 based on the mapping. + elif scoring_function == "RMSDFLEXALIGN": + molecule0 = flexAlign( + _Molecule(molecule0), + _Molecule(molecule1), + _from_sire_mapping(mapping), + property_map0=property_map0, + property_map1=property_map1, + )._sire_object + + # Append the mapping to the list. + mapping = _from_sire_mapping(mapping) + mapping = dict(sorted(mapping.items())) + mappings.append(mapping) + + # We now compute the RMSD between the coordinates of the matched atoms + # in molecule0 and molecule1. + + # Initialise lists to hold the coordinates. + c0 = [] + c1 = [] + + # Loop over each atom index in the map. + for idx0, idx1 in mapping.items(): + # Append the coordinates of the matched atom in molecule0. + c0.append(molecule0.atom(idx0).property("coordinates")) + # Append the coordinates of atom in molecule1 to which it maps. + c1.append(molecule1.atom(idx1).property("coordinates")) + + # Compute the RMSD between the two sets of coordinates. + scores.append(_SireMaths.getRMSD(c0, c1)) # No mappings were found. if len(mappings) == 0: diff --git a/tests/Align/test_align.py b/tests/Align/test_align.py index 341e4bf4f..44c53a74c 100644 --- a/tests/Align/test_align.py +++ b/tests/Align/test_align.py @@ -673,3 +673,25 @@ def test_roi_merge(protein_inputs): merged = BSS.Align.merge(aligned_p0, p1, protein_mapping, roi=roi) merged_system = merged.toSystem() assert merged_system.nPerturbableMolecules() == 1 + + +def test_ion_merge(system): + from sire.legacy.IO import createSodiumIon + + # Extract a water molecule. + water = system[-1] + + # Create a sodium ion using the water coordinates. + ion = createSodiumIon( + water.getAtoms()[0]._sire_object.property("coordinates"), "tip3p" + ) + + # Merge the water and ion. + merged = BSS.Align.merge(water, BSS._SireWrappers.Molecule(ion)) + + # Make sure the ion has the coordintes of the oxygen atom. + coords0 = merged._sire_object.property("coordinates0").toVector()[0] + coords1 = merged._sire_object.property("coordinates1").toVector()[0] + water_coords = water._sire_object.property("coordinates").toVector()[0] + assert coords0 == coords1 + assert coords0 == water_coords diff --git a/tests/Sandpit/Exscientia/Align/test_align.py b/tests/Sandpit/Exscientia/Align/test_align.py index d493b1624..12d8e68a4 100644 --- a/tests/Sandpit/Exscientia/Align/test_align.py +++ b/tests/Sandpit/Exscientia/Align/test_align.py @@ -725,3 +725,28 @@ def test_hydrogen_mass_repartitioning(): assert mass0 == masses1[idx] for idx, mass1 in dummy_masses1: assert mass1 == masses0[idx] + + +def test_ion_merge(): + from sire.legacy.IO import createSodiumIon + from tests.conftest import root_fp + + # Extract a water molecule from the system. + water = BSS.IO.readMolecules( + [f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"] + )[-1] + + # Create a sodium ion using the water coordinates. + ion = createSodiumIon( + water.getAtoms()[0]._sire_object.property("coordinates"), "tip3p" + ) + + # Merge the water and ion. + merged = BSS.Align.merge(water, BSS._SireWrappers.Molecule(ion)) + + # Make sure the ion has the coordintes of the oxygen atom. + coords0 = merged._sire_object.property("coordinates0").toVector()[0] + coords1 = merged._sire_object.property("coordinates1").toVector()[0] + water_coords = water._sire_object.property("coordinates").toVector()[0] + assert coords0 == coords1 + assert coords0 == water_coords From 15f19a660d2c76e932a07733b40e7aff85bec8ea Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Mon, 29 Jul 2024 10:01:46 +0100 Subject: [PATCH 03/14] Backport fix from PR #319. [ci skip] --- python/BioSimSpace/Process/_somd.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/python/BioSimSpace/Process/_somd.py b/python/BioSimSpace/Process/_somd.py index 2d1c5e508..f51d56774 100644 --- a/python/BioSimSpace/Process/_somd.py +++ b/python/BioSimSpace/Process/_somd.py @@ -78,6 +78,7 @@ def __init__( seed=None, extra_options={}, extra_lines=[], + extra_args={}, property_map={}, **kwargs, ): @@ -118,6 +119,9 @@ def __init__( extra_lines : [str] A list of extra lines to put at the end of the configuration file. + extra_args : dict + A dictionary of extra command-line arguments to pass to the AMBER executable. + property_map : dict A dictionary that maps system "properties" to their user defined values. This allows the user to refer to properties with their @@ -136,6 +140,7 @@ def __init__( seed=seed, extra_options=extra_options, extra_lines=extra_lines, + extra_args=extra_args, property_map=property_map, ) @@ -478,6 +483,10 @@ def _generate_args(self): self.setArg("-C", "%s.cfg" % self._name) # Config file. self.setArg("-p", self._platform) # Simulation platform. + # Add the extra arguments. + for key, value in self._extra_args.items(): + self.setArg(key, value) + def start(self): """ Start the SOMD process. From 66bc16cd5c26afa26f239fb52df2dfaa3275c322 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Mon, 29 Jul 2024 13:19:49 +0100 Subject: [PATCH 04/14] Typo. [ci skip] --- python/BioSimSpace/Process/_somd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/BioSimSpace/Process/_somd.py b/python/BioSimSpace/Process/_somd.py index f51d56774..fb8063e15 100644 --- a/python/BioSimSpace/Process/_somd.py +++ b/python/BioSimSpace/Process/_somd.py @@ -120,7 +120,7 @@ def __init__( A list of extra lines to put at the end of the configuration file. extra_args : dict - A dictionary of extra command-line arguments to pass to the AMBER executable. + A dictionary of extra command-line arguments to pass to the SOMD executable. property_map : dict A dictionary that maps system "properties" to their user defined From 9831a3f3711b9b6c81d4f748dc33f4ec5a6f7d39 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Tue, 30 Jul 2024 17:44:52 +0100 Subject: [PATCH 05/14] Backport fix from PR #323. [ci skip] --- python/BioSimSpace/FreeEnergy/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/BioSimSpace/FreeEnergy/_utils.py b/python/BioSimSpace/FreeEnergy/_utils.py index c4c3a61b7..540be2bf5 100644 --- a/python/BioSimSpace/FreeEnergy/_utils.py +++ b/python/BioSimSpace/FreeEnergy/_utils.py @@ -38,4 +38,4 @@ def engines(): engines : [str] The list of supported engines. """ - return ["SOMD", "GROMACS"] + return ["AMBER", "SOMD", "GROMACS"] From 55a3a09f0f926433e06895180ee6439cb7eac9dc Mon Sep 17 00:00:00 2001 From: zhiyi wu Date: Fri, 16 Aug 2024 15:27:44 +0100 Subject: [PATCH 06/14] use the new sire --- .github/workflows/Sandpit_exs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/Sandpit_exs.yml b/.github/workflows/Sandpit_exs.yml index 50160fe36..b75de18e4 100644 --- a/.github/workflows/Sandpit_exs.yml +++ b/.github/workflows/Sandpit_exs.yml @@ -33,7 +33,7 @@ jobs: - name: Install dependency run: | - conda install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2024.1.0" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm + conda install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2024.2.0" "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 From 39a880adbac48f2122edfc96a2209b740f368362 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Fri, 16 Aug 2024 16:06:47 +0100 Subject: [PATCH 07/14] Remove test that requires next Sire version. --- tests/Align/test_align.py | 22 ----------------- tests/Sandpit/Exscientia/Align/test_align.py | 25 -------------------- 2 files changed, 47 deletions(-) diff --git a/tests/Align/test_align.py b/tests/Align/test_align.py index 44c53a74c..341e4bf4f 100644 --- a/tests/Align/test_align.py +++ b/tests/Align/test_align.py @@ -673,25 +673,3 @@ def test_roi_merge(protein_inputs): merged = BSS.Align.merge(aligned_p0, p1, protein_mapping, roi=roi) merged_system = merged.toSystem() assert merged_system.nPerturbableMolecules() == 1 - - -def test_ion_merge(system): - from sire.legacy.IO import createSodiumIon - - # Extract a water molecule. - water = system[-1] - - # Create a sodium ion using the water coordinates. - ion = createSodiumIon( - water.getAtoms()[0]._sire_object.property("coordinates"), "tip3p" - ) - - # Merge the water and ion. - merged = BSS.Align.merge(water, BSS._SireWrappers.Molecule(ion)) - - # Make sure the ion has the coordintes of the oxygen atom. - coords0 = merged._sire_object.property("coordinates0").toVector()[0] - coords1 = merged._sire_object.property("coordinates1").toVector()[0] - water_coords = water._sire_object.property("coordinates").toVector()[0] - assert coords0 == coords1 - assert coords0 == water_coords diff --git a/tests/Sandpit/Exscientia/Align/test_align.py b/tests/Sandpit/Exscientia/Align/test_align.py index 12d8e68a4..d493b1624 100644 --- a/tests/Sandpit/Exscientia/Align/test_align.py +++ b/tests/Sandpit/Exscientia/Align/test_align.py @@ -725,28 +725,3 @@ def test_hydrogen_mass_repartitioning(): assert mass0 == masses1[idx] for idx, mass1 in dummy_masses1: assert mass1 == masses0[idx] - - -def test_ion_merge(): - from sire.legacy.IO import createSodiumIon - from tests.conftest import root_fp - - # Extract a water molecule from the system. - water = BSS.IO.readMolecules( - [f"{root_fp}/input/ala.crd", f"{root_fp}/input/ala.top"] - )[-1] - - # Create a sodium ion using the water coordinates. - ion = createSodiumIon( - water.getAtoms()[0]._sire_object.property("coordinates"), "tip3p" - ) - - # Merge the water and ion. - merged = BSS.Align.merge(water, BSS._SireWrappers.Molecule(ion)) - - # Make sure the ion has the coordintes of the oxygen atom. - coords0 = merged._sire_object.property("coordinates0").toVector()[0] - coords1 = merged._sire_object.property("coordinates1").toVector()[0] - water_coords = water._sire_object.property("coordinates").toVector()[0] - assert coords0 == coords1 - assert coords0 == water_coords From f28b9b57512a0ace45c7492f374d2184e63ed4e2 Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon <36005076+msuruzhon@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:44:09 +0100 Subject: [PATCH 08/14] Fixing a box check in GMX (#54) --- python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index b6e02992f..52a17da82 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -355,7 +355,13 @@ def _write_system(self, system, coord_file=None, topol_file=None, ref_file=None) # 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 "space" in 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 = 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 From d3701682d2c31b0febc83c470868e33d33f6ec66 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Tue, 27 Aug 2024 09:37:29 +0100 Subject: [PATCH 09/14] Exclude lipids from heavy atom restraint for Gromacs Process (#55) --- .../Sandpit/Exscientia/Process/_gromacs.py | 8 +++-- .../Exscientia/_SireWrappers/_molecule.py | 16 ++++++++++ .../Process/test_position_restraint.py | 31 +++++++++++++++++++ .../Exscientia/_SireWrappers/test_molecule.py | 13 ++++++++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index 52a17da82..f3f847588 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -2180,15 +2180,17 @@ def _add_position_restraints(self, config_options): for idx, mol_idx in enumerate(mol_idxs): # Get the indices of any restrained atoms in this molecule, # making sure that indices are relative to the molecule. - if restraint is not None: + if restraint is None: + atom_idxs = [] + elif self._system.getMolecule(mol_idx).isLipid(): + atom_idxs = [] + else: atom_idxs = self._system.getRestraintAtoms( restraint, mol_index=mol_idx, is_absolute=False, allow_zero_matches=True, ) - else: - atom_idxs = [] if self._system.getMolecule(mol_idx).isAlchemicalIon(): alch_ion = self._system.getMolecule(mol_idx).getAtoms() diff --git a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py index 6c9012c23..a9b270794 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py +++ b/python/BioSimSpace/Sandpit/Exscientia/_SireWrappers/_molecule.py @@ -518,6 +518,22 @@ def isDecoupled(self): else: return False + def isLipid(self): + """ + Whether this molecule is decoupled, i.e. it can be used in a + free-energy decoupling simulation. + + Returns + ------- + + is_decoupled : bool + Whether the molecule is decoupled. + """ + if self._sire_object.hasProperty("lipid"): + return True + else: + return False + def isML(self): """ Whether this molecule is marked as ML molecule, i.e. it can be used in a diff --git a/tests/Sandpit/Exscientia/Process/test_position_restraint.py b/tests/Sandpit/Exscientia/Process/test_position_restraint.py index 542b71c54..8fc68cd55 100644 --- a/tests/Sandpit/Exscientia/Process/test_position_restraint.py +++ b/tests/Sandpit/Exscientia/Process/test_position_restraint.py @@ -167,6 +167,37 @@ def test_gromacs(protocol, system, ref_system, tmp_path): assert len(diff) +@pytest.mark.skipif( + has_gromacs is False or has_openff is False, + reason="Requires GROMACS and openff to be installed", +) +@pytest.mark.parametrize("lipid", [True, False]) +def test_gromacs_lipid(system, tmp_path, lipid): + protocol = BSS.Protocol.Minimisation(restraint="heavy") + if lipid: + molecule = system.getMolecule(0) + sire_obj = molecule._sire_object + c = sire_obj.cursor() + c["lipid"] = True + molecule._sire_object = c.commit() + system.updateMolecule(0, molecule) + BSS.Process.Gromacs( + system, + protocol, + reference_system=system, + work_dir=str(tmp_path), + ignore_warnings=True, + ) + if lipid is False: + assert (tmp_path / "posre_0001.itp").is_file() + with open(tmp_path / "gromacs.top", "r") as f: + assert "posre_0001.itp" in f.read() + else: + assert not (tmp_path / "posre_0001.itp").is_file() + with open(tmp_path / "gromacs.top", "r") as f: + assert not "posre_0001.itp" in f.read() + + @pytest.mark.skipif( has_amber is False or has_openff is False, reason="Requires AMBER and openff to be installed", diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py index 7f6bc0aa3..47bbdc69d 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py @@ -126,3 +126,16 @@ def test_extract(system): # Make sure the numbers are different. assert partial_mol.number() != mol.number() + + +@pytest.mark.parametrize("lipid", [True, False]) +def test_lipid(lipid): + ff = "openff_unconstrained-2.0.0" + mol = BSS.Parameters.parameterise("c1ccccc1C", ff).getMolecule() + if lipid: + sire_obj = mol._sire_object + c = sire_obj.cursor() + c["lipid"] = lipid + mol._sire_object = c.commit() + + assert mol.isLipid() is lipid From 44ffd5904f9560f4a85d36284646204bac740c41 Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Thu, 5 Sep 2024 11:26:59 +0100 Subject: [PATCH 10/14] Fix the bug where AMBER Process will not use report interval (#56) --- python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py b/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py index 3daebf40d..bda35fb8d 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py @@ -189,7 +189,7 @@ def generateAmberConfig(self, extra_options=None, extra_lines=None): # Define some miscellaneous defaults. protocol_dict = { - "ntpr": 200, # Interval between reporting energies. + "ntpr": self._report_interval, # Interval between reporting energies. "ntwr": self._restart_interval, # Interval between saving restart files. "ntwx": self._restart_interval, # Trajectory sampling frequency. "ntxo": 2, # Output coordinates as NetCDF. From dc96c2f2fb3a2728909ba0f9c849142b42d132cd Mon Sep 17 00:00:00 2001 From: Miroslav Suruzhon <36005076+msuruzhon@users.noreply.github.com> Date: Thu, 12 Sep 2024 10:05:47 +0100 Subject: [PATCH 11/14] Fixing vacuum translations in GROMACS (#57) --- .../Sandpit/Exscientia/Process/_gromacs.py | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py index f3f847588..dd0ddd97c 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py @@ -47,6 +47,7 @@ from sire.legacy import Units as _SireUnits from sire.legacy import Vol as _SireVol +from ..Units.Length import angstrom from .._Utils import _assert_imported, _have_imported, _try_import # alchemlyb isn't available on all variants of Python that we support, so we @@ -2704,8 +2705,22 @@ def _getFinalFrame(self): ) # If this is a vacuum simulation, then translate the centre of mass - # of the system back to the origin. - if not space_prop in old_system._sire_object.propertyKeys(): + # of the system back to the middle of the box to preserve PBC. + if old_system.getBox() == (None, None): + try: + old_box = old_system._sire_object.property(space_prop) + except: + old_box = None + box = _SireVol.PeriodicBox(_SireMaths.Vector(9999, 9999, 9999)) + old_system._sire_object.setProperty(space_prop, box) + com = [angstrom * 9999 / 2 for _ in range(3)] + old_system.translate([x for x in com]) + old_system._sire_object.make_whole() + old_system.translate([-x for x in com]) + if old_box is None: + old_system._sire_object.removeProperty(space_prop) + else: + old_system._sire_object.setProperty(space_prop, old_box) com = old_system._getCenterOfMass() old_system.translate([-x for x in com]) @@ -2821,9 +2836,23 @@ def _getFrame(self, time): ) # If this is a vacuum simulation, then translate the centre of mass - # of the system back to the origin. - if not space_prop in old_system._sire_object.propertyKeys(): - com = new_system._getCenterOfMass() + # of the system back to the middle of the box to preserve PBC. + if old_system.getBox() == (None, None): + try: + old_box = old_system._sire_object.property(space_prop) + except: + old_box = None + box = _SireVol.PeriodicBox(_SireMaths.Vector(9999, 9999, 9999)) + old_system._sire_object.setProperty(space_prop, box) + com = [angstrom * 9999 / 2 for _ in range(3)] + old_system.translate([x for x in com]) + old_system._sire_object.make_whole() + old_system.translate([-x for x in com]) + if old_box is None: + old_system._sire_object.removeProperty(space_prop) + else: + old_system._sire_object.setProperty(space_prop, old_box) + com = old_system._getCenterOfMass() old_system.translate([-x for x in com]) return old_system From 58dba1c886664bb05c94113b367d3ac218032c95 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 24 Oct 2024 12:03:48 +0100 Subject: [PATCH 12/14] Guard test against missing OpenFF package. --- tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py index 47bbdc69d..af2a8bdb3 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py @@ -2,7 +2,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS -from tests.Sandpit.Exscientia.conftest import url, has_amber, has_pyarrow +from tests.Sandpit.Exscientia.conftest import url, has_amber, has_openff, has_pyarrow from tests.conftest import root_fp @@ -129,6 +129,10 @@ def test_extract(system): @pytest.mark.parametrize("lipid", [True, False]) +@pytest.mark.skipif( + has_openff is False, + reason="Requires OpenFF to be installed.", +) def test_lipid(lipid): ff = "openff_unconstrained-2.0.0" mol = BSS.Parameters.parameterise("c1ccccc1C", ff).getMolecule() From e33f4993df6cea2a7a404c4a72f3f11b1820b673 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 24 Oct 2024 12:08:16 +0100 Subject: [PATCH 13/14] Remove redundant variable. --- python/BioSimSpace/Align/_align.py | 18 ------------------ .../Sandpit/Exscientia/Align/_align.py | 18 ------------------ 2 files changed, 36 deletions(-) diff --git a/python/BioSimSpace/Align/_align.py b/python/BioSimSpace/Align/_align.py index bf5d94792..bdbe025cb 100644 --- a/python/BioSimSpace/Align/_align.py +++ b/python/BioSimSpace/Align/_align.py @@ -1547,12 +1547,6 @@ def _rmsdAlign(molecule0, molecule1, mapping=None, property_map0={}, property_ma mol0 = molecule0._getSireObject() mol1 = molecule1._getSireObject() - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Convert the mapping to AtomIdx key:value pairs. sire_mapping = _to_sire_mapping(mapping) @@ -2534,12 +2528,6 @@ def _score_rdkit_mappings( .commit() ) - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Get the set of matching substructures in each molecule. For some reason # setting uniquify to True removes valid matches, in some cases even the # best match! As such, we set uniquify to False and account for duplicate @@ -2770,12 +2758,6 @@ def _score_sire_mappings( .commit() ) - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Initialise a list to hold the mappings. mappings = [] diff --git a/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py b/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py index e8ef52d5b..27b0d373c 100644 --- a/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py +++ b/python/BioSimSpace/Sandpit/Exscientia/Align/_align.py @@ -1151,12 +1151,6 @@ def rmsdAlign(molecule0, molecule1, mapping=None, property_map0={}, property_map # Convert the mapping to AtomIdx key:value pairs. sire_mapping = _to_sire_mapping(mapping) - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Perform the alignment, mol0 to mol1. if len(mapping) == 1: idx0 = list(mapping.keys())[0] @@ -1785,12 +1779,6 @@ def _score_rdkit_mappings( .commit() ) - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Get the set of matching substructures in each molecule. For some reason # setting uniquify to True removes valid matches, in some cases even the # best match! As such, we set uniquify to False and account for duplicate @@ -2021,12 +2009,6 @@ def _score_sire_mappings( .commit() ) - # Do we have a monatomic ion? - if (molecule0.nAtoms() == 1) or (molecule1.nAtoms() == 1): - is_ion = True - else: - is_ion = False - # Initialise a list to hold the mappings. mappings = [] From 0365150da516ea1da075c3f1c58264e46d79f9d1 Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Thu, 24 Oct 2024 13:36:16 +0100 Subject: [PATCH 14/14] Test requires AMBER and OpenFF. [ci skip] --- tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py index af2a8bdb3..c9c9ea60b 100644 --- a/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py +++ b/tests/Sandpit/Exscientia/_SireWrappers/test_molecule.py @@ -130,8 +130,8 @@ def test_extract(system): @pytest.mark.parametrize("lipid", [True, False]) @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_lipid(lipid): ff = "openff_unconstrained-2.0.0"