Fix 116

.gitignore

@@ -65,3 +65,4 @@ tests/cache/*
 
 .DS_Store
 
+.coverage

doc/source/changelog.rst

@@ -12,9 +12,29 @@ Development was migrated into the
 `OpenBioSim <https://github.com/openbiosim>`__
 organisation on `GitHub <https://github.com/openbiosim/sire>`__.
 
-`2023.4.0 <https://github.com/openbiosim/sire/compare/2023.4.0...2023.5.0>`__ - December 2023
+`2023.5.0 <https://github.com/openbiosim/sire/compare/2023.4.0...2023.5.0>`__ - December 2023
 ---------------------------------------------------------------------------------------------
 
+* Fixed regression introduced in 2023.4.0 that meant that removed the constraints
+  from water molecules that had no internal bonds. These waters would blow up
+  as there was nothing holding them together. The need for these constraints is
+  now better detected and explicitly added.
+
+* Significantly sped up the OpenMM layer by checking for similar constraint lengths
+  and matching them all to be the same (within 0.05 A for calculated constraints,
+  e.g. unbonded atoms or angle constraints) or to R0 for bonds where the bond
+  length is within 0.1 A of R0 and the molecule isn't perturbable.
+
+* Added a custom minimiser that is based on OpenMM's LocalEnergyMinimizer,
+  but that copes better with exclusion errors, and that has deep integration
+  with the progress bar / interuption system.
+
+* Fixed a bug where the exclusions and exceptions were mismatched for the
+  OpenMM CPU platform, leading to exclusion errors.
+
+* Fixed an issue where the vacuum dynamics and minimisation simulations still
+  had a spurious periodic box added when ``.commit()`` was called.
+
 * Please add an item to this changelog when you create your PR
 
 `2023.4.0 <https://github.com/openbiosim/sire/compare/2023.3.0...2023.4.0>`__ - October 2023

doc/source/tutorial/part06/06_faster_fep.rst

@@ -323,7 +323,9 @@ ethane and methanol.
 ...     lambda_value = l / 100.0
 ...     print(f"Simulating lambda={lambda_value:.2f}")
 ...     # minimise the system at this lambda value
-...     min_mols = mols.minimisation(lambda_value=lambda_value).run().commit()
+...     min_mols = mols.minimisation(lambda_value=lambda_value,
+...                                  constraint="h-bonds",
+...                                  perturbable_constraint="none").run().commit()
 ...     # create a dynamics object for the system
 ...     d = min_mols.dynamics(timestep="4fs", temperature="25oC",
 ...                           lambda_value=lambda_value,

doc/source/tutorial/part06/scripts/run_md.py

@@ -25,7 +25,9 @@
 for i, lambda_value in enumerate(lambda_values):
     print(f"Simulating lambda={lambda_value:.2f}")
     # minimise the system at this lambda value
-    min_mols = mols.minimisation(lambda_value=lambda_value).run().commit()
+    min_mols = mols.minimisation(lambda_value=lambda_value,
+                                 constraint=constraint,
+                                 perturbable_constraint="none").run().commit()
 
     # create a dynamics object for the system
     d = min_mols.dynamics(
@@ -68,10 +70,14 @@
 
 for i, lambda_value in enumerate(lambda_values):
     print(f"Simulating lambda={lambda_value:.2f}")
-    # minimise the system at this lambda value
+    # minimise the system at this lambda value using the
+    # same constraints as the dynamics (but switching
+    # off the perturbable constraint)
     min_mols = (
         mols[0]
-        .minimisation(lambda_value=lambda_value, vacuum=True)
+        .minimisation(lambda_value=lambda_value, vacuum=True,
+                      constraint=constraint,
+                      perturbable_constraint="none")
         .run()
         .commit()
     )

src/sire/__init__.py

@@ -90,7 +90,7 @@ def _fix_openmm_path():
     if need_path:
         # importing openmm should add this path
         try:
-            import openmm
+            import openmm  # noqa: F401 (imported but unused)
         except Exception:
             print("OpenMM import failed!")
             # Copy the files

src/sire/_load.py

@@ -293,7 +293,8 @@ def expand(base: str, path: _Union[str, _List[str]], *args, **kwargs):
 
      Examples:
          >>> expand("https://sire.openbiosim.org/m", "urea.gro", "urea.top")
-         ["https://sire.openbiosim.org/m/urea.gro", "https://sire.openbiosim.org/n/urea.top"]
+         ["https://sire.openbiosim.org/m/urea.gro",
+          "https://sire.openbiosim.org/n/urea.top"]
 
          >>> expand("input", ["ala.top", "ala.crd"])
          ["input/ala.top", "input/ala.crd"]
@@ -438,8 +439,6 @@ def load(
     for key in kwargs.keys():
         m[key] = kwargs[key]
 
-    from .base import create_map
-
     map = create_map(map, m)
 
     return load_molecules(paths, map=create_map(map))

src/sire/_pythonize.py

@@ -177,7 +177,7 @@ def _load_new_api_modules(delete_old: bool = True, is_base: bool = False):
     _is_in_loading_process = True
 
     # call Pythonize on all of the new modules
-    from .legacy import (
+    from .legacy import (  # noqa: F401
         Base,
         Move,
         IO,
@@ -189,7 +189,8 @@ def _load_new_api_modules(delete_old: bool = True, is_base: bool = False):
         Analysis,
         CAS,
         Cluster,
-        Convert,  # does not need pythonizing, but importing will make it visible
+        # doesn't need pythonizing but importing will make it visible
+        Convert,
         Error,
         ID,
         Maths,

src/sire/analysis/__init__.py

@@ -1,6 +1,7 @@
 __all__ = []
 
-from ..legacy import Analysis as _Analysis
+# Need to import the module to pythonize it
+from ..legacy import Analysis as _Analysis  # noqa: F401
 
 from .. import use_new_api as _use_new_api
 

src/sire/base/__init__.py

@@ -1,12 +1,12 @@
-__all__ = ["create_map", "wrap", "PropertyMap"]
+__all__ = ["create_map", "wrap", "PropertyMap", "ProgressBar"]
 
 from ..legacy import Base as _Base
 
 from .. import use_new_api as _use_new_api
 
 from ..legacy.Base import PropertyMap
 
-from ._progressbar import *
+from ._progressbar import ProgressBar
 
 _use_new_api(is_base=True)
 

src/sire/cluster/__init__.py

@@ -1,6 +1,7 @@
 __all__ = []
 
-from ..legacy import Cluster as _Cluster
+# Need to import so it is pythonized
+from ..legacy import Cluster as _Cluster  # noqa: F401
 
 from .. import use_new_api as _use_new_api
 

src/sire/convert/__init__.py

@@ -143,7 +143,7 @@ def _to_type(o):
         else:
             raise TypeError(f"Unrecognised type {typ[0]}")
 
-        if type(c) != System:
+        if not isinstance(c, System):
             c = c.molecules()
 
         converted.append(c)
@@ -157,7 +157,7 @@ def _to_type(o):
         for i in range(1, len(converted)):
             mols += converted[i]
 
-    if type(mols) == System:
+    if isinstance(mols, System):
         return mols
     elif len(mols) == 1:
         return mols[0]
@@ -286,7 +286,7 @@ def sire_to_biosimspace(obj, map=None):
 
     from ..system import System
 
-    if type(obj) == System:
+    if isinstance(obj, System):
         return _BSS._SireWrappers.System(obj._system)
 
     obj = _to_selectormol(obj)

src/sire/ff/__init__.py

@@ -1,6 +1,7 @@
 __all__ = []
 
-from ..legacy import FF as _FF
+# Need to import so the module is pythonized
+from ..legacy import FF as _FF  # noqa: F401
 
 from .. import use_new_api as _use_new_api
 

src/sire/io/__init__.py

@@ -4,9 +4,10 @@
 from .. import use_new_api as _use_new_api
 
 # Imported to ensure that sire.maths.Vector is properly wrapped
-from ..maths import Vector as _Vector
+from ..maths import Vector as _Vector  # noqa: F401
 
-from . import parser
+# Importing the parser sub-module so that it autocompletes when used
+from . import parser  # noqa: F401
 
 _use_new_api()
 

src/sire/maths/_vector.py

@@ -166,7 +166,10 @@ def __str__(obj):
     _Vector.__repr__ = __str__
 
     def __format__(obj, format):
-        return f"({obj.x().value():{format}}, {obj.y().value():{format}}, {obj.z().value():{format}})"
+        return (
+            f"({obj.x().value():{format}}, {obj.y().value():{format}}, "
+            f"{obj.z().value():{format}})"
+        )
 
     _Vector.__format__ = __format__
 

src/sire/mol/__init__.py

@@ -47,7 +47,9 @@
 from ..legacy import Mol as _Mol
 from .. import use_new_api as _use_new_api
 
-from ..legacy import Base as _Base
+# Imported as we need to ensure that Base is pythonized before
+# loading the rest of this module
+from ..legacy import Base as _Base  # noqa: F401
 
 
 from ..legacy.Mol import (
@@ -114,8 +116,8 @@
     _selector_view2d,
 )
 
-# make sure that Vector has units attached
-from ..maths import Vector as _Vector
+# Imported to make sure that Vector has units attached
+from ..maths import Vector as _Vector  # noqa: F401
 
 _use_new_api()
 
@@ -1578,6 +1580,13 @@ def _dynamics(
 
         map.set("space", Cartesian())
 
+        view = view.clone()
+
+        try:
+            view.set_property("space", Cartesian())
+        except Exception:
+            pass
+
     if not map.specified("space"):
         map = create_map(map, {"space": "space"})
 
@@ -1588,7 +1597,7 @@ def _dynamics(
         and not view.shared_properties().has_property(map["space"])
     ):
         # space is not a shared property, so may be lost when we
-        # convert to molecules. Make sure this doens't happen by
+        # convert to molecules. Make sure this doesn't happen by
         # adding the space directly to the property map
         try:
             map.set("space", view.property(map["space"]))
@@ -1828,7 +1837,6 @@ def _minimisation(
     """
     from ..base import create_map
     from ..system import System
-    from .. import u
 
     map = create_map(map)
 
@@ -1837,6 +1845,13 @@ def _minimisation(
 
         map.set("space", Cartesian())
 
+        view = view.clone()
+
+        try:
+            view.set_property("space", Cartesian())
+        except Exception:
+            pass
+
     if not map.specified("space"):
         map = create_map(map, {"space": "space"})
 

src/sire/mol/_cursor.py

@@ -54,8 +54,8 @@ def __init__(self, molecule=None, map=None):
                     from ..utils import Console
 
                     Console.warning(
-                        f"Cannot auto-generate a connectivity for {self.molecule}."
-                        f" The error is:\n\n{e}"
+                        "Cannot auto-generate a connectivity for "
+                        f"{self.molecule}. The error is:\n\n{e}"
                     )
 
     def number(self):