Fixed the bug where passing a string for the "cutoff" option would le…

…ad to weird

behaviour (the cutoff was silently ignored)

Now you can set the cutoff using strings, e.g. `cutoff="10 A"` or even
`cutoff="infinite"` or `cutoff="none"`

Added a `map.unset("option")` to unset options from a property map.

corelib/src/libs/SireBase/propertymap.cpp

@@ -405,6 +405,13 @@ bool PropertyMap::specified(const PropertyName &propname) const
         return propname.hasValue();
 }
 
+/** Unset the property called 'name'. This will return it to default */
+void PropertyMap::unset(const QString &name)
+{
+    if (propmap.contains(name))
+        propmap.remove(name);
+}
+
 /** Set the property called 'name' to have the source or value
     in 'source'. This replaces any existing source or value
     for any existing property of this name in this map */

corelib/src/libs/SireBase/propertymap.h

@@ -206,6 +206,8 @@ namespace SireBase
 
         void set(const QString &name, const PropertyName &source);
 
+        void unset(const QString &name);
+
         PropertyMap addPrefix(const QString &prefix,
                               const QStringList &properties) const;
 

corelib/src/libs/SireSystem/forcefieldinfo.cpp

@@ -190,6 +190,15 @@ ForceFieldInfo::ForceFieldInfo(const System &system,
     {
         this->setCutoff(cutoff_prop.value().asA<GeneralUnitProperty>().toUnit<Length>());
     }
+    else if (cutoff_prop.source() != "cutoff")
+    {
+        throw SireError::invalid_arg(QObject::tr(
+                                         "The cutoff property should have a value. It cannot be the string "
+                                         "'%1'. If you want to specify the cutoff type, using "
+                                         "the 'cutoff_type' property.")
+                                         .arg(cutoff_prop.source()),
+                                     CODELOC);
+    }
 
     const auto cutoff_type = map["cutoff_type"];
 

doc/source/changelog.rst

@@ -76,6 +76,12 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.
 * Fixed the bug where the wrong return type from ``.minimisation()`` and
   ``.dynamics()`` was returned. This fixes issue #137.
 
+* Fixed the bug where the cutoff would not be set correctly if a string
+  was passed. You can now do ``mol.dynamics(cutoff="10A")`` or
+  ``mol.dynamics(cutoff="infinite")`` and it will be processed correctly.
+  This also required adding a ``map.unset("key")`` option to ``PropertyMap``,
+  to make it easier to unset mapped properties.
+
 * Please add an item to this changelog when you create your PR
 
 `2023.4.1 <https://github.com/openbiosim/sire/compare/2023.4.0...2023.4.1>`__ - October 2023

src/sire/mol/_dynamics.py

@@ -35,6 +35,22 @@ def __init__(self, mols=None, map=None, **kwargs):
                     mols.atoms().find(selection_to_atoms(mols, fixed_atoms)),
                 )
 
+            if map.specified("cutoff"):
+                cutoff = map["cutoff"]
+
+                if cutoff.has_source():
+                    cutoff = cutoff.source()
+
+                    if cutoff.lower() == "none" or cutoff.lower().startswith("infinit"):
+                        self._map.set("cutoff_type", "NONE")
+                        self._map.unset("cutoff")
+                    elif cutoff.lower() == "auto":
+                        self._map.unset("cutoff")
+                    elif cutoff != "cutoff":
+                        from .. import u
+
+                        self._map.set("cutoff", u(cutoff))
+
             # get the forcefield info from the passed parameters
             # and from whatever we can glean from the molecules
             from ..system import ForceFieldInfo, System

src/sire/system/__init__.py

@@ -22,6 +22,12 @@ def __init__(self, val, map=None):
 
         super().__init__(val, map=map)
 
+    def __repr__(self):
+        return self.__str__()
+
+    def __str__(self):
+        return super().__str__()
+
 
 _use_new_api()
 

tests/mol/test_dynamics.py

@@ -48,3 +48,30 @@ def test_minimisation_return_type(ala_mols):
     a = atom.minimisation(platform="Reference").run(1).commit()
 
     assert isinstance(a, type(atom))
+
+
+@pytest.mark.skipif(
+    "openmm" not in sr.convert.supported_formats(),
+    reason="openmm support is not available",
+)
+def test_cutoff_options(ala_mols):
+    mols = ala_mols
+
+    d = mols.dynamics(platform="Reference", cutoff="10 A")
+
+    assert d.info().cutoff() == sr.u("10 A")
+
+    d = mols[0].dynamics(platform="Reference", cutoff="infinite", vacuum=True)
+
+    # OpenMM cannot have no cutoff, so sets it to a very large number
+    assert d.info().cutoff() > sr.u("1000 A")
+
+    d = mols[0].dynamics(platform="Reference", cutoff="none", vacuum=True)
+
+    # OpenMM cannot have no cutoff, so sets it to a very large number
+    assert d.info().cutoff() > sr.u("1000 A")
+
+    d = mols.dynamics(platform="Reference", cutoff=sr.u("7.5A"), cutoff_type="PME")
+
+    assert d.info().cutoff() == sr.u("7.5 A")
+    assert d.info().cutoff_type() == "PME"

