Merge branch 'master' into dask_retis_2021

dwhswenson · Jul 21, 2020 · Jul 21, 2020 · Jul 21, 2020 · Jul 22, 2020 · Jul 22, 2020
commit 3f6dd10a5ba058469e0a9306ff5095ae83f419f9
diff --git a/openpathsampling/engines/external_engine.py b/openpathsampling/engines/external_engine.py
@@ -55,25 +55,78 @@ def _debug_snapshot_loading(snapshot):
 
 
 class FilenameSetter(StorableNamedObject):
+    """Just use numbers, as we did previously.
+
+    This is the default for compatibility reasons, but it not recommended.
+    Generally, we recommend using :class:`.RandomString`.
+    """
+    # the weird use of this as the base class is because engine options has
+    # some weird type testing that requires replace object to be instances
+    # of the default
     def __init__(self, count=0):
-        super().__init__()
+        super(FilenameSetter, self).__init__()
         self.count = count
 
     def __call__(self):
-        retval = '{:07d}'.format(self.count)
+        retval = self.count
         self.count += 1
         return retval
 
+    def reset(self, count=0):
+        self.count = count
+
 
-class RandomString(FilenameSetter):
-    allowed = np.array([a for a in 'abcdefghijklmnopqrstuvwxyz0123456789'])
-    def __init__(self, length=8):
-        super().__init__()
+class RandomStringFilenames(FilenameSetter):
+    """Use a random string for filename prefixes.
+
+    This is recommended, and will become the default in future versions of
+    OpenPathSampling.
+
+    Parameters
+    ----------
+    length : int
+        number of character in the resulting string
+    allowed : str
+        string containing the allowed characters to return
+    """
+    _allowed = 'abcdefghijklmnopqrstuvwxyz0123456789'
+    def __init__(self, length=8, allowed=None):
+        super(RandomStringFilenames, self).__init__()
         self.length = length
+        allowed = allowed if allowed is not None else self._allowed
+        self.allowed = np.array([a for a in allowed])
 
     def __call__(self):
         return "".join(np.random.choice(self.allowed, self.length))
 