wrapper/Base/CMakeAutogenFile.txt

@@ -1,68 +1,68 @@
 # WARNING - AUTOGENERATED FILE - CONTENTS WILL BE OVERWRITTEN!
 set ( PYPP_SOURCES
-       Array2DBase.pypp.cpp
-       Array2D_double_.pypp.cpp
-       ArrayProperty_QString_.pypp.cpp
-       ArrayProperty_double_.pypp.cpp
-       ArrayProperty_int_.pypp.cpp
-       BooleanProperty.pypp.cpp
-       CPUID.pypp.cpp
-       ChunkedVector_double_.pypp.cpp
-       CombineProperties.pypp.cpp
+       TrigArray2DBase.pypp.cpp
        DoubleArrayProperty.pypp.cpp
-       FlopsMark.pypp.cpp
-       GeneralUnitArrayProperty.pypp.cpp
-       GeneralUnitProperty.pypp.cpp
-       Incremint.pypp.cpp
-       IntegerArrayProperty.pypp.cpp
-       LazyEvaluator.pypp.cpp
-       LengthProperty.pypp.cpp
-       LinkToProperty.pypp.cpp
-       LowerCaseString.pypp.cpp
-       MajorMinorVersion.pypp.cpp
-       MemInfo.pypp.cpp
-       NoMangling.pypp.cpp
        NullProperty.pypp.cpp
-       NumberProperty.pypp.cpp
-       PackedArray2D_DoubleArrayProperty.pypp.cpp
-       PackedArray2D_DoubleArrayProperty_Array.pypp.cpp
-       PackedArray2D_IntegerArrayProperty.pypp.cpp
-       PackedArray2D_IntegerArrayProperty_Array.pypp.cpp
-       PackedArray2D_PropertyList.pypp.cpp
        PackedArray2D_PropertyList_Array.pypp.cpp
-       PackedArray2D_QString_.pypp.cpp
+       Process.pypp.cpp
+       CombineProperties.pypp.cpp
+       _Base_free_functions.pypp.cpp
        PackedArray2D_QString_Array.pypp.cpp
-       PackedArray2D_QVariant_.pypp.cpp
-       PackedArray2D_QVariant_Array.pypp.cpp
-       PackedArray2D_StringArrayProperty.pypp.cpp
-       PackedArray2D_StringArrayProperty_Array.pypp.cpp
-       PackedArray2D_double_.pypp.cpp
-       PackedArray2D_double_Array.pypp.cpp
-       PackedArray2D_int_.pypp.cpp
+       PackedArray2D_IntegerArrayProperty.pypp.cpp
+       ArrayProperty_int_.pypp.cpp
+       PackedArray2D_IntegerArrayProperty_Array.pypp.cpp
+       ArrayProperty_double_.pypp.cpp
+       Incremint.pypp.cpp
+       PackedArray2D_DoubleArrayProperty_Array.pypp.cpp
+       PackedArray2D_DoubleArrayProperty.pypp.cpp
+       GeneralUnitProperty.pypp.cpp
+       vector_less__double__greater_.pypp.cpp
        PackedArray2D_int_Array.pypp.cpp
-       Process.pypp.cpp
-       ProgressBar.pypp.cpp
-       Properties.pypp.cpp
-       Property.pypp.cpp
-       PropertyList.pypp.cpp
-       PropertyMap.pypp.cpp
-       PropertyName.pypp.cpp
+       TrimString.pypp.cpp
+       FlopsMark.pypp.cpp
+       NoMangling.pypp.cpp
+       GeneralUnitArrayProperty.pypp.cpp
        Range.pypp.cpp
-       SimpleRange.pypp.cpp
-       StringArrayProperty.pypp.cpp
-       StringMangler.pypp.cpp
+       LengthProperty.pypp.cpp
+       ChunkedVector_double_.pypp.cpp
        StringProperty.pypp.cpp
+       PackedArray2D_QString_.pypp.cpp
        TempDir.pypp.cpp
+       PackedArray2D_double_.pypp.cpp
+       PropertyName.pypp.cpp
        TimeProperty.pypp.cpp
-       TrigArray2DBase.pypp.cpp
-       TrigArray2D_double_.pypp.cpp
-       TrimString.pypp.cpp
+       Array2DBase.pypp.cpp
+       NumberProperty.pypp.cpp
+       LinkToProperty.pypp.cpp
        UnitTest.pypp.cpp
-       UpperCaseString.pypp.cpp
+       PackedArray2D_PropertyList.pypp.cpp
+       ProgressBar.pypp.cpp
+       PropertyMap.pypp.cpp
+       LowerCaseString.pypp.cpp
+       PackedArray2D_double_Array.pypp.cpp
+       Properties.pypp.cpp
+       PropertyList.pypp.cpp
+       TrigArray2D_double_.pypp.cpp
+       PackedArray2D_StringArrayProperty.pypp.cpp
        VariantProperty.pypp.cpp
+       ArrayProperty_QString_.pypp.cpp
+       CPUID.pypp.cpp
+       UpperCaseString.pypp.cpp
+       PackedArray2D_QVariant_Array.pypp.cpp
+       BooleanProperty.pypp.cpp
+       PackedArray2D_StringArrayProperty_Array.pypp.cpp
        Version.pypp.cpp
-       _Base_free_functions.pypp.cpp
-       vector_less__double__greater_.pypp.cpp
+       SimpleRange.pypp.cpp
+       Array2D_double_.pypp.cpp
+       PackedArray2D_QVariant_.pypp.cpp
+       MemInfo.pypp.cpp
+       IntegerArrayProperty.pypp.cpp
+       StringArrayProperty.pypp.cpp
+       LazyEvaluator.pypp.cpp
+       Property.pypp.cpp
+       MajorMinorVersion.pypp.cpp
+       StringMangler.pypp.cpp
+       PackedArray2D_int_.pypp.cpp
        SireBase_containers.cpp
        SireBase_properties.cpp
        SireBase_registrars.cpp

wrapper/Base/Properties.pypp.cpp

@@ -24,6 +24,8 @@ namespace bp = boost::python;
 
 #include <QHash>
 
+#include <memory>
+
 #include "properties.h"
 
 SireBase::Properties __copy__(const SireBase::Properties &other){ return SireBase::Properties(other); }
@@ -53,7 +55,7 @@ void register_Properties_class(){
                 , addLink_function_value
                 , ( bp::arg("key"), bp::arg("linked_property") )
                 , bp::release_gil_policy()
-                , "" );
+                , "Add a link from the property key to the property linked_property.\n  The linked_property will be returned if there is no property\n  called key in this set.\n\n  Note that the linked property must already be contained in this set.\n" );
 
         }
         { //::SireBase::Properties::allMetadata
@@ -153,7 +155,7 @@ void register_Properties_class(){
                 "getLinks"
                 , getLinks_function_value
                 , bp::release_gil_policy()
-                , "" );
+                , "Return all of the property links" );
 
         }
         { //::SireBase::Properties::hasLinks
@@ -165,7 +167,7 @@ void register_Properties_class(){
                 "hasLinks"
                 , hasLinks_function_value
                 , bp::release_gil_policy()
-                , "" );
+                , "Return whether or not there are any property links" );
 
         }
         { //::SireBase::Properties::hasMetadata
@@ -422,7 +424,7 @@ void register_Properties_class(){
                 "removeAllLinks"
                 , removeAllLinks_function_value
                 , bp::release_gil_policy()
-                , "" );
+                , "Remove all property links from this set" );
 
         }
         { //::SireBase::Properties::removeAllMetadata
@@ -460,7 +462,7 @@ void register_Properties_class(){
                 , removeLink_function_value
                 , ( bp::arg("key") )
                 , bp::release_gil_policy()
-                , "" );
+                , "Remove the link associated with the key key" );
 
         }
         { //::SireBase::Properties::removeMetadata

wrapper/Base/PropertyMap.pypp.cpp

@@ -226,6 +226,19 @@ void register_PropertyMap_class(){
                 , bp::release_gil_policy()
                 , "" );
 
+        }
+        { //::SireBase::PropertyMap::unset
+
+            typedef void ( ::SireBase::PropertyMap::*unset_function_type)( ::QString const & ) ;
+            unset_function_type unset_function_value( &::SireBase::PropertyMap::unset );
+
+            PropertyMap_exposer.def( 
+                "unset"
+                , unset_function_value
+                , ( bp::arg("name") )
+                , bp::release_gil_policy()
+                , "Unset the property called name. This will return it to default" );
+
         }
         { //::SireBase::PropertyMap::what
 

wrapper/Base/SireBase_properties.cpp

@@ -4,6 +4,15 @@
 #include "Base/convertproperty.hpp"
 #include "SireBase_properties.h"
 
+#include "SireStream/datastream.h"
+#include "range.h"
+#include "ranges.h"
+#include "range.h"
+#include "SireStream/datastream.h"
+#include "SireStream/shareddatastream.h"
+#include "stringmangler.h"
+#include <QMutex>
+#include "stringmangler.h"
 #include "SireError/errors.h"
 #include "SireError/getbacktrace.h"
 #include "SireStream/datastream.h"
@@ -14,18 +23,9 @@
 #include <QDebug>
 #include <QMutex>
 #include "property.h"
-#include "SireStream/datastream.h"
-#include "range.h"
-#include "ranges.h"
-#include "range.h"
-#include "SireStream/datastream.h"
-#include "SireStream/shareddatastream.h"
-#include "stringmangler.h"
-#include <QMutex>
-#include "stringmangler.h"
 void register_SireBase_properties()
 {
-    register_property_container< SireBase::PropertyPtr, SireBase::Property >();
     register_property_container< SireBase::RangePtr, SireBase::Range >();
     register_property_container< SireBase::StringManglerPtr, SireBase::StringMangler >();
+    register_property_container< SireBase::PropertyPtr, SireBase::Property >();
 }