+class _InternalizedEngineProxy(DynamicsEngine):
+    """Wrapper that allows snapshots to be "internalized."
+
+    This is needed because the dynamically-registered snapshot tables are
+    associated to an engine: each engine creates exactly one snapshot table.
+    We use the internalized engine to create those snapshots.
+    """
+    def __init__(self, engine):
+        descriptor = SnapshotDescriptor.construct(
+            snapshot_class=engine.InternalizedSnapshotClass,
+            snapshot_dimensions=engine.descriptor._dimensions
+        )
+        self.engine = engine
+        super(_InternalizedEngineProxy, self).__init__(options={},
+                                                       descriptor=descriptor)
+
+    def to_dict(self):
+        # We only need the engine to load again
+        dct = {'engine': self.engine}
+        return dct
+
+    @property
+    def name(self):
+        return self.engine.name + " (internalized)"
+
+    def __getattr__(self, attr):
+        return getattr(self.engine, attr)
+
 
 class ExternalEngine(DynamicsEngine):
     """

diff --git a/openpathsampling/engines/gromacs/engine.py b/openpathsampling/engines/gromacs/engine.py
@@ -30,19 +30,23 @@
     _debug_open_files, close_file_descriptors
 )
 
-def _remove_file_if_exists(filename):
+def _remove_file_if_exists(filename):  # pragma: no cover
+    #  not requiring coverage here because it's part of Gromacs integration;
+    #  gets covered if gmx is installed though
     if os.path.isfile(filename):
         os.remove(filename)
 
 class _GroFileEngine(ExternalEngine):
+    SnapshotClass = ExternalMDSnapshot
+    InternalizedSnapshotClass = InternalizedMDSnapshot
     def __init__(self, gro):
         self.gro = gro
         traj = md.load(gro)
         self.topology = MDTrajTopology(traj.topology)
         n_atoms = self.topology.n_atoms
         n_spatial = self.topology.n_spatial
         descriptor = SnapshotDescriptor.construct(
-            snapshot_class=ExternalMDSnapshot,
+            snapshot_class=self.SnapshotClass,
             snapshot_dimensions={'n_spatial': n_spatial,
                                  'n_atoms': n_atoms}
         )
@@ -167,7 +171,7 @@ def __init__(self, gro, mdp, top, options, base_dir="", prefix="gmx"):
         self.output_file = self.prefix + "_trr/OUTPUT_NAME.trr"
         self.edr_file = self.prefix + "_edr/OUTPUT_NAME.edr"
         self.log_file = self.prefix + "_log/OUTPUT_NAME.log"
-        self.tpr_file = "topol.top"
+        self.tpr_file = "topol.tpr"
         self.mdout_file = "mdout.mdp"
 
         self._mdtraj_topology = None
@@ -299,6 +303,7 @@ def set_filenames(self, number):
             num_str = '{:07d}'.format(number + 1)
             self.output_file = self.trajectory_filename(number + 1)
             init_filename = "initial_frame.trr"
+            self.filename_setter.reset(number + 1)
         else:
             num_str = number
             self.output_file = self.trajectory_filename(num_str)
@@ -331,7 +336,8 @@ def prepare(self):  # pragma: no cover
         return_code = psutil.Popen(run_cmd, preexec_fn=os.setsid).wait()
         return return_code
 
-    def cleanup(self):
+    def cleanup(self):  # pragma: no cover
+        # tested when traj is run, which we don't on CI
         _remove_file_if_exists(self.input_file)
         _remove_file_if_exists(self.tpr_file)
         _remove_file_if_exists(self.mdout_file)

diff --git a/openpathsampling/experimental/simstore/sql_backend.py b/openpathsampling/experimental/simstore/sql_backend.py
@@ -115,16 +115,22 @@ def __init__(self, filename, mode='r', sql_dialect='sqlite', **kwargs):
         self.connection_uri = None
         self._metadata = None
         if filename is not None:
-            if self.mode == "w" and os.path.exists(filename):
+            file_exists = os.path.exists(filename)
+            if self.mode == "w" and file_exists:
                 # delete existing file; write after
                 os.remove(filename)
 
+            if self.mode == 'a' and not file_exists:
+                # act as if the mode is 'w'; note we change this back later
+                self.mode = 'w'
+
             self.connection_uri = self.filename_from_dialect(
                 filename,
                 self.sql_dialect
             )
             engine = sql.create_engine(self.connection_uri, **self.kwargs)
             self._initialize_from_engine(engine)
+            self.mode = mode  # in case we changed when checking existence
 
     def _initialize_from_engine(self, engine):
         self.engine = engine
@@ -133,7 +139,10 @@ def _initialize_from_engine(self, engine):
 
     @property
     def identifier(self):
-        return self.connection_uri, self.mode
+        if self.connection_uri == "sqlite:///:memory:":
+            return "sqlite:///{}".format(id(self)), self.mode
+        else:
+            return self.connection_uri, self.mode
 
     def to_dict(self):
         return {

diff --git a/openpathsampling/experimental/simstore/storage.py b/openpathsampling/experimental/simstore/storage.py
@@ -52,7 +52,7 @@ class GeneralStorage(StorableNamedObject):
     def __init__(self, backend, class_info, schema=None,
                  simulation_classes=None, fallbacks=None, safemode=False):
         super().__init__()
-        GeneralStorage._known_storages = {backend.identifier: self}
+        GeneralStorage._known_storages[backend.identifier] = self
         self.backend = backend
         self.schema = schema.copy()
         self.class_info = class_info.copy()

diff --git a/openpathsampling/experimental/storage/ops_storage.py b/openpathsampling/experimental/storage/ops_storage.py
@@ -33,8 +33,6 @@
 
 from ..simstore import SQLStorageBackend  # TODO: generalize
 
-from .sql_backend import SQLStorageBackend  # TODO: generalize
-
 from . import snapshots
 from .snapshots_table import SnapshotsTable
 
@@ -264,10 +262,11 @@ def from_backend(cls, backend, schema=None, class_info=None,
                      simulation_classes=None, fallbacks=None,
                      safemode=False):
         # quick exit if this storage is known
-        exists = cls._known_storages.get(backend.identifier, None)
+        exists = None
+        if backend.identifier[1] != 'w':
+            exists = cls._known_storages.get(backend.identifier, None)
         if exists is not None:
             return exists
-
         obj = cls.__new__(cls)
         obj.n_snapshot_types = 0
         schema = tools.none_to_default(schema, ops_schema)
@@ -298,6 +297,14 @@ def from_dict(cls, dct):
     def __reduce__(self):
         return (self.from_dict, (self.to_dict(),))
 
+    @property
+    def movechanges(self):
+        return self.move_changes
+
+    @property
+    def samplesets(self):
+        return self.sample_sets
+
     def register_from_tables(self, table_names, classes):
         lookups = {}
         table_to_class = {tbl: cls for tbl, cls in zip(table_names, classes)}