From 31a5dc7d5bf63820a5c7023433f1d76308d71c12 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 5 Sep 2024 15:30:19 +0200 Subject: [PATCH 01/70] Storage Handlers These are intended to abstract out filesystem storage to be an arbitrary key-value store. As examples, I've implemented storage on the filesystem and in memory. Object storage (e.g., S3) can be implemented separately. I'm making this a separate PR so that it can be used in future PRs. I'd also appreciate suggestions on a better name! `StorageHandler` is kind of awful. --- .../tests/utils/test_storage_handler.py | 97 ++++++++++++++++ openpathsampling/utils/storage_handlers.py | 108 ++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 openpathsampling/tests/utils/test_storage_handler.py create mode 100644 openpathsampling/utils/storage_handlers.py diff --git a/openpathsampling/tests/utils/test_storage_handler.py b/openpathsampling/tests/utils/test_storage_handler.py new file mode 100644 index 000000000..2f8872c42 --- /dev/null +++ b/openpathsampling/tests/utils/test_storage_handler.py @@ -0,0 +1,97 @@ +from openpathsampling.utils.storage_handlers import * +import pytest +import tempfile + +class StorageHandlerTest: + def setup_method(self): + self.tmpdir_manager = tempfile.TemporaryDirectory() + self.tmpdir = pathlib.Path(self.tmpdir_manager.__enter__()) + self.localdir = self.tmpdir / "local" + self.localdir.mkdir() + + # unstored local file + self.localfile = self.localdir / "localfile" + with open(self.localfile, mode='w') as f: + f.write("localfile contents") + + self._initialize_handler() + + def teardown_method(self): + self.tmpdir_manager.__exit__(None, None, None) + + def test_store(self): + raise NotImplementedError() + + def test_load(self): + target_file = self.localdir / "foo" + assert not target_file.exists() + self.handler.load("prestored", target_file) + assert target_file.exists() + with open(target_file, mode='r') as f: + assert f.read() == "prestored contents" + + def test_transfer(self): + raise NotImplementedError() + + def test_list_directory(self): + expected = ["nested/nest_prestored"] + assert self.handler.list_directory("nested") == expected + +class TestLocalFileStorageHandler(StorageHandlerTest): + def _initialize_handler(self): + root = self.tmpdir / "stored" + self.handler = LocalFileStorageHandler(root) + # pre-stored file + with open(root / "prestored", mode='w') as f: + f.write("prestored contents") + + # pre-stored nested files (for directory testing) + nested_prestored = root / "nested/nest_prestored" + nested_prestored.parent.mkdir() + with open(nested_prestored, mode='w') as f: + f.write("nested prestored contents") + + def test_store(self): + stored_file = self.handler.root / "foo" + assert not stored_file.exists() + self.handler.store("foo", self.localfile) + assert stored_file.exists() + assert self.localfile.exists() + with open(stored_file, mode='r') as f: + assert f.read() == "localfile contents" + + def test_transfer(self): + stored_target = self.handler.root / "foo" + assert not stored_target.exists() + self.handler.transfer("foo", self.localfile) + assert stored_target.exists() + assert not self.localfile.exists() + with open(stored_target, mode='r') as f: + assert f.read() == "localfile contents" + + +class TestMemoryStorageHandler(StorageHandlerTest): + def _initialize_handler(self): + self.handler = MemoryStorageHandler() + data = { + "prestored": b"prestored contents", + "nested/nest_prestored": b"nested prestored contents", + } + self.handler._data = data + + def test_store(self): + stored_target = "foo" + assert stored_target not in self.handler._data + self.handler.store(stored_target, self.localfile) + assert stored_target in self.handler._data + assert self.localfile.exists() + assert self.handler._data[stored_target] == b"localfile contents" + + def test_transfer(self): + stored_target = "foo" + assert stored_target not in self.handler._data + self.handler.transfer(stored_target, self.localfile) + assert stored_target in self.handler._data + assert not self.localfile.exists() + assert self.handler._data[stored_target] == b"localfile contents" + diff --git a/openpathsampling/utils/storage_handlers.py b/openpathsampling/utils/storage_handlers.py new file mode 100644 index 000000000..f51446b11 --- /dev/null +++ b/openpathsampling/utils/storage_handlers.py @@ -0,0 +1,108 @@ +from abc import ABC, abstractmethod +import os +import pathlib +import shutil + +import logging +_logger = logging.getLogger(__name__) + + + +class StorageHandler(ABC): + """Abstract treatment of a key-value-like file/object store. + + This is generally assuming file-based semantics. This may typically mean + putting things into a temporary directory. This is particularly focused + on checkpointing, where we will copy the data to put it in a zip file. + """ + @abstractmethod + def store(self, storage_label, source_path): + raise NotImplementedError() + + @abstractmethod + def load(self, storage_label, target_path): + raise NotImplementedError() + + @abstractmethod + def delete(self, storage_label): + raise NotImplementedError() + + @abstractmethod + def __contains__(self, storage_label): + raise NotImplementedError() + + def transfer(self, storage_label, source_path): + """Transfer a file to the storage label from the source path. + + In some cases, this can be made faster than store followed by + os.remove, so this method can be overridden. (Example: moving ona + file system is faster than copying.) + """ + self.store(storage_label, source_path) + os.remove(source_path) + + @abstractmethod + def list_directory(self, storage_label): + raise NotImplementedError() + + +class LocalFileStorageHandler(StorageHandler): + """Concrete implementation of StorageHandler for local files. + """ + def __init__(self, root): + self.root = pathlib.Path(root) + self.root.mkdir(parents=True, exist_ok=True) + + def store(self, storage_label, source_path): + local_path = self.root / storage_label + local_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source_path, self.root / storage_label) + + def load(self, storage_label, target_path): + target_path.parent.mkdir(parents=True, exist_ok=True) + local_path = self.root / storage_label + _logger.debug("Copying file from {str(local_path)} " + f"to {str(target_path)}") + shutil.copyfile(local_path, target_path) + + def delete(self, storage_label): + os.remove(self.root / storage_label) + + def __contains__(self, storage_label): + return (self.root / storage_label).exists() + + def transfer(self, storage_label, source_path): + shutil.move(source_path, self.root / storage_label) + + def list_directory(self, storage_label): + return [str(p.relative_to(self.root)) + for p in (self.root / storage_label).iterdir()] + + +class MemoryStorageHandler(StorageHandler): + """Useful in testing.""" + def __init__(self): + self._data = {} + + def store(self, storage_label, source_path): + with open(source_path, mode='rb') as f: + self._data[storage_label] = f.read() + + def load(self, storage_label, target_path): + with open(target_path, mode='wb') as f: + f.write(self._data[storage_label]) + return self._data[storage_label] + + def delete(self, storage_label): + del self._data[storage_label] + + def __contains__(self, storage_label): + return storage_label in self._data + + def list_directory(self, storage_label): + # special case because the empty path becomes '.' as a string + if storage_label == pathlib.Path(""): + storage_label = "" + + return [key for key in self._data + if str(key).startswith(str(storage_label))] From 269fab32f95131d68955cf15e20ee06a7dac27c8 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 5 Sep 2024 15:39:26 +0200 Subject: [PATCH 02/70] fix name of test file --- .../utils/{test_storage_handler.py => test_storage_handlers.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename openpathsampling/tests/utils/{test_storage_handler.py => test_storage_handlers.py} (100%) diff --git a/openpathsampling/tests/utils/test_storage_handler.py b/openpathsampling/tests/utils/test_storage_handlers.py similarity index 100% rename from openpathsampling/tests/utils/test_storage_handler.py rename to openpathsampling/tests/utils/test_storage_handlers.py From 39d88ea260f554ae2588319ecedd35095d0b6cf5 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Fri, 13 Sep 2024 13:14:12 +0200 Subject: [PATCH 03/70] Make StorageHandlerTest proper ABC Includes docs for what the inherited function should do. --- openpathsampling/tests/utils/test_storage_handlers.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openpathsampling/tests/utils/test_storage_handlers.py b/openpathsampling/tests/utils/test_storage_handlers.py index 2f8872c42..cdeb27835 100644 --- a/openpathsampling/tests/utils/test_storage_handlers.py +++ b/openpathsampling/tests/utils/test_storage_handlers.py @@ -16,6 +16,17 @@ def setup_method(self): self._initialize_handler() + def _initialize_handler(self): + """Subclasses must implement this initialization routine. + + This method must create the following stored objects: + + * key ``prestored``, contents ``prestored contents`` + * key ``nested/nest_prestored``, contents ``nested prestored + contents`` + """ + raise NotImplementedError() + def teardown_method(self): self.tmpdir_manager.__exit__(None, None, None) From 593b04f21de0ad9ac440477e42a71b4203dc011a Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Fri, 13 Sep 2024 13:23:50 +0200 Subject: [PATCH 04/70] Docstrings on StorageHandler ABC --- openpathsampling/utils/storage_handlers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openpathsampling/utils/storage_handlers.py b/openpathsampling/utils/storage_handlers.py index f51446b11..52b21f8b8 100644 --- a/openpathsampling/utils/storage_handlers.py +++ b/openpathsampling/utils/storage_handlers.py @@ -17,14 +17,20 @@ class StorageHandler(ABC): """ @abstractmethod def store(self, storage_label, source_path): + """Store the data in ``source_path`` at key ``storage_label`` + """ raise NotImplementedError() @abstractmethod def load(self, storage_label, target_path): + """Load the data from ``storage_label`` into file at ``target_path`` + """ raise NotImplementedError() @abstractmethod def delete(self, storage_label): + """Delete key ``storage_label`` from the object store. + """ raise NotImplementedError() @abstractmethod From 82a8413cc260f754d6dfce6c6e6cd57a3d5b5b2d Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 3 Oct 2024 23:25:46 -0500 Subject: [PATCH 05/70] Bump 1.7.1.dev0 --- openpathsampling/netcdfplus/version.py | 2 +- setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openpathsampling/netcdfplus/version.py b/openpathsampling/netcdfplus/version.py index 2342e1b7c..952a2d0ae 100644 --- a/openpathsampling/netcdfplus/version.py +++ b/openpathsampling/netcdfplus/version.py @@ -1,4 +1,4 @@ -short_version = '1.7.0' +short_version = '1.7.1.dev0' version = short_version full_version = short_version git_revision = 'alpha' diff --git a/setup.cfg b/setup.cfg index d06868d5f..f79c019be 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = openpathsampling -version = 1.7.0 +version = 1.7.1.dev0 description = A Python package for path sampling simulations long_description = file: README.md long_description_content_type = text/markdown From e221a2c0c57504efd7f6606298ade34ecad00189 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 15 Oct 2024 15:40:54 -0600 Subject: [PATCH 06/70] Update to use setup-miniconda v3; use miniforge Mambaforge has been deprecated. See https://conda-forge.org/news/2024/07/29/sunsetting-mambaforge/ --- .github/workflows/deploy_docs.yml | 4 ++-- .github/workflows/tests.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/deploy_docs.yml b/.github/workflows/deploy_docs.yml index d62d14937..7bd9849c2 100644 --- a/.github/workflows/deploy_docs.yml +++ b/.github/workflows/deploy_docs.yml @@ -18,11 +18,11 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 - - uses: conda-incubator/setup-miniconda@v2 + - uses: conda-incubator/setup-miniconda@v3 with: auto-update-conda: true python-version: "3.11" - miniforge-variant: Mambaforge + miniforge-version: latest - name: "Install requirements" run: source devtools/conda_install_reqs.sh - name: "Install OPS" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 43d9b1fb4..864b2b4f7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,11 +44,11 @@ jobs: with: fetch-depth: 2 - uses: actions/setup-python@v2 - - uses: conda-incubator/setup-miniconda@v2 + - uses: conda-incubator/setup-miniconda@v3 with: auto-update-conda: true python-version: ${{ matrix.CONDA_PY }} - miniforge-variant: Mambaforge + miniforge-version: latest - name: "Install requirements" env: MINIMAL: ${{ matrix.MINIMAL }} From 8570db1f7ad252ebe1ae253f1075372515fc8750 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 9 Nov 2024 19:22:36 -0600 Subject: [PATCH 07/70] Updates in response to review * Switched to a general "directories not allowed" policy on transfer/delete/etc. * Changed `list_directory` to list all things in subdirectories as well --- .../tests/utils/test_storage_handlers.py | 69 ++++++++++++++++++- openpathsampling/utils/storage_handlers.py | 55 +++++++++++++-- 2 files changed, 117 insertions(+), 7 deletions(-) diff --git a/openpathsampling/tests/utils/test_storage_handlers.py b/openpathsampling/tests/utils/test_storage_handlers.py index cdeb27835..c144d3361 100644 --- a/openpathsampling/tests/utils/test_storage_handlers.py +++ b/openpathsampling/tests/utils/test_storage_handlers.py @@ -24,6 +24,7 @@ def _initialize_handler(self): * key ``prestored``, contents ``prestored contents`` * key ``nested/nest_prestored``, contents ``nested prestored contents`` + * key ``nested/deeply/prestored``, contents ``deeply nested`` """ raise NotImplementedError() @@ -33,6 +34,12 @@ def teardown_method(self): def test_store(self): raise NotImplementedError() + def test_delete(self): + raise NotImplementedError() + + def test_delete_directory(self): + raise NotImplementedError() + def test_load(self): target_file = self.localdir / "foo" assert not target_file.exists() @@ -44,9 +51,12 @@ def test_load(self): def test_transfer(self): raise NotImplementedError() + def test_transfer_directory(self): + raise NotImplementedError() + def test_list_directory(self): - expected = ["nested/nest_prestored"] - assert self.handler.list_directory("nested") == expected + expected = {"nested/nest_prestored", "nested/deeply/prestored"} + assert set(self.handler.list_directory("nested")) == expected class TestLocalFileStorageHandler(StorageHandlerTest): def _initialize_handler(self): @@ -62,6 +72,12 @@ def _initialize_handler(self): with open(nested_prestored, mode='w') as f: f.write("nested prestored contents") + # deeply nested file + deeply_prestored = root / "nested/deeply/prestored" + deeply_prestored.parent.mkdir(parents=True) + with open(deeply_prestored, mode='w') as f: + f.write("deeply nested") + def test_store(self): stored_file = self.handler.root / "foo" assert not stored_file.exists() @@ -71,6 +87,20 @@ def test_store(self): with open(stored_file, mode='r') as f: assert f.read() == "localfile contents" + def test_delete(self): + assert (self.handler.root / 'prestored').exists() + assert 'prestored' in self.handler + self.handler.delete('prestored') + assert "prestored" not in self.handler + assert not (self.handler.root / 'prestored').exists() + + def test_delete_directory(self): + nested_file = "nested/nest_prestored" + assert (self.handler.root / nested_file).exists() + assert nested_file in self.handler + with pytest.raises(ValueError, match="is a directory"): + self.handler.delete('nested') + def test_transfer(self): stored_target = self.handler.root / "foo" assert not stored_target.exists() @@ -80,6 +110,19 @@ def test_transfer(self): with open(stored_target, mode='r') as f: assert f.read() == "localfile contents" + def test_transfer_directory(self): + source_dir = self.localdir / "directory" + source_dir.mkdir(exist_ok=True, parents=True) + subfile = source_dir / "file" + with open(subfile, mode='w') as f: + f.write("directory/file contents") + + assert "directory/file" not in self.handler + assert subfile.exists() + + with pytest.raises(ValueError, match="is a directory"): + self.handler.transfer("directory", source_dir) + class TestMemoryStorageHandler(StorageHandlerTest): def _initialize_handler(self): @@ -87,6 +130,7 @@ def _initialize_handler(self): data = { "prestored": b"prestored contents", "nested/nest_prestored": b"nested prestored contents", + "nested/deeply/prestored": b"deeply nested", } self.handler._data = data @@ -106,3 +150,24 @@ def test_transfer(self): assert not self.localfile.exists() assert self.handler._data[stored_target] == b"localfile contents" + def test_delete(self): + assert "prestored" in self.handler._data + self.handler.delete("prestored") + assert "prestored" not in self.handler._data + + def test_delete_directory(self): + with pytest.raises(KeyError): + self.handler.delete("nested") + + def test_transfer_directory(self): + source_dir = self.localdir / "directory" + source_dir.mkdir(exist_ok=True, parents=True) + subfile = source_dir / "file" + with open(subfile, mode='w') as f: + f.write("directory/file contents") + + assert "directory/file" not in self.handler._data + assert subfile.exists() + + with pytest.raises(ValueError, match="is a directory"): + self.handler.transfer("directory", source_dir) diff --git a/openpathsampling/utils/storage_handlers.py b/openpathsampling/utils/storage_handlers.py index 52b21f8b8..db62a5f75 100644 --- a/openpathsampling/utils/storage_handlers.py +++ b/openpathsampling/utils/storage_handlers.py @@ -18,12 +18,26 @@ class StorageHandler(ABC): @abstractmethod def store(self, storage_label, source_path): """Store the data in ``source_path`` at key ``storage_label`` + + Parameters + ---------- + storage_label : str + The key to store the data at. + source_path : os.PathLike + The path to the data to store. """ raise NotImplementedError() @abstractmethod def load(self, storage_label, target_path): """Load the data from ``storage_label`` into file at ``target_path`` + + Parameters + ---------- + storage_label : str + The key to load the data from. + target_path : os.PathLike + The path to store the data at. """ raise NotImplementedError() @@ -44,16 +58,26 @@ def transfer(self, storage_label, source_path): os.remove, so this method can be overridden. (Example: moving ona file system is faster than copying.) """ + if pathlib.Path(source_path).is_dir(): + raise ValueError(f"'{source_path}' is a directory, and can't " + "be transferred.") self.store(storage_label, source_path) os.remove(source_path) @abstractmethod def list_directory(self, storage_label): + """List all objects in subdirectories of the given storage label. + """ raise NotImplementedError() class LocalFileStorageHandler(StorageHandler): """Concrete implementation of StorageHandler for local files. + + Parameters + ---------- + root : os.PathLike + The root directory for the storage handler. """ def __init__(self, root): self.root = pathlib.Path(root) @@ -72,21 +96,40 @@ def load(self, storage_label, target_path): shutil.copyfile(local_path, target_path) def delete(self, storage_label): - os.remove(self.root / storage_label) + obj = self.root / storage_label + if obj.is_dir(): + raise ValueError(f"'{obj}' is a directory, and can't be " + "deleted.") + else: + _logger.debug("Deleting file {str(obj)}") + os.remove(self.root / storage_label) def __contains__(self, storage_label): - return (self.root / storage_label).exists() + expected = self.root / storage_label + return expected.exists() and not expected.is_dir() def transfer(self, storage_label, source_path): + if pathlib.Path(source_path).is_dir(): + raise ValueError(f"'{source_path}' is a directory, and can't " + "be transferred.") shutil.move(source_path, self.root / storage_label) def list_directory(self, storage_label): - return [str(p.relative_to(self.root)) - for p in (self.root / storage_label).iterdir()] + path = self.root / storage_label + if not path.is_dir(): + raise ValueError(f"'{path}' is not a directory.") + return [ + str((pathlib.Path(p[0]) / subp).relative_to(self.root)) + for p in os.walk(path) + for subp in p[2] + ] class MemoryStorageHandler(StorageHandler): - """Useful in testing.""" + """In-memory storage handler. + + Useful in testing. + """ def __init__(self): self._data = {} @@ -109,6 +152,8 @@ def list_directory(self, storage_label): # special case because the empty path becomes '.' as a string if storage_label == pathlib.Path(""): storage_label = "" + elif not storage_label.endswith("/"): + storage_label += "/" return [key for key in self._data if str(key).startswith(str(storage_label))] From 0de77084d18b1bd60b05bd6cce1a9b4cde1c0e83 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 9 Nov 2024 19:57:55 -0600 Subject: [PATCH 08/70] Test coverage improvements --- .../tests/utils/test_storage_handlers.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/openpathsampling/tests/utils/test_storage_handlers.py b/openpathsampling/tests/utils/test_storage_handlers.py index c144d3361..8f2dec1da 100644 --- a/openpathsampling/tests/utils/test_storage_handlers.py +++ b/openpathsampling/tests/utils/test_storage_handlers.py @@ -58,6 +58,13 @@ def test_list_directory(self): expected = {"nested/nest_prestored", "nested/deeply/prestored"} assert set(self.handler.list_directory("nested")) == expected + def test_contains(self): + assert "prestored" in self.handler + assert "nested/nest_prestored" in self.handler + assert "nested" not in self.handler + assert "nonexistent" not in self.handler + + class TestLocalFileStorageHandler(StorageHandlerTest): def _initialize_handler(self): root = self.tmpdir / "stored" @@ -123,6 +130,10 @@ def test_transfer_directory(self): with pytest.raises(ValueError, match="is a directory"): self.handler.transfer("directory", source_dir) + def test_list_directory_not_directory(self): + with pytest.raises(ValueError, match="is not a directory"): + self.handler.list_directory("prestored") + class TestMemoryStorageHandler(StorageHandlerTest): def _initialize_handler(self): @@ -171,3 +182,12 @@ def test_transfer_directory(self): with pytest.raises(ValueError, match="is a directory"): self.handler.transfer("directory", source_dir) + + def test_list_root_directory(self): + expected = { + "prestored", + "nested/nest_prestored", + "nested/deeply/prestored" + } + root = pathlib.Path("") + assert set(self.handler.list_directory(root)) == expected From d511c0d0bd261310cf163e423f71c102db0a78a5 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 9 Nov 2024 20:23:39 -0600 Subject: [PATCH 09/70] Apply suggestions from code review Co-authored-by: Sander Roet --- openpathsampling/utils/storage_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpathsampling/utils/storage_handlers.py b/openpathsampling/utils/storage_handlers.py index db62a5f75..9495b669c 100644 --- a/openpathsampling/utils/storage_handlers.py +++ b/openpathsampling/utils/storage_handlers.py @@ -55,7 +55,7 @@ def transfer(self, storage_label, source_path): """Transfer a file to the storage label from the source path. In some cases, this can be made faster than store followed by - os.remove, so this method can be overridden. (Example: moving ona + os.remove, so this method can be overridden. (Example: moving on a file system is faster than copying.) """ if pathlib.Path(source_path).is_dir(): From 108a9c10322d53226e9e6d8eb66916f73bc8a3dc Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 9 Nov 2024 20:48:07 -0600 Subject: [PATCH 10/70] StorageHandler -> StorageInterface --- ...handlers.py => test_storage_interfaces.py} | 94 +++++++++---------- ...rage_handlers.py => storage_interfaces.py} | 12 +-- 2 files changed, 53 insertions(+), 53 deletions(-) rename openpathsampling/tests/utils/{test_storage_handlers.py => test_storage_interfaces.py} (64%) rename openpathsampling/utils/{storage_handlers.py => storage_interfaces.py} (94%) diff --git a/openpathsampling/tests/utils/test_storage_handlers.py b/openpathsampling/tests/utils/test_storage_interfaces.py similarity index 64% rename from openpathsampling/tests/utils/test_storage_handlers.py rename to openpathsampling/tests/utils/test_storage_interfaces.py index 8f2dec1da..9d351eea8 100644 --- a/openpathsampling/tests/utils/test_storage_handlers.py +++ b/openpathsampling/tests/utils/test_storage_interfaces.py @@ -1,8 +1,8 @@ -from openpathsampling.utils.storage_handlers import * +from openpathsampling.utils.storage_interfaces import * import pytest import tempfile -class StorageHandlerTest: +class StorageInterfaceTest: def setup_method(self): self.tmpdir_manager = tempfile.TemporaryDirectory() self.tmpdir = pathlib.Path(self.tmpdir_manager.__enter__()) @@ -14,9 +14,9 @@ def setup_method(self): with open(self.localfile, mode='w') as f: f.write("localfile contents") - self._initialize_handler() + self._initialize_interface() - def _initialize_handler(self): + def _initialize_interface(self): """Subclasses must implement this initialization routine. This method must create the following stored objects: @@ -43,7 +43,7 @@ def test_delete_directory(self): def test_load(self): target_file = self.localdir / "foo" assert not target_file.exists() - self.handler.load("prestored", target_file) + self.interface.load("prestored", target_file) assert target_file.exists() with open(target_file, mode='r') as f: assert f.read() == "prestored contents" @@ -56,19 +56,19 @@ def test_transfer_directory(self): def test_list_directory(self): expected = {"nested/nest_prestored", "nested/deeply/prestored"} - assert set(self.handler.list_directory("nested")) == expected + assert set(self.interface.list_directory("nested")) == expected def test_contains(self): - assert "prestored" in self.handler - assert "nested/nest_prestored" in self.handler - assert "nested" not in self.handler - assert "nonexistent" not in self.handler + assert "prestored" in self.interface + assert "nested/nest_prestored" in self.interface + assert "nested" not in self.interface + assert "nonexistent" not in self.interface -class TestLocalFileStorageHandler(StorageHandlerTest): - def _initialize_handler(self): +class TestLocalFileStorageInterface(StorageInterfaceTest): + def _initialize_interface(self): root = self.tmpdir / "stored" - self.handler = LocalFileStorageHandler(root) + self.interface = LocalFileStorageInterface(root) # pre-stored file with open(root / "prestored", mode='w') as f: f.write("prestored contents") @@ -86,32 +86,32 @@ def _initialize_handler(self): f.write("deeply nested") def test_store(self): - stored_file = self.handler.root / "foo" + stored_file = self.interface.root / "foo" assert not stored_file.exists() - self.handler.store("foo", self.localfile) + self.interface.store("foo", self.localfile) assert stored_file.exists() assert self.localfile.exists() with open(stored_file, mode='r') as f: assert f.read() == "localfile contents" def test_delete(self): - assert (self.handler.root / 'prestored').exists() - assert 'prestored' in self.handler - self.handler.delete('prestored') - assert "prestored" not in self.handler - assert not (self.handler.root / 'prestored').exists() + assert (self.interface.root / 'prestored').exists() + assert 'prestored' in self.interface + self.interface.delete('prestored') + assert "prestored" not in self.interface + assert not (self.interface.root / 'prestored').exists() def test_delete_directory(self): nested_file = "nested/nest_prestored" - assert (self.handler.root / nested_file).exists() - assert nested_file in self.handler + assert (self.interface.root / nested_file).exists() + assert nested_file in self.interface with pytest.raises(ValueError, match="is a directory"): - self.handler.delete('nested') + self.interface.delete('nested') def test_transfer(self): - stored_target = self.handler.root / "foo" + stored_target = self.interface.root / "foo" assert not stored_target.exists() - self.handler.transfer("foo", self.localfile) + self.interface.transfer("foo", self.localfile) assert stored_target.exists() assert not self.localfile.exists() with open(stored_target, mode='r') as f: @@ -124,51 +124,51 @@ def test_transfer_directory(self): with open(subfile, mode='w') as f: f.write("directory/file contents") - assert "directory/file" not in self.handler + assert "directory/file" not in self.interface assert subfile.exists() with pytest.raises(ValueError, match="is a directory"): - self.handler.transfer("directory", source_dir) + self.interface.transfer("directory", source_dir) def test_list_directory_not_directory(self): with pytest.raises(ValueError, match="is not a directory"): - self.handler.list_directory("prestored") + self.interface.list_directory("prestored") -class TestMemoryStorageHandler(StorageHandlerTest): - def _initialize_handler(self): - self.handler = MemoryStorageHandler() +class TestMemoryStorageInterface(StorageInterfaceTest): + def _initialize_interface(self): + self.interface = MemoryStorageInterface() data = { "prestored": b"prestored contents", "nested/nest_prestored": b"nested prestored contents", "nested/deeply/prestored": b"deeply nested", } - self.handler._data = data + self.interface._data = data def test_store(self): stored_target = "foo" - assert stored_target not in self.handler._data - self.handler.store(stored_target, self.localfile) - assert stored_target in self.handler._data + assert stored_target not in self.interface._data + self.interface.store(stored_target, self.localfile) + assert stored_target in self.interface._data assert self.localfile.exists() - assert self.handler._data[stored_target] == b"localfile contents" + assert self.interface._data[stored_target] == b"localfile contents" def test_transfer(self): stored_target = "foo" - assert stored_target not in self.handler._data - self.handler.transfer(stored_target, self.localfile) - assert stored_target in self.handler._data + assert stored_target not in self.interface._data + self.interface.transfer(stored_target, self.localfile) + assert stored_target in self.interface._data assert not self.localfile.exists() - assert self.handler._data[stored_target] == b"localfile contents" + assert self.interface._data[stored_target] == b"localfile contents" def test_delete(self): - assert "prestored" in self.handler._data - self.handler.delete("prestored") - assert "prestored" not in self.handler._data + assert "prestored" in self.interface._data + self.interface.delete("prestored") + assert "prestored" not in self.interface._data def test_delete_directory(self): with pytest.raises(KeyError): - self.handler.delete("nested") + self.interface.delete("nested") def test_transfer_directory(self): source_dir = self.localdir / "directory" @@ -177,11 +177,11 @@ def test_transfer_directory(self): with open(subfile, mode='w') as f: f.write("directory/file contents") - assert "directory/file" not in self.handler._data + assert "directory/file" not in self.interface._data assert subfile.exists() with pytest.raises(ValueError, match="is a directory"): - self.handler.transfer("directory", source_dir) + self.interface.transfer("directory", source_dir) def test_list_root_directory(self): expected = { @@ -190,4 +190,4 @@ def test_list_root_directory(self): "nested/deeply/prestored" } root = pathlib.Path("") - assert set(self.handler.list_directory(root)) == expected + assert set(self.interface.list_directory(root)) == expected diff --git a/openpathsampling/utils/storage_handlers.py b/openpathsampling/utils/storage_interfaces.py similarity index 94% rename from openpathsampling/utils/storage_handlers.py rename to openpathsampling/utils/storage_interfaces.py index db62a5f75..b4e415b8b 100644 --- a/openpathsampling/utils/storage_handlers.py +++ b/openpathsampling/utils/storage_interfaces.py @@ -8,7 +8,7 @@ -class StorageHandler(ABC): +class StorageInterface(ABC): """Abstract treatment of a key-value-like file/object store. This is generally assuming file-based semantics. This may typically mean @@ -71,13 +71,13 @@ def list_directory(self, storage_label): raise NotImplementedError() -class LocalFileStorageHandler(StorageHandler): - """Concrete implementation of StorageHandler for local files. +class LocalFileStorageInterface(StorageInterface): + """Concrete implementation of StorageInterface for local files. Parameters ---------- root : os.PathLike - The root directory for the storage handler. + The root directory for the storage interface. """ def __init__(self, root): self.root = pathlib.Path(root) @@ -125,8 +125,8 @@ def list_directory(self, storage_label): ] -class MemoryStorageHandler(StorageHandler): - """In-memory storage handler. +class MemoryStorageInterface(StorageInterface): + """In-memory storage interface. Useful in testing. """ From e28414643fb3a5fd1955453465dcf9a05ece59fe Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 11 Nov 2024 14:45:41 -0600 Subject: [PATCH 11/70] Update openpathsampling/utils/storage_interfaces.py Co-authored-by: Sander Roet --- openpathsampling/utils/storage_interfaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpathsampling/utils/storage_interfaces.py b/openpathsampling/utils/storage_interfaces.py index 48f98d8db..3870925be 100644 --- a/openpathsampling/utils/storage_interfaces.py +++ b/openpathsampling/utils/storage_interfaces.py @@ -102,7 +102,7 @@ def delete(self, storage_label): "deleted.") else: _logger.debug("Deleting file {str(obj)}") - os.remove(self.root / storage_label) + os.remove(obj) def __contains__(self, storage_label): expected = self.root / storage_label From b4811fbfaca631065592ee48e2bcda33d6a31e79 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 05:48:17 -0500 Subject: [PATCH 12/70] update for SPEC0 as of Dec 2024 --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 864b2b4f7..c1690cd54 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -31,12 +31,12 @@ jobs: # CVs) can differ between minor Python versions. matrix: CONDA_PY: + - "3.13" - "3.12" - "3.11" - - "3.10" MINIMAL: [""] include: - - CONDA_PY: "3.10" + - CONDA_PY: "3.11" MINIMAL: "minimal" steps: From 29c1f098d7e985531cb2636fdd3a673ac3cbec46 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 06:11:26 -0500 Subject: [PATCH 13/70] See if we can safely unpin numpy<2.0 now This was an issue that dependencies weren't yet ready for it. I think that has now been fixed. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f79c019be..febad0f66 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,7 +30,7 @@ python_requires = >=3.10 install_requires = future psutil - numpy<2.0 + numpy scipy pandas netcdf4 From be63f72859aeaf4a156ef9dd485fac5d78a4cd6f Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 09:49:11 -0500 Subject: [PATCH 14/70] Core of trajectory export, and SimStore exporter with tests! --- openpathsampling/exports/__init__.py | 0 .../exports/trajectories/__init__.py | 1 + openpathsampling/exports/trajectories/core.py | 51 ++++++++ .../tests/exports/trajectories/test_core.py | 117 ++++++++++++++++++ 4 files changed, 169 insertions(+) create mode 100644 openpathsampling/exports/__init__.py create mode 100644 openpathsampling/exports/trajectories/__init__.py create mode 100644 openpathsampling/exports/trajectories/core.py create mode 100644 openpathsampling/tests/exports/trajectories/test_core.py diff --git a/openpathsampling/exports/__init__.py b/openpathsampling/exports/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/openpathsampling/exports/trajectories/__init__.py b/openpathsampling/exports/trajectories/__init__.py new file mode 100644 index 000000000..a1d92a6cc --- /dev/null +++ b/openpathsampling/exports/trajectories/__init__.py @@ -0,0 +1 @@ +from .core import TrajectoryWriter, SimStoreTrajectoryWriter diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py new file mode 100644 index 000000000..c2894e7be --- /dev/null +++ b/openpathsampling/exports/trajectories/core.py @@ -0,0 +1,51 @@ +import pathlib +import contextlib +import logging +from openpathsampling.integrations import HAS_MDTRAJ + +import numpy as np + +_logger = logging.getLogger(__name__) + + +class TrajectoryWriter: + """Base class for tools to write trajectories to extenral files. + + This is essentially a wrapper for a function to write the trajectory to + a file. The function should take two arguments: the trajectory to write, + and the filename to write to. + + We use an object-oriented approach here so that initialization can + inlude arbitrary parameters, but the interface used by other code is + consistent. Additionally, :meth:`__call__` is can handle some standard + error checking. + """ + def __call__(self, trajectory, filename, force=False): + if not force and pathlib.Path(filename).exists(): + raise FileExistsError(f"File {filename} already exists") + + self._write(trajectory, filename) + + def _write(self, trajectory, filename): + """Write the trajectory to the file. + + Parameters + ---------- + trajectory : openpathsampling.Trajectory + the trajectory to save + filename : str + the name of the file to save to + """ + raise NotImplementedError() + +class SimStoreTrajectoryWriter(TrajectoryWriter): + """Trajectory writer that uses the OpenPathSampling storage format. + + This is the default trajectory writer, since all engines should be able + to use it. + """ + def _write(self, trajectory, filename): + # TODO: maybe error if not monkey-patched? + from openpathsampling.experimental.storage import Storage + storage = Storage(filename, mode='w') + storage.save(trajectory) diff --git a/openpathsampling/tests/exports/trajectories/test_core.py b/openpathsampling/tests/exports/trajectories/test_core.py new file mode 100644 index 000000000..bb255234b --- /dev/null +++ b/openpathsampling/tests/exports/trajectories/test_core.py @@ -0,0 +1,117 @@ +from openpathsampling.exports.trajectories import * + +from openpathsampling.tests.test_helpers import ( + data_filename, make_1d_traj +) +import openpathsampling as paths + +import pytest +import pathlib + + +@pytest.fixture +def _ad_gmx_engine(): + engine = paths.engines.gromacs.Engine( + gro="conf.gro", + mdp="md.mdp", + top="topol.top", + options = { + 'mdrun_args': '-nt 1', + 'grompp_args': '-maxwarn 2', + }, + base_dir=data_filename("gromacs_engine"), + prefix="project" + ) + return engine + + +@pytest.fixture +def ad_trajpath(): + test_dir = pathlib.Path(data_filename("gromacs_engine")) + trajfile = test_dir / "project_trr/0000000.trr" + return trajfile + + +@pytest.fixture +def ad_trajectory(ad_trajpath, _ad_gmx_engine): + traj = paths.Trajectory([ + _ad_gmx_engine.read_frame_from_file(str(ad_trajpath), i) + for i in [0, 1, 2] + ]) + return traj + + +@pytest.fixture +def toy_trajectory(): + return make_1d_traj([1.0, 2.0, 3.0]) + + +class TrajectoryWriterTestBase: + def _read_trajectory(self, filename): + raise NotImplementedError() + + def _test_trajectory(self, trajectory, outfile): + assert not outfile.exists() + self.writer(trajectory, outfile) + assert outfile.exists() + + traj = self._read_trajectory(outfile) + assert len(traj) == len(trajectory) + assert traj == trajectory + + def _test_call_outfile_exists(self, trajectory, outfile): + outfile.touch() + with pytest.raises(FileExistsError): + self.writer(trajectory, outfile) + + def _test_call_outfile_exists_force(self, trajectory, outfile): + outfile.touch() + assert outfile.exists() + assert len(outfile.read_bytes()) == 0 + self.writer(trajectory, outfile, force=True) + assert outfile.exists() + assert len(outfile.read_bytes()) > 0 + + def test_call(self, trajectory_fixture, request, tmp_path): + raise NotImplementedError() + + def test_call_outfile_exists(self, request, tmp_path): + raise NotImplementedError() + + def test_call_outfile_exists_force(self, request, tmp_path): + raise NotImplementedError() + + +class TestSimStoreTrajectoryWriter(TrajectoryWriterTestBase): + def setup_method(self): + self.writer = SimStoreTrajectoryWriter() + + def _read_trajectory(self, filename): + from openpathsampling.experimental.storage import Storage + storage = Storage(filename, mode='r') + return storage.trajectories[0] + + @pytest.mark.parametrize("trajectory_fixture", [ + "ad_trajectory", + "toy_trajectory", + ]) + def test_call(self, trajectory_fixture, request, tmp_path): + # monkey patch for SimStore + trajectory = request.getfixturevalue(trajectory_fixture) + import openpathsampling as paths + from openpathsampling.experimental.storage import monkey_patches + paths = monkey_patches.monkey_patch_all(paths) + + outfile = tmp_path / f"{trajectory_fixture}.nc" + self._test_trajectory(trajectory, outfile) + + # undo the monkey patch + monkey_patches.unpatch(paths) + + def test_call_outfile_exists(self, request, tmp_path): + trajectory = request.getfixturevalue("ad_trajectory") + self._test_call_outfile_exists(trajectory, tmp_path / "test.db") + + def test_call_outfile_exists_force(self, request, tmp_path): + trajectory = request.getfixturevalue("ad_trajectory") + self._test_call_outfile_exists_force(trajectory, tmp_path / "test.db") From d758c24f3fd22db4e14e211dfe3ce19a005d0891 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 10:04:31 -0500 Subject: [PATCH 15/70] fix old name --- openpathsampling/exports/trajectories/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index c2894e7be..f78ca0108 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -1,7 +1,7 @@ import pathlib import contextlib import logging -from openpathsampling.integrations import HAS_MDTRAJ +from openpathsampling.integration_tools import HAS_MDTRAJ import numpy as np From e7a8e083f2ed3f78a7beb0c66e56ca41a88cc636 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 10:15:59 -0500 Subject: [PATCH 16/70] Try to error if SimStore isn't patched --- openpathsampling/exports/trajectories/core.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index f78ca0108..710ca7a98 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -47,5 +47,11 @@ class SimStoreTrajectoryWriter(TrajectoryWriter): def _write(self, trajectory, filename): # TODO: maybe error if not monkey-patched? from openpathsampling.experimental.storage import Storage + from openpathsampling.experimental.storage.monkey_patch import ( + _IS_PATCHED_SAVING + ) + if not _IS_PATCHED_SAVING: + raise RuntimeError("SimStoreTrajectoryWriter requires the " + "monkey-patch to be active") storage = Storage(filename, mode='w') storage.save(trajectory) From 5692fdf8fefd4ba630faaa01c71f86063f72120f Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 23 Dec 2024 11:01:38 -0500 Subject: [PATCH 17/70] fix import name --- openpathsampling/engines/dynamics_engine.py | 25 +++++++++++++++++++ openpathsampling/exports/trajectories/core.py | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/openpathsampling/engines/dynamics_engine.py b/openpathsampling/engines/dynamics_engine.py index e77c123fb..c8a50134e 100644 --- a/openpathsampling/engines/dynamics_engine.py +++ b/openpathsampling/engines/dynamics_engine.py @@ -321,6 +321,31 @@ def set_as_default(self): import openpathsampling as p p.EngineMover.engine = self + def _default_trajectory_writer(self): + """Subclasses should override this to provide a default writer + suitable for their engine. + + By default, we use the :class:`.SimStoreTrajectoryWriter`, because + it should be able to handle any OPS objects. + """ + return SimStoreTrajectoryWriter() + + def export_trajectory(self, trajectory, filename, *, writer=None, + force=False): + """Export a trajectory to a file + + Parameters + ---------- + trajectory : :class:`.Trajectory` + the trajectory to export + filename : str + the name of the file to which to export the trajectory + """ + if writer is None: + writer = self._default_trajectory_writer() + + writer.write(trajectory, filename, force=force) + @property def default_options(self): default_options = {} diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index 710ca7a98..1f0810fad 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -47,7 +47,7 @@ class SimStoreTrajectoryWriter(TrajectoryWriter): def _write(self, trajectory, filename): # TODO: maybe error if not monkey-patched? from openpathsampling.experimental.storage import Storage - from openpathsampling.experimental.storage.monkey_patch import ( + from openpathsampling.experimental.storage.monkey_patches import ( _IS_PATCHED_SAVING ) if not _IS_PATCHED_SAVING: From 2a32a8532f547093461766562719b30af9e61fc4 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 24 Dec 2024 10:02:18 -0500 Subject: [PATCH 18/70] Updates for the trajectory export core tests All SimStore exports should use patching; all should ensure an unpatch --- .../tests/exports/trajectories/test_core.py | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/openpathsampling/tests/exports/trajectories/test_core.py b/openpathsampling/tests/exports/trajectories/test_core.py index bb255234b..0e774dfd5 100644 --- a/openpathsampling/tests/exports/trajectories/test_core.py +++ b/openpathsampling/tests/exports/trajectories/test_core.py @@ -102,16 +102,31 @@ def test_call(self, trajectory_fixture, request, tmp_path): from openpathsampling.experimental.storage import monkey_patches paths = monkey_patches.monkey_patch_all(paths) - outfile = tmp_path / f"{trajectory_fixture}.nc" - self._test_trajectory(trajectory, outfile) - - # undo the monkey patch - monkey_patches.unpatch(paths) + try: + outfile = tmp_path / f"{trajectory_fixture}.nc" + self._test_trajectory(trajectory, outfile) + finally: + # undo the monkey patch + monkey_patches.unpatch(paths) def test_call_outfile_exists(self, request, tmp_path): trajectory = request.getfixturevalue("ad_trajectory") - self._test_call_outfile_exists(trajectory, tmp_path / "test.db") + import openpathsampling as paths + from openpathsampling.experimental.storage import monkey_patches + paths = monkey_patches.monkey_patch_all(paths) + + try: + self._test_call_outfile_exists(trajectory, tmp_path / "test.db") + finally: + monkey_patches.unpatch(paths) def test_call_outfile_exists_force(self, request, tmp_path): trajectory = request.getfixturevalue("ad_trajectory") - self._test_call_outfile_exists_force(trajectory, tmp_path / "test.db") + import openpathsampling as paths + from openpathsampling.experimental.storage import monkey_patches + paths = monkey_patches.monkey_patch_all(paths) + try: + self._test_call_outfile_exists_force(trajectory, + tmp_path / "test.db") + finally: + monkey_patches.unpatch(paths) From 9c96789058439cc4a1c23a15d26f08cf19effba9 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 24 Dec 2024 10:04:10 -0500 Subject: [PATCH 19/70] Test for deprecation can assume it hasn't been run --- openpathsampling/tests/test_external_engine.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openpathsampling/tests/test_external_engine.py b/openpathsampling/tests/test_external_engine.py index 8f498ccbe..7e3edf84f 100644 --- a/openpathsampling/tests/test_external_engine.py +++ b/openpathsampling/tests/test_external_engine.py @@ -138,6 +138,7 @@ def setup_method(self): self.ensemble = paths.LengthEnsemble(5) def test_deprecation(self): + NEW_DEFAULT_FILENAME_SETTER.has_warned = False slow_options = {'n_frames_max': 10000, 'engine_sleep': 100, 'name_prefix': "test", From 9bc4a23471997e348b022a6cdb4690777e859541 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 24 Dec 2024 10:04:52 -0500 Subject: [PATCH 20/70] Fix problems related to unpatching We would have different objects loaded for `FunctionCV` and would get results errors claiming that we were calling `super()` with an instance that wasn't a subclass. --- openpathsampling/tests/test_pathmover.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/openpathsampling/tests/test_pathmover.py b/openpathsampling/tests/test_pathmover.py index afda5fcbd..389c95fe5 100644 --- a/openpathsampling/tests/test_pathmover.py +++ b/openpathsampling/tests/test_pathmover.py @@ -14,7 +14,6 @@ import pytest import openpathsampling as paths -from openpathsampling.collectivevariable import FunctionCV from openpathsampling.engines.trajectory import Trajectory from openpathsampling.ensemble import LengthEnsemble from openpathsampling.pathmover import * @@ -128,7 +127,7 @@ class TestShootingMover(object): def setup_method(self): self.dyn = CalvinistDynamics([-0.1, 0.1, 0.3, 0.5, 0.7, -0.1, 0.2, 0.4, 0.6, 0.8]) - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) self.stateA = CVDefinedVolume(op, -100, 0.0) self.stateB = CVDefinedVolume(op, 0.65, 100) self.tps = A2BEnsemble(self.stateA, self.stateB) @@ -509,7 +508,7 @@ def test_to_dict_from_dict(self): class TestPathReversalMover(object): def setup_method(self): - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) volA = CVDefinedVolume(op, -100, 0.0) volB = CVDefinedVolume(op, 1.0, 100) @@ -576,7 +575,7 @@ def test_replica_not_in_sample_set(self): class TestReplicaExchangeMover(object): def setup_method(self): - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) state1 = CVDefinedVolume(op, -100, 0.0) state2 = CVDefinedVolume(op, 1, 100) @@ -724,7 +723,7 @@ def setup_method(self): ]) self.dyn.initialized = True # SampleMover.engine = self.dyn - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) stateA = CVDefinedVolume(op, -100, 0.0) stateB = CVDefinedVolume(op, 0.65, 100) volX = CVDefinedVolume(op, -100, 0.25) @@ -1176,7 +1175,7 @@ def test_move(self): class TestMinusMover(object): def setup_method(self): - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) volA = CVDefinedVolume(op, -100, 0.0) volB = CVDefinedVolume(op, 1.0, 100) volX = CVDefinedVolume(op, -100, 0.25) @@ -1410,7 +1409,7 @@ def test_extension_fails(self): class TestSingleReplicaMinusMover(object): def setup_method(self): - op = FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) + op = paths.FunctionCV("myid", f=lambda snap: snap.coordinates[0][0]) volA = CVDefinedVolume(op, -100, 0.0) volB = CVDefinedVolume(op, 1.0, 100) volX = CVDefinedVolume(op, -100, 0.25) From 5bca863b98a4a93b70dee4042278c23dd638476d Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 24 Dec 2024 10:07:47 -0500 Subject: [PATCH 21/70] Fix more problems related to unpatching (PLUMEDCV) Again, we seemed to have the issue that Python 2-style super calls get mixed up because the explicit class imported in the file is no longer seen as the actual superclass. However, switching to Python 3-style super() fixes (but must be done throughout the inheritance stack. --- openpathsampling/collectivevariable.py | 13 +++++++------ .../collectivevariables/plumed_wrapper.py | 7 ++++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/openpathsampling/collectivevariable.py b/openpathsampling/collectivevariable.py index f4cbc5ad5..d523dfa67 100644 --- a/openpathsampling/collectivevariable.py +++ b/openpathsampling/collectivevariable.py @@ -57,7 +57,7 @@ def __init__( name, cv_time_reversible=False ): - super(CollectiveVariable, self).__init__(name, paths.BaseSnapshot) + super().__init__(name, paths.BaseSnapshot) self.cv_time_reversible = cv_time_reversible self.diskcache_allow_incomplete = not self.cv_time_reversible @@ -207,7 +207,7 @@ def __init__( ``mdtraj`` """ - super(CallableCV, self).__init__( + super().__init__( name, cv_time_reversible=cv_time_reversible ) @@ -323,7 +323,7 @@ def __init__( """ - super(FunctionCV, self).__init__( + super().__init__( name, cv_callable=f, cv_time_reversible=cv_time_reversible, @@ -374,10 +374,11 @@ def __init__( :class:`openpathsampling.collectivevariable.CallableCV` """ - - super(FunctionCV, self).__init__( + # not sure why this used to skip the parent init; all it does is + # rename anyway + super().__init__( name, - cv_callable=f, + f=f, cv_time_reversible=True, cv_requires_lists=cv_requires_lists, cv_wrap_numpy_array=cv_wrap_numpy_array, diff --git a/openpathsampling/collectivevariables/plumed_wrapper.py b/openpathsampling/collectivevariables/plumed_wrapper.py index 85e700f81..cee69249e 100755 --- a/openpathsampling/collectivevariables/plumed_wrapper.py +++ b/openpathsampling/collectivevariables/plumed_wrapper.py @@ -71,7 +71,7 @@ def __init__(self, kwargs """ - super(PLUMEDCV, self).__init__( + super().__init__( name, f=PLUMEDCV.compute_cv, cv_requires_lists=cv_requires_lists, @@ -185,6 +185,11 @@ def to_dict(self): 'cv_scalarize_numpy_singletons': self.cv_scalarize_numpy_singletons } + @classmethod + def from_dict(cls, dct): + return cls(**dct) + + class PLUMEDInterface(StorableNamedObject): From df461a380f14c90504b6191172f309304038960a Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 25 Dec 2024 07:22:27 -0500 Subject: [PATCH 22/70] tests for DynamicsEngine.export_trajectory --- openpathsampling/engines/dynamics_engine.py | 3 +- openpathsampling/tests/test_dynamicsengine.py | 94 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/openpathsampling/engines/dynamics_engine.py b/openpathsampling/engines/dynamics_engine.py index c8a50134e..dbff93828 100644 --- a/openpathsampling/engines/dynamics_engine.py +++ b/openpathsampling/engines/dynamics_engine.py @@ -10,6 +10,7 @@ from openpathsampling.netcdfplus import StorableNamedObject from openpathsampling.integration_tools import is_simtk_unit_type +from openpathsampling.exports.trajectories import SimStoreTrajectoryWriter from .snapshot import BaseSnapshot from .trajectory import Trajectory @@ -344,7 +345,7 @@ def export_trajectory(self, trajectory, filename, *, writer=None, if writer is None: writer = self._default_trajectory_writer() - writer.write(trajectory, filename, force=force) + writer(trajectory, filename, force=force) @property def default_options(self): diff --git a/openpathsampling/tests/test_dynamicsengine.py b/openpathsampling/tests/test_dynamicsengine.py index cf762ab21..0b8b136a8 100644 --- a/openpathsampling/tests/test_dynamicsengine.py +++ b/openpathsampling/tests/test_dynamicsengine.py @@ -107,3 +107,97 @@ def test_generate_multiple_running_conditions(self, stoppable): # doesn't work traj = self.stupid.generate(init_snap, conditions) assert len(traj) == 2 + + def test_export_trajectory(self, tmp_path): + global paths + outfile = tmp_path / "test_traj.db" + init_snap = make_1d_traj([0.0])[0] + continue_condition = paths.LengthEnsemble(5).can_append + traj = self.stupid.generate(init_snap, [continue_condition]) + assert len(traj) == 5 + assert not outfile.exists() + from openpathsampling.experimental.storage import ( + monkey_patches, Storage + ) + paths = monkey_patches.monkey_patch_all(paths) + try: + self.stupid.export_trajectory(traj, outfile) + assert outfile.exists() + storage = Storage(outfile, mode='r') + assert len(storage.trajectories) == 1 + reloaded = storage.trajectories[0] + assert len(reloaded) == 5 + assert reloaded == traj + finally: + monkey_patches.unpatch(paths) + + + def test_export_trajectory_force(self, tmp_path): + global paths + outfile = tmp_path / "test_traj.db" + outfile.touch() + init_snap = make_1d_traj([0.0])[0] + continue_condition = paths.LengthEnsemble(5).can_append + traj = self.stupid.generate(init_snap, [continue_condition]) + assert len(traj) == 5 + assert outfile.exists() + from openpathsampling.experimental.storage import ( + monkey_patches, Storage + ) + paths = monkey_patches.monkey_patch_all(paths) + + try: + self.stupid.export_trajectory(traj, outfile, force=True) + assert outfile.exists() + storage = Storage(outfile, mode='r') + assert len(storage.trajectories) == 1 + reloaded = storage.trajectories[0] + assert len(reloaded) == 5 + assert reloaded == traj + finally: + monkey_patches.unpatch(paths) + + def test_export_trajecory_file_exists_fail(self, tmp_path): + global paths + outfile = tmp_path / "test_traj.db" + outfile.touch() + init_snap = make_1d_traj([0.0])[0] + continue_condition = paths.LengthEnsemble(5).can_append + traj = self.stupid.generate(init_snap, [continue_condition]) + assert len(traj) == 5 + assert outfile.exists() + from openpathsampling.experimental.storage import ( + monkey_patches, Storage + ) + paths = monkey_patches.monkey_patch_all(paths) + + try: + with pytest.raises(FileExistsError): + self.stupid.export_trajectory(traj, outfile) + finally: + monkey_patches.unpatch(paths) + + def test_export_trajectory_custom_writer(self, tmp_path): + # here we use a custom writer that will write to a string + from openpathsampling.exports.trajectories.core import ( + TrajectoryWriter + ) + outfile = tmp_path / "test_traj.txt" + class StringXWriter(TrajectoryWriter): + def _write(self, trajectory, filename): + outstring = " ".join([ + f"{snap.xyz[0][0]:.2f}" for snap in trajectory + ]) + with open(filename, 'w') as f: + f.write(outstring) + + init_snap = make_1d_traj([0.0])[0] + continue_condition = paths.LengthEnsemble(5).can_append + traj = self.stupid.generate(init_snap, [continue_condition]) + assert len(traj) == 5 + writer = StringXWriter() + self.stupid.export_trajectory(traj, outfile, writer=writer) + with open(outfile, 'r') as f: + outstring = f.read() + + assert outstring == ("0.00 " * 5)[:-1] From c5985313968760cbdd85c79b56eff44f5b7e84d7 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 25 Dec 2024 07:52:55 -0500 Subject: [PATCH 23/70] Add docs for engines_trajectory_export --- docs/developers/engines.rst | 1 + docs/developers/engines_trajectory_export.rst | 24 +++++++++++++++++++ openpathsampling/exports/trajectories/core.py | 1 - 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 docs/developers/engines_trajectory_export.rst diff --git a/docs/developers/engines.rst b/docs/developers/engines.rst index b3312f441..0a92f3a09 100644 --- a/docs/developers/engines.rst +++ b/docs/developers/engines.rst @@ -29,4 +29,5 @@ implement engines that do not only deal with standard molecular dynamics. snapshot_features engines_direct_api engines_indirect_api + engines_trajectory_export advanced_engines diff --git a/docs/developers/engines_trajectory_export.rst b/docs/developers/engines_trajectory_export.rst new file mode 100644 index 000000000..cb6c38d8f --- /dev/null +++ b/docs/developers/engines_trajectory_export.rst @@ -0,0 +1,24 @@ +Trajectory Export from Engines +============================== + +Users frequently want trajectories to be exported from Python into format +that they can analyze with other tools. Since the expected trajectory format +can differ between engines, a developer can set a default trajectory writer +using the :meth:`._default_trajectory_writer` method. This method should +return a callable that takes a trajectory, a filename, and the boolean flag +``force``, where the ``force`` flag forces overwrite of an existing file (a +``FileExistsError`` should be raised if the file exists and ``force`` is +``False``. Note that the default writer should be lossless: output from this +should be sufficient to perform an MD restart for, e.g., a shooting move. + +We recommend creating this callable as a subclass for the +:class:`.TrajectoryWriter` class. To implement this, you only need to +implement the :meth:`.TrajectoryWriter._write` method, which takes the +trajectory and the filename to write to as arguments. Additional information +that can't be extracted from the trajectory can be passed in the class +``__init__`` method. + +If you do not implement :meth:`._default_trajectory_writer`, you will use +the default from the base class, which creates a SimStore file for the +trajectory. OPS should be able to read/write to SimStore for snapshots from +any engine, so this will work, although it may not be convenient to users. diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index 1f0810fad..961578040 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -45,7 +45,6 @@ class SimStoreTrajectoryWriter(TrajectoryWriter): to use it. """ def _write(self, trajectory, filename): - # TODO: maybe error if not monkey-patched? from openpathsampling.experimental.storage import Storage from openpathsampling.experimental.storage.monkey_patches import ( _IS_PATCHED_SAVING From 6698b0ee9e3c7802b198086d46ce067d734dadae Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 25 Dec 2024 07:55:17 -0500 Subject: [PATCH 24/70] Add test to cover missing monkey patch --- openpathsampling/tests/exports/trajectories/test_core.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openpathsampling/tests/exports/trajectories/test_core.py b/openpathsampling/tests/exports/trajectories/test_core.py index 0e774dfd5..daa789831 100644 --- a/openpathsampling/tests/exports/trajectories/test_core.py +++ b/openpathsampling/tests/exports/trajectories/test_core.py @@ -130,3 +130,8 @@ def test_call_outfile_exists_force(self, request, tmp_path): tmp_path / "test.db") finally: monkey_patches.unpatch(paths) + + def test_call_not_patched_fail(self, request, tmp_path): + trajectory = request.getfixturevalue("ad_trajectory") + with pytest.raises(RuntimeError, match="monkey-patch"): + self.writer(trajectory, tmp_path / "test.db") From 00d63d245a26b39977ab614a3cc5a78e12bfe049 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 26 Dec 2024 09:53:55 -0500 Subject: [PATCH 25/70] Refactor testing to add more exporters --- openpathsampling/exports/trajectories/core.py | 1 - .../tests/exports/trajectories/conftest.py | 37 +++++++++++++++++++ .../tests/exports/trajectories/test_core.py | 32 ---------------- 3 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 openpathsampling/tests/exports/trajectories/conftest.py diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index 961578040..20d4b3642 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -1,7 +1,6 @@ import pathlib import contextlib import logging -from openpathsampling.integration_tools import HAS_MDTRAJ import numpy as np diff --git a/openpathsampling/tests/exports/trajectories/conftest.py b/openpathsampling/tests/exports/trajectories/conftest.py new file mode 100644 index 000000000..f855fad12 --- /dev/null +++ b/openpathsampling/tests/exports/trajectories/conftest.py @@ -0,0 +1,37 @@ +import pytest +import pathlib +from openpathsampling.tests.test_helpers import data_filename +import openpathsampling as paths + +@pytest.fixture +def ad_trajpath(): + test_dir = pathlib.Path(data_filename("gromacs_engine")) + trajfile = test_dir / "project_trr/0000000.trr" + return trajfile + +@pytest.fixture +def ad_grofile(): + test_dir = pathlib.Path(data_filename("gromacs_engine")) + topfile = test_dir / "conf.gro" + return str(topfile) + + +@pytest.fixture +def ad_trajectory(ad_trajpath): + engine = paths.engines.gromacs.Engine( + gro="conf.gro", + mdp="md.mdp", + top="topol.top", + options = { + 'mdrun_args': '-nt 1', + 'grompp_args': '-maxwarn 2', + }, + base_dir=data_filename("gromacs_engine"), + prefix="project" + ) + + traj = paths.Trajectory([ + engine.read_frame_from_file(str(ad_trajpath), i) + for i in [0, 1, 2] + ]) + return traj diff --git a/openpathsampling/tests/exports/trajectories/test_core.py b/openpathsampling/tests/exports/trajectories/test_core.py index daa789831..4e2825c92 100644 --- a/openpathsampling/tests/exports/trajectories/test_core.py +++ b/openpathsampling/tests/exports/trajectories/test_core.py @@ -9,38 +9,6 @@ import pathlib -@pytest.fixture -def _ad_gmx_engine(): - engine = paths.engines.gromacs.Engine( - gro="conf.gro", - mdp="md.mdp", - top="topol.top", - options = { - 'mdrun_args': '-nt 1', - 'grompp_args': '-maxwarn 2', - }, - base_dir=data_filename("gromacs_engine"), - prefix="project" - ) - return engine - - -@pytest.fixture -def ad_trajpath(): - test_dir = pathlib.Path(data_filename("gromacs_engine")) - trajfile = test_dir / "project_trr/0000000.trr" - return trajfile - - -@pytest.fixture -def ad_trajectory(ad_trajpath, _ad_gmx_engine): - traj = paths.Trajectory([ - _ad_gmx_engine.read_frame_from_file(str(ad_trajpath), i) - for i in [0, 1, 2] - ]) - return traj - - @pytest.fixture def toy_trajectory(): return make_1d_traj([1.0, 2.0, 3.0]) From 6db79277e24271546377474292ca8773ab566b23 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 26 Dec 2024 09:54:13 -0500 Subject: [PATCH 26/70] Add TRR trajectory writer (and tests) --- .../trajectories/trrtrajectorywriter.py | 26 ++++++++++++++++ .../trajectories/test_trrtrajectorywriter.py | 30 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 openpathsampling/exports/trajectories/trrtrajectorywriter.py create mode 100644 openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py diff --git a/openpathsampling/exports/trajectories/trrtrajectorywriter.py b/openpathsampling/exports/trajectories/trrtrajectorywriter.py new file mode 100644 index 000000000..2b5c5b27a --- /dev/null +++ b/openpathsampling/exports/trajectories/trrtrajectorywriter.py @@ -0,0 +1,26 @@ +from openpathsampling.integration_tools import HAS_MDTRAJ +import numpy as np +from .core import TrajectoryWriter + +class TRRTrajectoryWriter(TrajectoryWriter): + """Write a trajectory to a Gromacs TRR file. + + This inludes velocities, so it can be used for restarts. + """ + def __init__(self): + if not HAS_MDTRAJ: # -no-cov- + raise ImportError("MDTraj is not available") + + def _write(self, trajectory, filename): + # this uses some "unofficial" MDTraj API + import mdtraj as md + nframes = len(trajectory) + xyz = np.asarray(trajectory.xyz, dtype=np.float32) + time = np.asarray([0.0]*nframes, dtype=np.float32) + box = np.asarray(trajectory.box_vectors, dtype=np.float32) + lambd = np.asarray([0.0]*nframes, dtype=np.float32) + vel = np.asarray(trajectory.velocities, dtype=np.float32) + trr = md.formats.TRRTrajectoryFile(str(filename), 'w') + step = np.arange(0, nframes, dtype=np.int32) + trr._write(xyz, time, step, box, lambd, vel) + trr.close() diff --git a/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py b/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py new file mode 100644 index 000000000..db89b2391 --- /dev/null +++ b/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py @@ -0,0 +1,30 @@ +import pytest + +from openpathsampling.exports.trajectories.trrtrajectorywriter import * +from openpathsampling.integration_tools import HAS_MDTRAJ +import numpy.testing as npt + +def test_trr_trajectory_writer(ad_trajectory, tmp_path): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + import mdtraj as md + outfile = tmp_path / "test.trr" + writer = TRRTrajectoryWriter() + assert not outfile.exists() + writer(ad_trajectory, outfile) + assert outfile.exists() + + trr = md.formats.TRRTrajectoryFile(str(outfile), mode='r') + xyz, time, step, box, lambd, vel, forces = trr._read( + int(len(ad_trajectory)), + atom_indices=None, + get_velocities=True, + ) + + assert forces is None + npt.assert_allclose(xyz, ad_trajectory.xyz) + npt.assert_allclose(vel, ad_trajectory.velocities) + npt.assert_allclose(box, ad_trajectory.box_vectors) + npt.assert_allclose(lambd, np.zeros(len(ad_trajectory))) + npt.assert_allclose(time, np.zeros(len(ad_trajectory))) From 7f0a07dbb76f5bbbe395e183001e342690c2a58c Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 26 Dec 2024 09:54:32 -0500 Subject: [PATCH 27/70] Add MDTrajTrajectoryWriter (and tests) --- .../trajectories/mdtrajtrajectorywriter.py | 35 +++++++++++ .../test_mdtrajtrajectorywriter.py | 63 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py create mode 100644 openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py diff --git a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py new file mode 100644 index 000000000..9511929ed --- /dev/null +++ b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py @@ -0,0 +1,35 @@ +from .core import TrajectoryWriter +from openpathsampling.integration_tools import HAS_MDTRAJ +from collections.abc import Iterable + + +class MDTrajTrajectoryWriter(TrajectoryWriter): + """Generic save to MDTraj. + + Note that this will not include velocities, and therefore isn't suitable + for saving data that could be used in a restart. + """ + def __init__(self, mdtraj_selection=None): + if not HAS_MDTRAJ: # -no-cov- + raise ImportError("MDTraj is not available") + + self.mdtraj_selection = mdtraj_selection + + def _write(self, trajectory, filename): + mdt = trajectory.to_mdtraj() + + sel = None + if isinstance(self.mdtraj_selection, str): + sel = mdt.topology.select(self.mdtraj_selection) + elif isinstance(self.mdtraj_selection, Iterable): + sel = list(self.mdtraj_selection) + elif self.mdtraj_selection is None: + pass + else: + raise TypeError("mdtraj_selection must be a string or an " + f"iterable; got '{self.mdtraj_selection}'") + + if sel is not None: + mdt = mdt.atom_slice(sel) + + mdt.save(filename) diff --git a/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py new file mode 100644 index 000000000..ca9e99106 --- /dev/null +++ b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py @@ -0,0 +1,63 @@ +import pytest + +from openpathsampling.exports.trajectories.mdtrajtrajectorywriter import * +from openpathsampling.integration_tools import HAS_MDTRAJ +import numpy.testing as npt + +def test_mdtraj_trajectory_writer(ad_trajectory, ad_grofile, tmp_path): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + import mdtraj as md + outfile = tmp_path / "test.xtc" + writer = MDTrajTrajectoryWriter() + assert not outfile.exists() + writer(ad_trajectory, outfile) + assert outfile.exists() + + reloaded = md.load(str(outfile), top=ad_grofile) + assert len(reloaded) == len(ad_trajectory) + assert reloaded.xyz.shape == ad_trajectory.xyz.shape + npt.assert_allclose(reloaded.xyz, ad_trajectory.xyz, atol=1e-3) + + +@pytest.mark.parametrize("selection", [ + "backbone", + [4, 5, 6, 8, 14, 15, 16, 18] +]) +def test_subtrajectory_selection(ad_trajectory, selection, tmp_path): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + import mdtraj as md + outfile = tmp_path / "test.xtc" + writer = MDTrajTrajectoryWriter(mdtraj_selection=selection) + assert not outfile.exists() + writer(ad_trajectory, outfile) + assert outfile.exists() + + # save file for the modified topology + mdt = ad_trajectory.to_mdtraj() + if isinstance(selection, str): + subset = mdt.topology.select(selection) + else: + subset = selection + + subtraj = mdt.atom_slice(subset) + subgro = tmp_path / "subset.gro" + subtraj[0].save(str(subgro)) + + reloaded = md.load(str(outfile), top=subgro) + assert len(reloaded) == len(ad_trajectory) + assert reloaded.xyz.shape == subtraj.xyz.shape + assert reloaded.xyz.shape != ad_trajectory.xyz.shape + npt.assert_allclose(reloaded.xyz, subtraj.xyz, atol=1e-3) + + +def test_mdtraj_trajectory_writer_selection_error(ad_trajectory, tmp_path): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + writer = MDTrajTrajectoryWriter(mdtraj_selection=object()) + with pytest.raises(TypeError): + writer(ad_trajectory, tmp_path / "test.xtc") From d3871a7c2565434cd20d5e7f06c4bfe283cd2f23 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Fri, 3 Jan 2025 17:26:20 -0600 Subject: [PATCH 28/70] Add default export to OpenMM engine Also add some gitignore for pixi --- .gitignore | 4 ++++ openpathsampling/engines/openmm/engine.py | 4 ++++ .../exports/trajectories/__init__.py | 2 ++ openpathsampling/tests/test_openmm_engine.py | 19 +++++++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/.gitignore b/.gitignore index 607e931bb..a560fe5af 100644 --- a/.gitignore +++ b/.gitignore @@ -113,3 +113,7 @@ target/ # IPython .ipynb_checkpoints + +# pixi environments +.pixi +*.egg-info diff --git a/openpathsampling/engines/openmm/engine.py b/openpathsampling/engines/openmm/engine.py index 63209fb1f..ab9ebb750 100644 --- a/openpathsampling/engines/openmm/engine.py +++ b/openpathsampling/engines/openmm/engine.py @@ -5,6 +5,7 @@ from openpathsampling.engines import DynamicsEngine, SnapshotDescriptor from openpathsampling.engines.openmm import tools +from openpathsampling.exports.trajectories import TRRTrajectoryWriter from .snapshot import Snapshot import numpy as np @@ -257,6 +258,9 @@ def unload_context(self): del self._simulation.context self._simulation = None + def _default_trajectory_writer(self): + return TRRTrajectoryWriter() + def initialize(self, platform=None): """ Create the final OpenMMEngine diff --git a/openpathsampling/exports/trajectories/__init__.py b/openpathsampling/exports/trajectories/__init__.py index a1d92a6cc..dd0d3adcd 100644 --- a/openpathsampling/exports/trajectories/__init__.py +++ b/openpathsampling/exports/trajectories/__init__.py @@ -1 +1,3 @@ from .core import TrajectoryWriter, SimStoreTrajectoryWriter +from .trrtrajectorywriter import TRRTrajectoryWriter +from .mdtrajtrajectorywriter import MDTrajTrajectoryWriter diff --git a/openpathsampling/tests/test_openmm_engine.py b/openpathsampling/tests/test_openmm_engine.py index 177594779..ff6425e85 100644 --- a/openpathsampling/tests/test_openmm_engine.py +++ b/openpathsampling/tests/test_openmm_engine.py @@ -20,6 +20,8 @@ import openpathsampling.engines.openmm as peng import openpathsampling.engines as dyn +import numpy.testing as npt + import openpathsampling as paths from .test_helpers import ( @@ -277,3 +279,20 @@ def test_has_constraints(self, has_constraints): integrator=omt.integrators.VVVRIntegrator() ) assert engine.has_constraints() == has_constraints + + def test_export_trajectory(self, tmp_path): + filename = tmp_path / "test.trr" + assert not filename.exists() + traj = self.engine.generate(self.engine.current_snapshot, [ + paths.LengthEnsemble(3).can_append + ]) + self.engine.export_trajectory(traj, filename) + assert filename.exists() + + # reload the trajectory with MDTraj; check positions only + import mdtraj as md + topfile = data_filename("ala_small_traj.pdb") + reloaded = md.load(str(filename), top=str(topfile)) + assert len(reloaded) == len(traj) + npt.assert_allclose(reloaded.xyz, traj.xyz) + From ee1a8f787658cac8c9e2b7a176da84eed3d66be6 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Fri, 3 Jan 2025 18:08:16 -0600 Subject: [PATCH 29/70] Add support for GromacsEngine trajectory export --- openpathsampling/engines/gromacs/engine.py | 4 ++++ openpathsampling/tests/test_gromacs_engine.py | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/openpathsampling/engines/gromacs/engine.py b/openpathsampling/engines/gromacs/engine.py index 614b7d5b1..ad7c8fc9b 100644 --- a/openpathsampling/engines/gromacs/engine.py +++ b/openpathsampling/engines/gromacs/engine.py @@ -20,6 +20,7 @@ from openpathsampling.engines.external_snapshots import \ ExternalMDSnapshot, InternalizedMDSnapshot from openpathsampling.tools import ensure_file +from openpathsampling.exports.trajectories import TRRTrajectoryWriter import os import psutil @@ -186,6 +187,9 @@ def __init__(self, gro, mdp, top, options, base_dir="", prefix="gmx"): super(GromacsEngine, self).__init__(options, descriptor, template, first_frame_in_file=True) + def _default_trajectory_writer(self): + return TRRTrajectoryWriter() + def to_dict(self): dct = super(GromacsEngine, self).to_dict() local_dct = { diff --git a/openpathsampling/tests/test_gromacs_engine.py b/openpathsampling/tests/test_gromacs_engine.py index a1282278a..616cf351f 100644 --- a/openpathsampling/tests/test_gromacs_engine.py +++ b/openpathsampling/tests/test_gromacs_engine.py @@ -318,6 +318,29 @@ def test_serialization_cycle(self): reserialized = deserialized.to_dict() assert serialized == reserialized + def test_export_trajectory(self, tmp_path): + if not has_gmx: + pytest.skip("gmx not found. Skipping test.") + + if not HAS_MDTRAJ: + pytest.skip("MDTraj not found. Skipping test.") + + traj_0 = self.engine.trajectory_filename(0) + snap = self.engine.read_frame_from_file(traj_0, 0) + self.engine.filename_setter.reset(0) + + ens = paths.LengthEnsemble(5) + traj = self.engine.generate(snap, running=[ens.can_append]) + + filename = tmp_path / "test.trr" + assert not filename.exists() + self.engine.export_trajectory(traj, filename) + assert filename.exists() + + traj_md = md.load(str(filename), top=self.engine.gro) + assert len(traj_md) == 5 + npt.assert_allclose(traj_md.xyz, traj.xyz) + class TestGromacsExternalMDSnapshot(object): def setup_method(self): From f2e4897c3b492dcd697a04d85393768949cc4a77 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 6 Jan 2025 19:13:16 -0600 Subject: [PATCH 30/70] MDTrajTrajectoryWriter: Cache atom selection --- .../trajectories/mdtrajtrajectorywriter.py | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py index 9511929ed..564cadc34 100644 --- a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py +++ b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py @@ -14,22 +14,23 @@ def __init__(self, mdtraj_selection=None): raise ImportError("MDTraj is not available") self.mdtraj_selection = mdtraj_selection + self._sel = None # the actual atom indices of selection def _write(self, trajectory, filename): mdt = trajectory.to_mdtraj() - sel = None - if isinstance(self.mdtraj_selection, str): - sel = mdt.topology.select(self.mdtraj_selection) - elif isinstance(self.mdtraj_selection, Iterable): - sel = list(self.mdtraj_selection) - elif self.mdtraj_selection is None: - pass - else: - raise TypeError("mdtraj_selection must be a string or an " - f"iterable; got '{self.mdtraj_selection}'") - - if sel is not None: - mdt = mdt.atom_slice(sel) + if self._sel is None: + if self.mdtraj_selection is None: + pass + elif isinstance(self.mdtraj_selection, str): + self._sel = mdt.topology.select(self.mdtraj_selection) + elif isinstance(self.mdtraj_selection, Iterable): + self._sel = list(self.mdtraj_selection) + else: + raise TypeError("mdtraj_selection must be a string or an " + f"iterable; got '{self.mdtraj_selection}'") + + if self._sel is not None: + mdt = mdt.atom_slice(self._sel) mdt.save(filename) From 84c98d11d6195ce664d49553570123d561076bdf Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 13 Jan 2025 20:40:23 -0600 Subject: [PATCH 31/70] Revert "MDTrajTrajectoryWriter: Cache atom selection" This reverts commit f2e4897c3b492dcd697a04d85393768949cc4a77. --- .../trajectories/mdtrajtrajectorywriter.py | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py index 564cadc34..9511929ed 100644 --- a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py +++ b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py @@ -14,23 +14,22 @@ def __init__(self, mdtraj_selection=None): raise ImportError("MDTraj is not available") self.mdtraj_selection = mdtraj_selection - self._sel = None # the actual atom indices of selection def _write(self, trajectory, filename): mdt = trajectory.to_mdtraj() - if self._sel is None: - if self.mdtraj_selection is None: - pass - elif isinstance(self.mdtraj_selection, str): - self._sel = mdt.topology.select(self.mdtraj_selection) - elif isinstance(self.mdtraj_selection, Iterable): - self._sel = list(self.mdtraj_selection) - else: - raise TypeError("mdtraj_selection must be a string or an " - f"iterable; got '{self.mdtraj_selection}'") - - if self._sel is not None: - mdt = mdt.atom_slice(self._sel) + sel = None + if isinstance(self.mdtraj_selection, str): + sel = mdt.topology.select(self.mdtraj_selection) + elif isinstance(self.mdtraj_selection, Iterable): + sel = list(self.mdtraj_selection) + elif self.mdtraj_selection is None: + pass + else: + raise TypeError("mdtraj_selection must be a string or an " + f"iterable; got '{self.mdtraj_selection}'") + + if sel is not None: + mdt = mdt.atom_slice(sel) mdt.save(filename) From af18b4488d865fd6565069719947627ca2879465 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 13:26:46 -0500 Subject: [PATCH 32/70] Use `autorelease check` instead of customs script --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 864b2b4f7..a5ce9b9e6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -72,7 +72,7 @@ jobs: - name: "Versions" run: conda list - name: "Autorelease check" - run: python devtools/autorelease_check.py + run: autorelease check #- name: "DEBUG: enable SSH login" #uses: mxschmitt/action-tmate@v3 - name: "Unit Tests" From 842276085f79074a94d0ebd91b866479b84b648f Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 13:45:54 -0500 Subject: [PATCH 33/70] see if install procedure needs to change --- .github/workflows/tests.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a5ce9b9e6..4dba05680 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -57,15 +57,18 @@ jobs: if [ -z "$MINIMAL" ] ; then source devtools/conda_install_reqs.sh else - # In this case we're actually double-installing (not just - # installing requirements here). We prefer this to using the - # conda_install_reqs script so we can test deps coming from PyPI + # if we are in minimal mode, we do the full installation here python -m pip install -e .[test] fi python -m pip install autorelease - name: "Install" run: | - python -m pip install --no-deps -e . + if [ -n "$MINIMAL" ] ; then + # if we are not in minimal mode, we install the package here + python -m pip install --no-deps -e . + fi + - name: "Working directory" + run: pwd - name: "Check installation" run: | python -c "import openpathsampling; print(openpathsampling.version.full_version)" From 1616adefc6706bd1baeaf36acdab93130ba4a9a0 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 13:48:07 -0500 Subject: [PATCH 34/70] Add ls to pwd debugging --- .github/workflows/tests.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4dba05680..897b45701 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -68,7 +68,9 @@ jobs: python -m pip install --no-deps -e . fi - name: "Working directory" - run: pwd + run: | + pwd + ls -l - name: "Check installation" run: | python -c "import openpathsampling; print(openpathsampling.version.full_version)" From ee33481e771de3e0819f3b29fc0c6b4317abc9ef Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 14:23:59 -0500 Subject: [PATCH 35/70] Of course it can't find config that isn't committed! --- .autorelease/autorelease.yaml | 48 +++++++++++++++++++++++++++++++++++ .github/workflows/tests.yml | 2 +- 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 .autorelease/autorelease.yaml diff --git a/.autorelease/autorelease.yaml b/.autorelease/autorelease.yaml new file mode 100644 index 000000000..4f13f1221 --- /dev/null +++ b/.autorelease/autorelease.yaml @@ -0,0 +1,48 @@ +project: + repo_owner: openpathsampling + repo_name: openpathsampling + project_name: OpenPathSampling + +repo: + release-branches: + - stable + release-tag: "v{BASE_VERSION}" + # TODO: this format will not work long-term: need a dev branch per release + # branch + dev-branch: master + +release-check: + versions: + - setup-cfg + - getattr: openpathsampling.version.version + - getattr: openpathsampling.netcdfplus.version.version + +notes: + labels: + - label: feature + heading: New features + - label: enhancement + heading: Other enhancements + - label: experimental + heading: Experimental (beta) features + - label: bugfix + heading: Bugs fixed + - label: misc PR + heading: Miscellaneous improvements + topics: + - label: docs + name: Improvements to documentation + - label: code-style + name: Improvements to code readability and style + - label: internal + name: Behind-the-scenes improvements to CI, testing, and deployment + + allow-duplicates: + - label: experimental + show-as: + - experimental + + standard_contributors: + - dwhswenson + - jhprinz + - sroet diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 897b45701..bb770f135 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -77,7 +77,7 @@ jobs: - name: "Versions" run: conda list - name: "Autorelease check" - run: autorelease check + run: autorelease check --conf ./.autorelease/autorelease.yaml #- name: "DEBUG: enable SSH login" #uses: mxschmitt/action-tmate@v3 - name: "Unit Tests" From ba2e2ca357469f8afc5bd72f97dd347d7c9f4612 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 14:29:31 -0500 Subject: [PATCH 36/70] (partially) Revert "see if install procedure needs to change" This reverts commit 842276085f79074a94d0ebd91b866479b84b648f. --- .github/workflows/tests.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bb770f135..671852c17 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -57,16 +57,15 @@ jobs: if [ -z "$MINIMAL" ] ; then source devtools/conda_install_reqs.sh else - # if we are in minimal mode, we do the full installation here + # In this case we're actually double-installing (not just + # installing requirements here). We prefer this to using the + # conda_install_reqs script so we can test deps coming from PyPI python -m pip install -e .[test] fi python -m pip install autorelease - name: "Install" run: | - if [ -n "$MINIMAL" ] ; then - # if we are not in minimal mode, we install the package here - python -m pip install --no-deps -e . - fi + python -m pip install --no-deps -e . - name: "Working directory" run: | pwd From 47b9d9d4059a6b2ac9ba018faa003caa26b0d997 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 14:40:31 -0500 Subject: [PATCH 37/70] remove the old autorelease_check.py --- devtools/autorelease_check.py | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 devtools/autorelease_check.py diff --git a/devtools/autorelease_check.py b/devtools/autorelease_check.py deleted file mode 100644 index bc9b786da..000000000 --- a/devtools/autorelease_check.py +++ /dev/null @@ -1,29 +0,0 @@ -#/usr/bin/env python -from __future__ import print_function -import setup -import openpathsampling -from autorelease import DefaultCheckRunner, conda_recipe_version -from autorelease.version import get_setup_version -from packaging.version import Version - -repo_path = '.' -SETUP_VERSION = get_setup_version(None, directory='.') -versions = { - 'package': openpathsampling.version.version, - 'netcdfplus': openpathsampling.netcdfplus.version.version, - 'setup.py': SETUP_VERSION, -} - -RELEASE_BRANCHES = ['stable'] -RELEASE_TAG = "v" + Version(SETUP_VERSION).base_version - -if __name__ == "__main__": - checker = DefaultCheckRunner( - versions=versions, - setup=setup, - repo_path='.' - ) - checker.release_branches = RELEASE_BRANCHES + [RELEASE_TAG] - - tests = checker.select_tests() - n_fails = checker.run_as_test(tests) From 92f76414aec3f17f0fe5756f459c5539be41e08f Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 11 May 2025 15:21:21 -0500 Subject: [PATCH 38/70] Re-vendor actions for autorelease 0.6.1 --- .github/workflows/autorelease-default-env.sh | 4 ++-- .github/workflows/autorelease-deploy.yml | 12 ++++++------ .github/workflows/autorelease-gh-rel.yml | 8 ++++---- .github/workflows/autorelease-prep.yml | 17 +++++++++-------- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/.github/workflows/autorelease-default-env.sh b/.github/workflows/autorelease-default-env.sh index 4a09393a9..e6ffd17ea 100644 --- a/.github/workflows/autorelease-default-env.sh +++ b/.github/workflows/autorelease-default-env.sh @@ -1,6 +1,6 @@ -# Vendored from Autorelease 0.5.0 +# Vendored from Autorelease 0.6.1 # Update by updating Autorelease and running `autorelease vendor actions` -INSTALL_AUTORELEASE="python -m pip install autorelease==0.5.0" +INSTALL_AUTORELEASE="python -m pip install autorelease==0.6.1" if [ -f autorelease-env.sh ]; then source autorelease-env.sh fi diff --git a/.github/workflows/autorelease-deploy.yml b/.github/workflows/autorelease-deploy.yml index 73fca082a..44c789bfe 100644 --- a/.github/workflows/autorelease-deploy.yml +++ b/.github/workflows/autorelease-deploy.yml @@ -1,4 +1,4 @@ -# Vendored from Autorelease 0.5.0 +# Vendored from Autorelease 0.6.1 # Update by updating Autorelease and running `autorelease vendor actions` name: "Autorelease Deploy" on: @@ -10,9 +10,11 @@ jobs: if: ${{ github.repository == 'openpathsampling/openpathsampling' }} runs-on: ubuntu-latest name: "Deploy to PyPI" + permissions: + id-token: write steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: "3.x" - run: | # TODO: move this to an action @@ -33,8 +35,6 @@ jobs: python setup.py sdist bdist_wheel twine check dist/* name: "Build and check package" - - uses: pypa/gh-action-pypi-publish@master - with: - password: ${{ secrets.pypi_password }} + - uses: pypa/gh-action-pypi-publish@release/v1 name: "Deploy to pypi" diff --git a/.github/workflows/autorelease-gh-rel.yml b/.github/workflows/autorelease-gh-rel.yml index 8393d79fc..1e0a743b3 100644 --- a/.github/workflows/autorelease-gh-rel.yml +++ b/.github/workflows/autorelease-gh-rel.yml @@ -1,4 +1,4 @@ -# Vendored from Autorelease 0.5.0 +# Vendored from Autorelease 0.6.1 # Update by updating Autorelease and running `autorelease vendor actions` name: "Autorelease Release" on: @@ -13,10 +13,10 @@ jobs: runs-on: ubuntu-latest name: "Cut release" steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: - python-version: "3.7" + python-version: "3.x" - run: | # TODO: move this to an action source ./.github/workflows/autorelease-default-env.sh if [ -f "autorelease-env.sh" ]; then diff --git a/.github/workflows/autorelease-prep.yml b/.github/workflows/autorelease-prep.yml index d68853a03..3328704fb 100644 --- a/.github/workflows/autorelease-prep.yml +++ b/.github/workflows/autorelease-prep.yml @@ -1,4 +1,4 @@ -# Vendored from Autorelease 0.5.0 +# Vendored from Autorelease 0.6.1 # Update by updating Autorelease and running `autorelease vendor actions` name: "Autorelease testpypi" on: @@ -13,12 +13,14 @@ defaults: jobs: deploy_testpypi: + permissions: + id-token: write if: ${{ github.repository == 'openpathsampling/openpathsampling' }} runs-on: ubuntu-latest name: "Deployment test" steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: "3.x" - run: | # TODO: move this to an action @@ -43,9 +45,8 @@ jobs: python setup.py sdist bdist_wheel twine check dist/* name: "Build and check package" - - uses: pypa/gh-action-pypi-publish@master + - uses: pypa/gh-action-pypi-publish@release/v1 with: - password: ${{ secrets.testpypi_password }} repository_url: https://test.pypi.org/legacy/ name: "Deploy to testpypi" test_testpypi: @@ -54,10 +55,10 @@ jobs: name: "Test deployed" needs: deploy_testpypi steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: - python-version: "3.11" + python-version: "3.x" - run: | # TODO: move this to an action source ./.github/workflows/autorelease-default-env.sh if [ -f "autorelease-env.sh" ]; then From 0c97c5881dabed9c40de0f9e3abc02235414fe24 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Tue, 3 Jun 2025 16:47:16 -0500 Subject: [PATCH 39/70] fix for Python 3.13 hating features/base --- openpathsampling/engines/features/base.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openpathsampling/engines/features/base.py b/openpathsampling/engines/features/base.py index 8b74db407..97ed17617 100644 --- a/openpathsampling/engines/features/base.py +++ b/openpathsampling/engines/features/base.py @@ -51,8 +51,8 @@ def _register_function(cls, name, code, __features__): try: source_code = '\n'.join(code) cc = compile(source_code, '', 'exec') - #exec cc in locals() - exec_(cc, locals()) + namespace = {'np': np, 'cls': cls} + exec_(cc, namespace) if name not in cls.__dict__: if hasattr(cls, '__features__') and cls.__features__.debug[name] is None: @@ -61,7 +61,7 @@ def _register_function(cls, name, code, __features__): 'function is overridden again, otherwise some features might not be copied. ' 'The general practise of overriding is not recommended.') % name) - setattr(cls, name, locals()[name]) + setattr(cls, name, namespace[name]) __features__['debug'][name] = source_code From c99979c12bac03f40ed22a44bf455542a2b111aa Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 4 Jun 2025 17:54:12 -0500 Subject: [PATCH 40/70] Drop numpy pin --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f79c019be..febad0f66 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,7 +30,7 @@ python_requires = >=3.10 install_requires = future psutil - numpy<2.0 + numpy scipy pandas netcdf4 From 4e0833f8a584a53d70ecf9cbf82759e94a02d14e Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 4 Jun 2025 18:32:51 -0500 Subject: [PATCH 41/70] fix broken notebooks for numpy 2 --- examples/tests/test_cv.ipynb | 32 ++++++++++++++-------------- examples/tests/test_netcdfplus.ipynb | 20 +++++++++-------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/examples/tests/test_cv.ipynb b/examples/tests/test_cv.ipynb index 88f1b2747..b81ea4aff 100644 --- a/examples/tests/test_cv.ipynb +++ b/examples/tests/test_cv.ipynb @@ -115,7 +115,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "283860163354172696480456782077640048652\n" + "306465958118793424807025199580056649754\n" ] } ], @@ -141,7 +141,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "[(store.attributes[PseudoAttribute] : 4 object(s), 20, 283860163354172696480456782077640048654L), (store.attributes[PseudoAttribute] : 4 object(s), 20, 283860163354172696480456782077640048656L), (store.attributes[PseudoAttribute] : 4 object(s), 20, 283860163354172696480456782077640048658L), (store.attributes[PseudoAttribute] : 4 object(s), 20, 283860163354172696480456782077640048660L)]\n" + "[(store.attributes[PseudoAttribute] : 4 object(s), 20, 306465958118793424807025199580056649756), (store.attributes[PseudoAttribute] : 4 object(s), 20, 306465958118793424807025199580056649758), (store.attributes[PseudoAttribute] : 4 object(s), 20, 306465958118793424807025199580056649760), (store.attributes[PseudoAttribute] : 4 object(s), 20, 306465958118793424807025199580056649762)]\n" ] } ], @@ -159,7 +159,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "{283860163354172696480456782077640048656L: 1, 283860163354172696480456782077640048658L: 2, 283860163354172696480456782077640048660L: 3, 283860163354172696480456782077640048654L: 0}\n" + "{306465958118793424807025199580056649756: 0, 306465958118793424807025199580056649758: 1, 306465958118793424807025199580056649760: 2, 306465958118793424807025199580056649762: 3}\n" ] } ], @@ -267,7 +267,7 @@ "output_type": "stream", "text": [ "[10.0, 10.0]\n", - "[3.3921003341674805, 3.3921003341674805]\n" + "[np.float32(3.3921003), np.float32(3.3921003)]\n" ] } ], @@ -285,10 +285,10 @@ "name": "stdout", "output_type": "stream", "text": [ - "\n", - "\n", - "\n", - "\n" + "\n", + "\n", + "\n", + "\n" ] } ], @@ -309,10 +309,10 @@ "name": "stdout", "output_type": "stream", "text": [ - "[u'{\"_cls\":\"CollectiveVariable\",\"_dict\":{\"cv_time_reversible\":false,\"name\":\"func0\"}}'\n", - " u'{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func1\",\"f\":{\"_marshal\":\"YwMAAAADAAAAAwAAAEMAAQBzGwAAAHwCAGoAAHwAAGoBAGoCAGQBABmDAQB8AQAYUygCAAAATmkAAAAAKAMAAAB0AwAAAHN1bXQLAAAAY29vcmRpbmF0ZXN0BgAAAF92YWx1ZSgDAAAAdAgAAABzbmFwc2hvdHQGAAAAY2VudGVydAIAAABucCgAAAAAKAAAAABzHgAAADxpcHl0aG9uLWlucHV0LTUtNzlhNzEwNDZhYjU1PnQEAAAAZGlzdAIAAABzAgAAAAAB\",\"_module_vars\":[],\"_global_vars\":[]},\"cv_time_reversible\":false,\"cv_wrap_numpy_array\":false,\"cv_requires_lists\":false,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"np\":{\"_import\":\"numpy\"},\"center\":1}}}'\n", - " u'{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func2\",\"f\":{\"_marshal\":\"YwMAAAADAAAAAwAAAEMAAQBzGwAAAHwCAGoAAHwAAGoBAGoCAGQBABmDAQB8AQAYUygCAAAATmkAAAAAKAMAAAB0AwAAAHN1bXQLAAAAY29vcmRpbmF0ZXN0BgAAAF92YWx1ZSgDAAAAdAgAAABzbmFwc2hvdHQGAAAAY2VudGVydAIAAABucCgAAAAAKAAAAABzHgAAADxpcHl0aG9uLWlucHV0LTUtNzlhNzEwNDZhYjU1PnQEAAAAZGlzdAIAAABzAgAAAAAB\",\"_module_vars\":[],\"_global_vars\":[]},\"cv_time_reversible\":false,\"cv_wrap_numpy_array\":true,\"cv_requires_lists\":false,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"np\":{\"_import\":\"numpy\"},\"center\":1}}}'\n", - " u'{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func3\",\"f\":{\"_marshal\":\"YwMAAAADAAAAAwAAAEMAAQBzGwAAAHwCAGoAAHwAAGoBAGoCAGQBABmDAQB8AQAYUygCAAAATmkAAAAAKAMAAAB0AwAAAHN1bXQLAAAAY29vcmRpbmF0ZXN0BgAAAF92YWx1ZSgDAAAAdAgAAABzbmFwc2hvdHQGAAAAY2VudGVydAIAAABucCgAAAAAKAAAAABzHgAAADxpcHl0aG9uLWlucHV0LTUtNzlhNzEwNDZhYjU1PnQEAAAAZGlzdAIAAABzAgAAAAAB\",\"_module_vars\":[],\"_global_vars\":[]},\"cv_time_reversible\":true,\"cv_wrap_numpy_array\":true,\"cv_requires_lists\":false,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"np\":{\"_import\":\"numpy\"},\"center\":1}}}']\n" + "['{\"_cls\":\"CollectiveVariable\",\"_dict\":{\"name\":\"func0\",\"cv_time_reversible\":false}}'\n", + " '{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func1\",\"cv_time_reversible\":false,\"f\":{\"__callable_name__\":\"dist\",\"_dilled\":\"gASVpgEAAAAAAACMCmRpbGwuX2RpbGyUjBBfY3JlYXRlX2Z1bmN0aW9ulJOUKGgAjAxfY3JlYXRl\\\\nX2NvZGWUk5QoQwICAZRLA0sASwBLA0sESwNDUpcAfAKgAAAAAAAAAAAAAAAAAAAAAAAAAAAAfABq\\\\nAQAAAAAAAAAAagIAAAAAAAAAAGQBGQAAAAAAAAAAAKYBAACrAQAAAAAAAAAAfAF6CgAAUwCUTksA\\\\nhpSMA3N1bZSMC2Nvb3JkaW5hdGVzlIwGX3ZhbHVllIeUjAhzbmFwc2hvdJSMBmNlbnRlcpSMAm5w\\\\nlIeUjE4vdmFyL2ZvbGRlcnMvdmovMjhjMTA3NDk2c3E1ejR5MTByano1Z2RtMDAwMGduL1QvaXB5\\\\na2VybmVsXzk2Mjk5LzQxODQ5NTE0NDUucHmUjARkaXN0lGgRSwJDJIAA2AsNjzaKNpAo1BIm1BIt\\\\nqGHUEjDRCzHUCzGwRtELOtAEOpRDAJQpKXSUUpR9lIwIX19uYW1lX1+UjAhfX21haW5fX5RzaBFO\\\\nTnSUUpR9lH2UjA9fX2Fubm90YXRpb25zX1+UfZRzhpRiLg==\\\\n\"},\"cv_requires_lists\":false,\"cv_wrap_numpy_array\":false,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"center\":1,\"np\":{\"_import\":\"numpy\"}}}}'\n", + " '{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func2\",\"cv_time_reversible\":false,\"f\":{\"__callable_name__\":\"dist\",\"_dilled\":\"gASVpgEAAAAAAACMCmRpbGwuX2RpbGyUjBBfY3JlYXRlX2Z1bmN0aW9ulJOUKGgAjAxfY3JlYXRl\\\\nX2NvZGWUk5QoQwICAZRLA0sASwBLA0sESwNDUpcAfAKgAAAAAAAAAAAAAAAAAAAAAAAAAAAAfABq\\\\nAQAAAAAAAAAAagIAAAAAAAAAAGQBGQAAAAAAAAAAAKYBAACrAQAAAAAAAAAAfAF6CgAAUwCUTksA\\\\nhpSMA3N1bZSMC2Nvb3JkaW5hdGVzlIwGX3ZhbHVllIeUjAhzbmFwc2hvdJSMBmNlbnRlcpSMAm5w\\\\nlIeUjE4vdmFyL2ZvbGRlcnMvdmovMjhjMTA3NDk2c3E1ejR5MTByano1Z2RtMDAwMGduL1QvaXB5\\\\na2VybmVsXzk2Mjk5LzQxODQ5NTE0NDUucHmUjARkaXN0lGgRSwJDJIAA2AsNjzaKNpAo1BIm1BIt\\\\nqGHUEjDRCzHUCzGwRtELOtAEOpRDAJQpKXSUUpR9lIwIX19uYW1lX1+UjAhfX21haW5fX5RzaBFO\\\\nTnSUUpR9lH2UjA9fX2Fubm90YXRpb25zX1+UfZRzhpRiLg==\\\\n\"},\"cv_requires_lists\":false,\"cv_wrap_numpy_array\":true,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"center\":1,\"np\":{\"_import\":\"numpy\"}}}}'\n", + " '{\"_cls\":\"FunctionCV\",\"_dict\":{\"name\":\"func3\",\"cv_time_reversible\":true,\"f\":{\"__callable_name__\":\"dist\",\"_dilled\":\"gASVpgEAAAAAAACMCmRpbGwuX2RpbGyUjBBfY3JlYXRlX2Z1bmN0aW9ulJOUKGgAjAxfY3JlYXRl\\\\nX2NvZGWUk5QoQwICAZRLA0sASwBLA0sESwNDUpcAfAKgAAAAAAAAAAAAAAAAAAAAAAAAAAAAfABq\\\\nAQAAAAAAAAAAagIAAAAAAAAAAGQBGQAAAAAAAAAAAKYBAACrAQAAAAAAAAAAfAF6CgAAUwCUTksA\\\\nhpSMA3N1bZSMC2Nvb3JkaW5hdGVzlIwGX3ZhbHVllIeUjAhzbmFwc2hvdJSMBmNlbnRlcpSMAm5w\\\\nlIeUjE4vdmFyL2ZvbGRlcnMvdmovMjhjMTA3NDk2c3E1ejR5MTByano1Z2RtMDAwMGduL1QvaXB5\\\\na2VybmVsXzk2Mjk5LzQxODQ5NTE0NDUucHmUjARkaXN0lGgRSwJDJIAA2AsNjzaKNpAo1BIm1BIt\\\\nqGHUEjDRCzHUCzGwRtELOtAEOpRDAJQpKXSUUpR9lIwIX19uYW1lX1+UjAhfX21haW5fX5RzaBFO\\\\nTnSUUpR9lH2UjA9fX2Fubm90YXRpb25zX1+UfZRzhpRiLg==\\\\n\"},\"cv_requires_lists\":false,\"cv_wrap_numpy_array\":true,\"cv_scalarize_numpy_singletons\":false,\"kwargs\":{\"center\":1,\"np\":{\"_import\":\"numpy\"}}}}']\n" ] } ], @@ -374,7 +374,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "(store.trajectories[Trajectory] : 1 object(s), 0, 283860163354172696480456782077640048728L)\n" + "(store.trajectories[Trajectory] : 1 object(s), 0, 306465958118793424807025199580056649838)\n" ] } ], @@ -411,7 +411,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "283860163354172696480456782077640048730\n" + "306465958118793424807025199580056649840\n" ] } ], @@ -587,7 +587,7 @@ { "data": { "text/plain": [ - "" + "" ] }, "execution_count": 37, @@ -634,7 +634,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.7" + "version": "3.11.12" }, "toc": { "base_numbering": 1, diff --git a/examples/tests/test_netcdfplus.ipynb b/examples/tests/test_netcdfplus.ipynb index 0e3525edc..c3508cffb 100644 --- a/examples/tests/test_netcdfplus.ipynb +++ b/examples/tests/test_netcdfplus.ipynb @@ -810,7 +810,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "['{\"_hex_uuid\":\"0xe3d4c53a010011ef9d1f000000000026\",\"_store\":\"nodes\"}'\n", + "['{\"_hex_uuid\":\"0x9e916aba419a11f0bd9a000000000026\",\"_store\":\"nodes\"}'\n", " '{\"Hallo\":2,\"Test\":3}']\n" ] } @@ -969,9 +969,9 @@ "name": "stdout", "output_type": "stream", "text": [ - "[[['e3d4c53a-0100-11ef-9d1f-00000000002c'\n", - " 'e3d4c53a-0100-11ef-9d1f-000000000030']\n", - " ['e3d4c53a-0100-11ef-9d1f-00000000002e' '']]\n", + "[[['9e916aba-419a-11f0-bd9a-00000000002c'\n", + " '9e916aba-419a-11f0-bd9a-000000000030']\n", + " ['9e916aba-419a-11f0-bd9a-00000000002e' '']]\n", "\n", " [['' '']\n", " ['' '']]]\n", @@ -1049,7 +1049,7 @@ "text": [ "Type: \n", "Class: \n", - "Content: {'__uuid__': 302839522207420201522962921396994310194, 'value': 'First'}\n", + "Content: {'__uuid__': 210773071070663257590889269553700798514, 'value': 'First'}\n", "Access: First\n" ] } @@ -1817,7 +1817,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "[ {\"_hex_uuid\":\"0xe3d4c53a010011ef9d1f00000000004a\",\"_store\":\"nodesunique\"}, {\"_hex_uuid\":\"0xe3d4c53a010011ef9d1f00000000004c\",\"_store\":\"nodesunique\"}, {\"_hex_uuid\":\"0xe3d4c53a010011ef9d1f00000000004e\",\"_store\":\"nodesunique\"} ]\n" + "[ {\"_hex_uuid\":\"0x9e916aba419a11f0bd9a00000000004a\",\"_store\":\"nodesunique\"}, {\"_hex_uuid\":\"0x9e916aba419a11f0bd9a00000000004c\",\"_store\":\"nodesunique\"}, {\"_hex_uuid\":\"0x9e916aba419a11f0bd9a00000000004e\",\"_store\":\"nodesunique\"} ]\n" ] } ], @@ -1965,11 +1965,13 @@ "output_type": "stream", "text": [ "# We had an exception\n", - "invalid literal for int() with base 10: 'test'\n" + "invalid literal for int() with base 10: np.str_('test')\n" ] } ], "source": [ + "# NBVAL_IGNORE_OUTPUT\n", + "# until we drop numpy 1.x support -- output differs for np.str_('test') vs. 'test'\n", "try:\n", " a = Node('test')\n", " print(st.varstore.save(a))\n", @@ -2092,7 +2094,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "0xe3d4c53a010011ef9d1f000000000014\n" + "0x9e916aba419a11f0bd9a000000000014\n" ] } ], @@ -2150,7 +2152,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.7" + "version": "3.11.12" }, "toc": { "base_numbering": 1, From ba5e76e1dd4b243d2ebb5f12ccbc4c063fbee24d Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 4 Jun 2025 19:00:29 -0500 Subject: [PATCH 42/70] ignore openmmtools line that spits stderr in some cases --- examples/tests/test_openmm_integration.ipynb | 30 +++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/examples/tests/test_openmm_integration.ipynb b/examples/tests/test_openmm_integration.ipynb index b6a43d34d..f3d1e857a 100644 --- a/examples/tests/test_openmm_integration.ipynb +++ b/examples/tests/test_openmm_integration.ipynb @@ -42,7 +42,27 @@ "cell_type": "code", "execution_count": 2, "metadata": {}, - "outputs": [], + "outputs": [ + { + "name": "stderr", + "output_type": "stream", + "text": [ + "WARNING:pymbar.timeseries:Warning on use of the timeseries module: If the inherent timescales of the system are long compared to those being analyzed, this statistical inefficiency may be an underestimate. The estimate presumes the use of many statistically independent samples. Tests should be performed to assess whether this condition is satisfied. Be cautious in the interpretation of the data.\n", + "WARNING:pymbar.mbar_solvers:\n", + "****** PyMBAR will use 64-bit JAX! *******\n", + "* JAX is currently set to 32-bit bitsize *\n", + "* which is its default. *\n", + "* *\n", + "* PyMBAR requires 64-bit mode and WILL *\n", + "* enable JAX's 64-bit mode when called. *\n", + "* *\n", + "* This MAY cause problems with other *\n", + "* Uses of JAX in the same code. *\n", + "******************************************\n", + "\n" + ] + } + ], "source": [ "# NBVAL_IGNORE_OUTPUT\n", "# hide stderr from openmmtools (apparently nbval still gets it)\n", @@ -66,6 +86,8 @@ "metadata": {}, "outputs": [], "source": [ + "# NBVAL_IGNORE_OUTPUT\n", + "# apparently we get some weird output from openmmtools on this, too\n", "testsystem = omt.testsystems.AlanineDipeptideVacuum()" ] }, @@ -475,9 +497,9 @@ "metadata": { "anaconda-cloud": {}, "kernelspec": { - "display_name": "dev", + "display_name": "Python 3 (ipykernel)", "language": "python", - "name": "dev" + "name": "python3" }, "language_info": { "codemirror_mode": { @@ -489,7 +511,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.9.16" + "version": "3.11.12" }, "toc": { "base_numbering": 1, From 89de6169047b2a638f819164bdcca0c2b4c75520 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 4 Jun 2025 21:48:55 -0500 Subject: [PATCH 43/70] SymLinkStepExporter Dump steps as files, organized by raw (real files) and trial and active (symlinks). Works in principle as of this commit, but need to try it out in reality. --- .../exports/steps/symlink_step_exporter.py | 186 ++++++++++ .../steps/test_symlink_step_exporter.py | 339 ++++++++++++++++++ 2 files changed, 525 insertions(+) create mode 100644 openpathsampling/exports/steps/symlink_step_exporter.py create mode 100644 openpathsampling/tests/exports/steps/test_symlink_step_exporter.py diff --git a/openpathsampling/exports/steps/symlink_step_exporter.py b/openpathsampling/exports/steps/symlink_step_exporter.py new file mode 100644 index 000000000..02b706bd6 --- /dev/null +++ b/openpathsampling/exports/steps/symlink_step_exporter.py @@ -0,0 +1,186 @@ +_DEFAULT_RAW_DATA_PATTERN = "raw_data/{sample.trajectory.__uuid__}.{ext}" +_DEFAULT_TRIAL_PATTERN = "{step.mccycle}/trials/{ensemble_id}.{ext}" +_DEFAULT_ACTIVE_PATTERN = "{step.mccycle}/active/{ensemble_id}.{ext}" + +import os +import collections +import pathlib + +class SymLinkStepExporter: + """Export steps as raw data and symlink to the raw data. + + In the patterns used for raw data, trials, and active data, the + following substitutions are available: + + - step: The step being exported. This can be used with, e.g., + step.mccycle to get the Monte Carlo cycle number. + - sample: The sample being exported. + - ensemble_id: The ensemble ID for the sample. This is the name of the + ensemble if it is set, or the UUID of the ensemble if the name is not + set. This behavior can be customized by subclassing this class and + overriding the :meth:`._get_ensemble_id` method. + - ext: The extension for the raw data files. + + These parameters can be further customized by subclassing this class and + overriding the :meth:`._substitution_dict` method. + + Parameters + ---------- + ext : str + The extension to use for the raw data files. + writer : Optional[Callable] + The TrajectoryWriter to use for exporting the raw data. If None, the + default writer for the most common engine in the step will be used. + raw_data_pattern : str + File pattern for the raw data files. + trial_pattern : str + File pattern for the trial data symlinks. + active_pattern : str + File pattern for the active data symlinks. + """ + def __init__( + self, + ext, + writer=None, + *, + base_dir='.', + # TODO: add in a base using storage handlers, default to cwd + raw_data_pattern=_DEFAULT_RAW_DATA_PATTERN, + trial_pattern=_DEFAULT_TRIAL_PATTERN, + active_pattern=_DEFAULT_ACTIVE_PATTERN, + ): + self.ext = ext + self.writer = writer + self.base_dir = pathlib.Path(base_dir) + self.raw_data_pattern = raw_data_pattern + self.trial_pattern = trial_pattern + self.active_pattern = active_pattern + + def _get_ensemble_id(self, sample): + """Get the ensemble ID (used in file names) for a sample. + """ + ensemble_id = sample.ensemble.name + if not ensemble_id: + ensemble_id = str(sample.ensemble.__uuid__) + return ensemble_id + + def _get_writer(self, sample): + """Get the TrajectoryWriter to use for a sample. + + If ``self.writer`` is set, it will be used. Otherwise, the most + common engine in the sample's trajectory will be used. + """ + if self.writer is not None: + writer = self.writer + else: + engines = collections.Counter([s.engine for s in + sample.trajectory]) + engine = engines.most_common(1)[0][0] + writer = engine.export_trajectory + return writer + + def _substitution_dict(self, step, sample): + writer = self._get_writer(sample) + ensemble_id = self._get_ensemble_id(sample) + return { + "step": step, + "sample": sample, + "ensemble_id": ensemble_id, + "ext": self.ext, + } + + def _export_sample_symlink(self, pattern, step, sample): + if pattern is None: + return + + subs_dict = self._substitution_dict(step, sample) + path = pattern.format(**subs_dict) + raw_data_path = self.raw_data_pattern.format(**subs_dict) + if not pathlib.Path(raw_data_path).exists(): + self.export_raw_sample(step, sample) + + # ensure parent directory exists + pathlib.Path(path).parent.mkdir(parents=True, exist_ok=True) + + # Calculate relative path from symlink location to target + symlink_path = pathlib.Path(path) + target_path = pathlib.Path(raw_data_path) + relative_target = os.path.relpath(target_path, symlink_path.parent) + + # Skip if symlink already exists + if not symlink_path.exists(): + os.symlink(relative_target, path) + + def export_trial_sample(self, step, sample): + """Export a symlink to the raw data for a trial sample. + + Parameters + ---------- + step : Step + The step containing the sample. + sample : Sample + The trial sample to export. + """ + self._export_sample_symlink(self.trial_pattern, step, sample) + + def export_active_sample(self, step, sample): + """Export a symlink to the raw data for an active sample. + + Parameters + ---------- + step : Step + The step containing the sample. + sample : Sample + The active sample to export. + """ + self._export_sample_symlink(self.active_pattern, step, sample) + + def export_raw_sample(self, step, sample): + """Export the raw data for a sample. + + Parameters + ---------- + step : Step + The step containing the sample. + sample : Sample + The sample to export. + """ + subs_dict = self._substitution_dict(step, sample) + raw_data_path = self.raw_data_pattern.format(**subs_dict) + if os.path.exists(raw_data_path): + return + + # ensure parent directory exists + pathlib.Path(raw_data_path).parent.mkdir(parents=True, exist_ok=True) + writer = self._get_writer(sample) + writer(sample.trajectory, raw_data_path) + + def export_step(self, step): + """Export a step. + + Parameters + ---------- + step : Step + The step to export. + """ + for sample in step.change.trials: + self.export_raw_sample(step, sample) + self.export_trial_sample(step, sample) + + for sample in step.active: + self.export_active_sample(step, sample) + + +def export_steps(steps, ext, writer=None, *, export_trials=True, + export_active=True): + trial_pattern = _DEFAULT_TRIAL_PATTERN if export_trials else None + active_pattern = _DEFAULT_ACTIVE_PATTERN if export_active else None + exporter = SymLinkStepExporter( + ext=ext, + writer=writer, + trial_pattern=trial_pattern, + active_pattern=active_pattern, + ) + for step in steps: + exporter.export_step(step) + diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py new file mode 100644 index 000000000..60b21562b --- /dev/null +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -0,0 +1,339 @@ +import pytest +import os +import pathlib +import openpathsampling as paths +from unittest.mock import Mock + +from openpathsampling.exports.steps.symlink_step_exporter import ( + SymLinkStepExporter, + _DEFAULT_TRIAL_PATTERN, _DEFAULT_ACTIVE_PATTERN, + export_steps, +) + +from openpathsampling.tests.analysis.utils.mock_movers import ( + MockForwardShooting, MockRepex, + MockPathReversal, run_moves, +) + +from openpathsampling.tests.test_helpers import make_1d_traj + +@pytest.fixture +def shooting_step(default_unidirectional_tis): + scheme = default_unidirectional_tis.scheme + traj = default_unidirectional_tis.make_tis_trajectory(5) + init_conds = scheme.initial_conditions_from_trajectories(traj) + ensemble = scheme.network.sampling_ensembles[0] + + # Create a shooting move that will be accepted + # Use make_trajectory from fixture for proper trajectory generation + partial_traj = default_unidirectional_tis.make_trajectory(-1, 2).reversed + move = MockForwardShooting( + shooting_index=2, + partial_traj=partial_traj, + scheme=scheme, + ensemble=ensemble, + accepted=True + ) + + steplist = list(run_moves(init_conds, [move])) + assert len(steplist) == 1 + step = steplist[0] + assert ensemble(step.change.trials[0]) # acceptance is allowed + return step + +@pytest.fixture +def repex_step(default_unidirectional_tis): + scheme = default_unidirectional_tis.scheme + t1 = default_unidirectional_tis.make_tis_trajectory(4) + t2 = default_unidirectional_tis.make_tis_trajectory(10) + init_conds = scheme.initial_conditions_from_trajectories([t1, t2]) + + e1, e2 = scheme.network.sampling_ensembles[:2] + ensembles = [e1, e2] + + move = MockRepex(scheme, ensembles) + + steplist = list(run_moves(init_conds, [move])) + assert len(steplist) == 1 + step = steplist[0] + return step + +@pytest.fixture +def pathreversal_step(default_unidirectional_tis): + scheme = default_unidirectional_tis.scheme + traj = default_unidirectional_tis.make_tis_trajectory(6) # will be accepted + init_conds = scheme.initial_conditions_from_trajectories(traj) + + # Create a path reversal move + move = MockPathReversal(scheme) + + # Run the move to get a step + steplist = list(run_moves(init_conds, [move])) + assert len(steplist) == 1 + step = steplist[0] + return step + +@pytest.fixture +def all_steps(shooting_step, repex_step, pathreversal_step): + """Collection of all step fixtures""" + return [shooting_step, repex_step, pathreversal_step] + +@pytest.fixture +def unnamed_ensemble(): + cv = paths.FunctionCV("x", lambda s: s.xyz[0][0]) + ensemble = paths.CVDefinedVolume(cv, -1.0, 1.0) + # Explicitly set name to None to ensure it's truly unnamed + ensemble._name = None + return ensemble + +@pytest.fixture +def named_ensemble(): + cv = paths.FunctionCV("x", lambda s: s.xyz[0][0]) + ensemble = paths.CVDefinedVolume(cv, -1.0, 1.0).named("EnsembleName") + return ensemble + + +class TestSymLinkStepExporter: + def setup_method(self): + """Setup method to create exporter instance for tests.""" + self.exporter = SymLinkStepExporter(ext="dat") + + self.mock_writer = Mock() + def mock_write_func(trajectory, filename): + pathlib.Path(filename).parent.mkdir(parents=True, exist_ok=True) + pathlib.Path(filename).touch() + self.mock_writer.side_effect = mock_write_func + + @pytest.mark.parametrize("source", ["name", "uuid"]) + def test_get_ensemble_id(self, source, request): + fixture = {'name': 'named_ensemble', + 'uuid': 'unnamed_ensemble'}[source] + ensemble = request.getfixturevalue(fixture) + expected = {'name': 'EnsembleName', + 'uuid': str(ensemble.__uuid__)}[source] + + sample = Mock() + sample.ensemble = ensemble + assert self.exporter._get_ensemble_id(sample) == expected + + @pytest.mark.parametrize("source", ["obj", "most_common"]) + def test_get_writer(self, source): + if source == "obj": + mock_writer = Mock() + exporter = SymLinkStepExporter(ext="dat", writer=mock_writer) + sample = Mock() + sample.trajectory = [] + + result = exporter._get_writer(sample) + assert result is mock_writer + + elif source == "most_common": + exporter = SymLinkStepExporter(ext="dat", writer=None) + + traj1 = make_1d_traj([1.0]) + traj2 = make_1d_traj([2.0, 3.0]) + + engine1 = traj1[0].engine + engine2 = traj2[0].engine + assert engine1 is not engine2 + + combined_traj = traj1 + traj2 + + sample = Mock() + sample.trajectory = combined_traj + + result = exporter._get_writer(sample) + assert result == engine2.export_trajectory + + def test_substitution_dict(self, shooting_step): + exporter = SymLinkStepExporter(ext="dat") + + step = shooting_step + sample = step.change.trials[0] + + subs = exporter._substitution_dict(step, sample) + + assert subs["step"] is step + assert subs["sample"] is sample + assert subs["ensemble_id"] == exporter._get_ensemble_id(sample) + assert subs["ext"] == "dat" + + def test_export_trial_sample(self, shooting_step, tmp_path): + step = shooting_step + sample = step.change.trials[0] + + exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + assert tmp_path.exists() + assert tmp_path.is_dir() + assert len(list(tmp_path.iterdir())) == 0 + + original_cwd = os.getcwd() + os.chdir(tmp_path) + + try: + exporter.export_trial_sample(step, sample) + + subs_dict = exporter._substitution_dict(step, sample) + raw_data_path = exporter.raw_data_pattern.format(**subs_dict) + trial_path = exporter.trial_pattern.format(**subs_dict) + assert pathlib.Path(raw_data_path).exists() + + assert pathlib.Path(trial_path).exists() + assert pathlib.Path(trial_path).is_symlink() + + assert pathlib.Path(raw_data_path).samefile(trial_path) + + finally: + os.chdir(original_cwd) + + def test_export_active_sample(self, shooting_step, tmp_path): + step = shooting_step + sample = step.active[0] + + exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + + original_cwd = os.getcwd() + os.chdir(tmp_path) + + try: + exporter.export_active_sample(step, sample) + + subs_dict = exporter._substitution_dict(step, sample) + raw_data_path = exporter.raw_data_pattern.format(**subs_dict) + assert pathlib.Path(raw_data_path).exists() + + active_path = exporter.active_pattern.format(**subs_dict) + assert pathlib.Path(active_path).exists() + assert pathlib.Path(active_path).is_symlink() + + assert pathlib.Path(raw_data_path).samefile(active_path) + + finally: + os.chdir(original_cwd) + + def test_export_raw_sample(self, shooting_step, tmp_path): + step = shooting_step + sample = step.active[0] + + exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + + original_cwd = os.getcwd() + os.chdir(tmp_path) + + try: + exporter.export_raw_sample(step, sample) + + subs_dict = exporter._substitution_dict(step, sample) + raw_data_path = exporter.raw_data_pattern.format(**subs_dict) + assert pathlib.Path(raw_data_path).exists() + + assert pathlib.Path(raw_data_path).is_file() + assert not pathlib.Path(raw_data_path).is_symlink() + + finally: + os.chdir(original_cwd) + + @pytest.mark.parametrize("step_type", ["shooting", "repex", + "pathreversal"]) + def test_export_step(self, step_type, request, tmp_path): + expected_trials_by_type = { + "shooting": 1, + "repex": 2, + "pathreversal": 1, + } + expected_trials = expected_trials_by_type[step_type] + + step = request.getfixturevalue(f"{step_type}_step") + + all_trajectories = [] + for sample in step.change.trials: + all_trajectories.append(sample.trajectory) + for sample in step.active: + all_trajectories.append(sample.trajectory) + + unique_trajectory_uuids = set(traj.__uuid__ for traj in all_trajectories) + expected_raw_files = len(unique_trajectory_uuids) + + actual_trials = len(step.change.trials) + expected_active = len(step.active) + + exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + + original_cwd = os.getcwd() + os.chdir(tmp_path) + + try: + assert actual_trials == expected_trials, f"Expected {expected_trials} trials for {step_type}, got {actual_trials}" + + exporter.export_step(step) + + if pathlib.Path("raw_data").exists(): + raw_files = list(pathlib.Path("raw_data").glob("*.db")) + assert len(raw_files) == expected_raw_files, f"Expected {expected_raw_files} raw files, got {len(raw_files)}" + + trial_files = [] + if pathlib.Path(f"{step.mccycle}/trials").exists(): + trial_files = list(pathlib.Path(f"{step.mccycle}/trials").glob("*.db")) + assert len(trial_files) == actual_trials, f"Expected {actual_trials} trial symlinks, got {len(trial_files)}" + + for trial_file in trial_files: + assert trial_file.is_symlink(), f"Trial file {trial_file} should be a symlink" + + active_files = [] + if pathlib.Path(f"{step.mccycle}/active").exists(): + active_files = list(pathlib.Path(f"{step.mccycle}/active").glob("*.db")) + + assert len(active_files) == expected_active, f"Expected {expected_active} active symlinks, got {len(active_files)}" + + for active_file in active_files: + assert active_file.is_symlink(), f"Active file {active_file} should be a symlink" + + finally: + os.chdir(original_cwd) + + +def test_export_steps(all_steps, tmp_path): + mock_writer = Mock() + def mock_write_func(trajectory, filename): + pathlib.Path(filename).parent.mkdir(parents=True, exist_ok=True) + pathlib.Path(filename).touch() + mock_writer.side_effect = mock_write_func + + original_cwd = os.getcwd() + os.chdir(tmp_path) + + try: + export_steps(all_steps, ext="db", writer=mock_writer) + + assert pathlib.Path("raw_data").exists(), "Raw data directory should exist" + raw_files = list(pathlib.Path("raw_data").glob("*.db")) + assert len(raw_files) > 0, "Should have raw data files" + + for step in all_steps: + step_dir = pathlib.Path(str(step.mccycle)) + assert step_dir.exists(), f"Step directory {step_dir} should exist" + + trials_dir = step_dir / "trials" + assert trials_dir.exists(), f"Trials directory {trials_dir} should exist" + + active_dir = step_dir / "active" + assert active_dir.exists(), f"Active directory {active_dir} should exist" + + export_steps(all_steps, ext="db2", writer=mock_writer, export_trials=False) + + for step in all_steps: + trials_dir = pathlib.Path(str(step.mccycle)) / "trials" + if trials_dir.exists(): + trial_files = list(trials_dir.glob("*.db2")) + assert len(trial_files) == 0, "Should not have .db2 trial files when export_trials=False" + + export_steps(all_steps, ext="db3", writer=mock_writer, export_active=False) + + for step in all_steps: + active_dir = pathlib.Path(str(step.mccycle)) / "active" + if active_dir.exists(): + active_files = list(active_dir.glob("*.db3")) + assert len(active_files) == 0, "Should not have .db3 active files when export_active=False" + + finally: + os.chdir(original_cwd) From 9fc9d50cd730fd27c66ace705f162337bca92a00 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 5 Jun 2025 19:13:27 -0500 Subject: [PATCH 44/70] fix incorrect approach to unnamed ensembles --- .../exports/steps/symlink_step_exporter.py | 12 +++++------- openpathsampling/tests/exports/steps/conftest.py | 5 +++++ .../exports/steps/test_symlink_step_exporter.py | 8 +------- 3 files changed, 11 insertions(+), 14 deletions(-) create mode 100644 openpathsampling/tests/exports/steps/conftest.py diff --git a/openpathsampling/exports/steps/symlink_step_exporter.py b/openpathsampling/exports/steps/symlink_step_exporter.py index 02b706bd6..a70c0dab7 100644 --- a/openpathsampling/exports/steps/symlink_step_exporter.py +++ b/openpathsampling/exports/steps/symlink_step_exporter.py @@ -59,9 +59,11 @@ def __init__( def _get_ensemble_id(self, sample): """Get the ensemble ID (used in file names) for a sample. """ - ensemble_id = sample.ensemble.name - if not ensemble_id: + if sample.ensemble.is_named: + ensemble_id = sample.ensemble.name + else: ensemble_id = str(sample.ensemble.__uuid__) + return ensemble_id def _get_writer(self, sample): @@ -99,15 +101,11 @@ def _export_sample_symlink(self, pattern, step, sample): if not pathlib.Path(raw_data_path).exists(): self.export_raw_sample(step, sample) - # ensure parent directory exists pathlib.Path(path).parent.mkdir(parents=True, exist_ok=True) - - # Calculate relative path from symlink location to target symlink_path = pathlib.Path(path) target_path = pathlib.Path(raw_data_path) relative_target = os.path.relpath(target_path, symlink_path.parent) - - # Skip if symlink already exists + if not symlink_path.exists(): os.symlink(relative_target, path) diff --git a/openpathsampling/tests/exports/steps/conftest.py b/openpathsampling/tests/exports/steps/conftest.py new file mode 100644 index 000000000..e6b4fa953 --- /dev/null +++ b/openpathsampling/tests/exports/steps/conftest.py @@ -0,0 +1,5 @@ +# Import fixtures from analysis conftest +from openpathsampling.tests.analysis.conftest import default_unidirectional_tis + +# Make the fixture available in this module +__all__ = ['default_unidirectional_tis'] \ No newline at end of file diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index 60b21562b..af6b3e423 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -25,7 +25,6 @@ def shooting_step(default_unidirectional_tis): ensemble = scheme.network.sampling_ensembles[0] # Create a shooting move that will be accepted - # Use make_trajectory from fixture for proper trajectory generation partial_traj = default_unidirectional_tis.make_trajectory(-1, 2).reversed move = MockForwardShooting( shooting_index=2, @@ -61,13 +60,11 @@ def repex_step(default_unidirectional_tis): @pytest.fixture def pathreversal_step(default_unidirectional_tis): scheme = default_unidirectional_tis.scheme - traj = default_unidirectional_tis.make_tis_trajectory(6) # will be accepted + traj = default_unidirectional_tis.make_tis_trajectory(6) init_conds = scheme.initial_conditions_from_trajectories(traj) - # Create a path reversal move move = MockPathReversal(scheme) - # Run the move to get a step steplist = list(run_moves(init_conds, [move])) assert len(steplist) == 1 step = steplist[0] @@ -75,15 +72,12 @@ def pathreversal_step(default_unidirectional_tis): @pytest.fixture def all_steps(shooting_step, repex_step, pathreversal_step): - """Collection of all step fixtures""" return [shooting_step, repex_step, pathreversal_step] @pytest.fixture def unnamed_ensemble(): cv = paths.FunctionCV("x", lambda s: s.xyz[0][0]) ensemble = paths.CVDefinedVolume(cv, -1.0, 1.0) - # Explicitly set name to None to ensure it's truly unnamed - ensemble._name = None return ensemble @pytest.fixture From bc5d7ea3382d848d11714e3646f32c9f4018406a Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 5 Jun 2025 19:23:16 -0500 Subject: [PATCH 45/70] style --- .../steps/test_symlink_step_exporter.py | 68 +++++++++++++------ 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index af6b3e423..d2bf1c7bc 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -156,7 +156,9 @@ def test_export_trial_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.change.trials[0] - exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + exporter = SymLinkStepExporter( + ext="db", base_dir=tmp_path, writer=self.mock_writer + ) assert tmp_path.exists() assert tmp_path.is_dir() assert len(list(tmp_path.iterdir())) == 0 @@ -184,7 +186,9 @@ def test_export_active_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.active[0] - exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + exporter = SymLinkStepExporter( + ext="db", base_dir=tmp_path, writer=self.mock_writer + ) original_cwd = os.getcwd() os.chdir(tmp_path) @@ -209,7 +213,9 @@ def test_export_raw_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.active[0] - exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + exporter = SymLinkStepExporter( + ext="db", base_dir=tmp_path, writer=self.mock_writer + ) original_cwd = os.getcwd() os.chdir(tmp_path) @@ -245,42 +251,50 @@ def test_export_step(self, step_type, request, tmp_path): for sample in step.active: all_trajectories.append(sample.trajectory) - unique_trajectory_uuids = set(traj.__uuid__ for traj in all_trajectories) + unique_trajectory_uuids = set( + traj.__uuid__ for traj in all_trajectories + ) expected_raw_files = len(unique_trajectory_uuids) actual_trials = len(step.change.trials) expected_active = len(step.active) - exporter = SymLinkStepExporter(ext="db", base_dir=tmp_path, writer=self.mock_writer) + exporter = SymLinkStepExporter( + ext="db", base_dir=tmp_path, writer=self.mock_writer + ) original_cwd = os.getcwd() os.chdir(tmp_path) try: - assert actual_trials == expected_trials, f"Expected {expected_trials} trials for {step_type}, got {actual_trials}" + assert actual_trials == expected_trials exporter.export_step(step) if pathlib.Path("raw_data").exists(): raw_files = list(pathlib.Path("raw_data").glob("*.db")) - assert len(raw_files) == expected_raw_files, f"Expected {expected_raw_files} raw files, got {len(raw_files)}" + assert len(raw_files) == expected_raw_files trial_files = [] if pathlib.Path(f"{step.mccycle}/trials").exists(): - trial_files = list(pathlib.Path(f"{step.mccycle}/trials").glob("*.db")) - assert len(trial_files) == actual_trials, f"Expected {actual_trials} trial symlinks, got {len(trial_files)}" + trial_files = list( + pathlib.Path(f"{step.mccycle}/trials").glob("*.db") + ) + assert len(trial_files) == actual_trials for trial_file in trial_files: - assert trial_file.is_symlink(), f"Trial file {trial_file} should be a symlink" + assert trial_file.is_symlink() active_files = [] if pathlib.Path(f"{step.mccycle}/active").exists(): - active_files = list(pathlib.Path(f"{step.mccycle}/active").glob("*.db")) + active_files = list( + pathlib.Path(f"{step.mccycle}/active").glob("*.db") + ) - assert len(active_files) == expected_active, f"Expected {expected_active} active symlinks, got {len(active_files)}" + assert len(active_files) == expected_active for active_file in active_files: - assert active_file.is_symlink(), f"Active file {active_file} should be a symlink" + assert active_file.is_symlink() finally: os.chdir(original_cwd) @@ -299,35 +313,45 @@ def mock_write_func(trajectory, filename): try: export_steps(all_steps, ext="db", writer=mock_writer) - assert pathlib.Path("raw_data").exists(), "Raw data directory should exist" + assert pathlib.Path("raw_data").exists() raw_files = list(pathlib.Path("raw_data").glob("*.db")) - assert len(raw_files) > 0, "Should have raw data files" + assert len(raw_files) > 0 for step in all_steps: step_dir = pathlib.Path(str(step.mccycle)) - assert step_dir.exists(), f"Step directory {step_dir} should exist" + assert step_dir.exists() trials_dir = step_dir / "trials" - assert trials_dir.exists(), f"Trials directory {trials_dir} should exist" + assert trials_dir.exists() active_dir = step_dir / "active" - assert active_dir.exists(), f"Active directory {active_dir} should exist" + assert active_dir.exists() - export_steps(all_steps, ext="db2", writer=mock_writer, export_trials=False) + export_steps( + all_steps, + ext="db2", + writer=mock_writer, + export_trials=False + ) for step in all_steps: trials_dir = pathlib.Path(str(step.mccycle)) / "trials" if trials_dir.exists(): trial_files = list(trials_dir.glob("*.db2")) - assert len(trial_files) == 0, "Should not have .db2 trial files when export_trials=False" + assert len(trial_files) == 0 - export_steps(all_steps, ext="db3", writer=mock_writer, export_active=False) + export_steps( + all_steps, + ext="db3", + writer=mock_writer, + export_active=False + ) for step in all_steps: active_dir = pathlib.Path(str(step.mccycle)) / "active" if active_dir.exists(): active_files = list(active_dir.glob("*.db3")) - assert len(active_files) == 0, "Should not have .db3 active files when export_active=False" + assert len(active_files) == 0 finally: os.chdir(original_cwd) From 4140e5d7ad712ce81f190d95aa935d7a1dae33de Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 13 Dec 2025 15:27:27 -0600 Subject: [PATCH 46/70] Switch to conda-rc-check@v2 This has been failing for a while (the old web-scraping approach was fragile), so I just cut a new release of the action with a better implementation. This PR updates our workflows to use the newer versions. --- .github/workflows/check-openmm-rc.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-openmm-rc.yml b/.github/workflows/check-openmm-rc.yml index 1d2c0b176..15aa6fcc5 100644 --- a/.github/workflows/check-openmm-rc.yml +++ b/.github/workflows/check-openmm-rc.yml @@ -16,8 +16,8 @@ jobs: runs-on: ubuntu-latest name: "Check for OpenMM RC" steps: - - uses: actions/checkout@v2 - - uses: dwhswenson/conda-rc-check@v1 + - uses: actions/checkout@v6 + - uses: dwhswenson/conda-rc-check@v2 id: checkrc with: channel: conda-forge From c1d310d04bbb3e74dc1dd9ec442626aaede17e7e Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 13 Dec 2025 17:03:09 -0600 Subject: [PATCH 47/70] Update notebook tests to include py313 --- examples/ipynbtests.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/examples/ipynbtests.sh b/examples/ipynbtests.sh index 6b307340d..09447f5e4 100755 --- a/examples/ipynbtests.sh +++ b/examples/ipynbtests.sh @@ -42,8 +42,12 @@ case $PYTHON_VERSION in mstis="$dropbox_base_url2/wnqnwh0mmnhvm69h7h0br/toy_mstis_1k_OPS1_py312.nc?rlkey=6l2m6xiphvkpspp0ynix2m6zn&dl=1" mistis="$dropbox_base_url2/tvajjucljm83a2eo6m9ps/toy_mistis_1k_OPS1_py312.nc?rlkey=dk99cj8nlwu1re42drqngf59c&dl=1" ;; + "3.13") + mstis="$dropbox_base_url2/ofr9e8doooev99vmk6394/toy_mstis_1k_OPS1_py313.nc?rlkey=hzwzwxa5u57abv43ibuap5hpo&dl=1" + mistis="$dropbox_base_url2/tyxlixrnaog2rqepb7kt3/toy_mistis_1k_OPS1_py313.nc?rlkey=sv860b9clyja0ccn1nfsg0w07&dl=1" + ;; *) - echo "Unsupported Python version: $PYTHON_VERSION" + echo "Unsupported Python version: $PYTHON_VERSION" && exit 1 esac set -x @@ -55,7 +59,7 @@ set +x ls *nc cd toy_model_mstis/ date -py.test --nbval-lax --current-env -v \ +py.test --nbval-lax --nbval-current-env -v \ toy_mstis_1_setup.ipynb \ toy_mstis_2_run.ipynb \ toy_mstis_3_analysis.ipynb \ @@ -65,14 +69,14 @@ py.test --nbval-lax --current-env -v \ cd ../toy_model_mistis/ date # skip toy_mistis_2_flux: not needed -py.test --nbval-lax --current-env -v \ +py.test --nbval-lax --nbval-current-env -v \ toy_mistis_1_setup_run.ipynb \ toy_mistis_3_analysis.ipynb \ || testfail=1 cd ../tests/ cp ../toy_model_mstis/mstis.nc ./ -py.test --nbval --current-env \ +py.test --nbval --nbval-current-env \ test_openmm_integration.ipynb \ test_snapshot.ipynb \ test_netcdfplus.ipynb \ @@ -81,7 +85,7 @@ py.test --nbval --current-env \ cd ../misc/ cp ../toy_model_mstis/mstis.nc ./ -pytest --nbval-lax --current-env tutorial_storage.ipynb || testfail=1 +pytest --nbval-lax --nbval-current-env tutorial_storage.ipynb || testfail=1 cd .. rm toy_mstis_1k_OPS1.nc From 1aad121b13c159f6ba24dba6ff32f17afb493684 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 13 Dec 2025 17:07:29 -0600 Subject: [PATCH 48/70] Cleanup some output changes from numpy --- examples/tests/test_cv.ipynb | 3 ++- examples/tests/test_netcdfplus.ipynb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/tests/test_cv.ipynb b/examples/tests/test_cv.ipynb index 88f1b2747..bd426606e 100644 --- a/examples/tests/test_cv.ipynb +++ b/examples/tests/test_cv.ipynb @@ -272,6 +272,7 @@ } ], "source": [ + "# NBVAL_IGNORE_OUTPUT\n", "print(cv0([template, template]))\n", "print(cv1([template, template]))" ] @@ -634,7 +635,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.7" + "version": "3.13.3" }, "toc": { "base_numbering": 1, diff --git a/examples/tests/test_netcdfplus.ipynb b/examples/tests/test_netcdfplus.ipynb index 0e3525edc..9f38239c7 100644 --- a/examples/tests/test_netcdfplus.ipynb +++ b/examples/tests/test_netcdfplus.ipynb @@ -1970,6 +1970,7 @@ } ], "source": [ + "# NBVAL_IGNORE_OUTPUT\n", "try:\n", " a = Node('test')\n", " print(st.varstore.save(a))\n", @@ -2150,7 +2151,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.7" + "version": "3.13.3" }, "toc": { "base_numbering": 1, From 84c984d33201e7fad318d2acc83d50c90902519d Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 14 Dec 2025 14:55:36 -0600 Subject: [PATCH 49/70] Move the ext into the responsibility of the writer --- .../exports/steps/symlink_step_exporter.py | 12 +- openpathsampling/exports/trajectories/core.py | 9 ++ .../trajectories/mdtrajtrajectorywriter.py | 7 +- .../trajectories/trrtrajectorywriter.py | 4 + .../steps/test_symlink_step_exporter.py | 103 ++++++++---------- .../test_mdtrajtrajectorywriter.py | 6 +- 6 files changed, 73 insertions(+), 68 deletions(-) diff --git a/openpathsampling/exports/steps/symlink_step_exporter.py b/openpathsampling/exports/steps/symlink_step_exporter.py index a70c0dab7..ebab81d17 100644 --- a/openpathsampling/exports/steps/symlink_step_exporter.py +++ b/openpathsampling/exports/steps/symlink_step_exporter.py @@ -26,8 +26,6 @@ class SymLinkStepExporter: Parameters ---------- - ext : str - The extension to use for the raw data files. writer : Optional[Callable] The TrajectoryWriter to use for exporting the raw data. If None, the default writer for the most common engine in the step will be used. @@ -40,7 +38,6 @@ class SymLinkStepExporter: """ def __init__( self, - ext, writer=None, *, base_dir='.', @@ -49,7 +46,6 @@ def __init__( trial_pattern=_DEFAULT_TRIAL_PATTERN, active_pattern=_DEFAULT_ACTIVE_PATTERN, ): - self.ext = ext self.writer = writer self.base_dir = pathlib.Path(base_dir) self.raw_data_pattern = raw_data_pattern @@ -78,7 +74,7 @@ def _get_writer(self, sample): engines = collections.Counter([s.engine for s in sample.trajectory]) engine = engines.most_common(1)[0][0] - writer = engine.export_trajectory + writer = engine._default_trajectory_writer() return writer def _substitution_dict(self, step, sample): @@ -88,7 +84,7 @@ def _substitution_dict(self, step, sample): "step": step, "sample": sample, "ensemble_id": ensemble_id, - "ext": self.ext, + "ext": writer.ext, } def _export_sample_symlink(self, pattern, step, sample): @@ -169,16 +165,14 @@ def export_step(self, step): self.export_active_sample(step, sample) -def export_steps(steps, ext, writer=None, *, export_trials=True, +def export_steps(steps, writer=None, *, export_trials=True, export_active=True): trial_pattern = _DEFAULT_TRIAL_PATTERN if export_trials else None active_pattern = _DEFAULT_ACTIVE_PATTERN if export_active else None exporter = SymLinkStepExporter( - ext=ext, writer=writer, trial_pattern=trial_pattern, active_pattern=active_pattern, ) for step in steps: exporter.export_step(step) - diff --git a/openpathsampling/exports/trajectories/core.py b/openpathsampling/exports/trajectories/core.py index 20d4b3642..a385e942d 100644 --- a/openpathsampling/exports/trajectories/core.py +++ b/openpathsampling/exports/trajectories/core.py @@ -25,6 +25,11 @@ def __call__(self, trajectory, filename, force=False): self._write(trajectory, filename) + @property + def ext(self): + """The file extension used by this writer.""" + raise NotImplementedError() + def _write(self, trajectory, filename): """Write the trajectory to the file. @@ -43,6 +48,10 @@ class SimStoreTrajectoryWriter(TrajectoryWriter): This is the default trajectory writer, since all engines should be able to use it. """ + @property + def ext(self): + return "db" + def _write(self, trajectory, filename): from openpathsampling.experimental.storage import Storage from openpathsampling.experimental.storage.monkey_patches import ( diff --git a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py index 9511929ed..026716b17 100644 --- a/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py +++ b/openpathsampling/exports/trajectories/mdtrajtrajectorywriter.py @@ -9,12 +9,17 @@ class MDTrajTrajectoryWriter(TrajectoryWriter): Note that this will not include velocities, and therefore isn't suitable for saving data that could be used in a restart. """ - def __init__(self, mdtraj_selection=None): + def __init__(self, ext, mdtraj_selection=None): + self._ext = ext if not HAS_MDTRAJ: # -no-cov- raise ImportError("MDTraj is not available") self.mdtraj_selection = mdtraj_selection + @property + def ext(self): + return self._ext + def _write(self, trajectory, filename): mdt = trajectory.to_mdtraj() diff --git a/openpathsampling/exports/trajectories/trrtrajectorywriter.py b/openpathsampling/exports/trajectories/trrtrajectorywriter.py index 2b5c5b27a..5b3164b75 100644 --- a/openpathsampling/exports/trajectories/trrtrajectorywriter.py +++ b/openpathsampling/exports/trajectories/trrtrajectorywriter.py @@ -11,6 +11,10 @@ def __init__(self): if not HAS_MDTRAJ: # -no-cov- raise ImportError("MDTraj is not available") + @property + def ext(self): + return "trr" + def _write(self, trajectory, filename): # this uses some "unofficial" MDTraj API import mdtraj as md diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index d2bf1c7bc..ce1f00ff2 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -90,13 +90,13 @@ def named_ensemble(): class TestSymLinkStepExporter: def setup_method(self): """Setup method to create exporter instance for tests.""" - self.exporter = SymLinkStepExporter(ext="dat") - self.mock_writer = Mock() + self.mock_writer.ext = "dat" def mock_write_func(trajectory, filename): pathlib.Path(filename).parent.mkdir(parents=True, exist_ok=True) pathlib.Path(filename).touch() self.mock_writer.side_effect = mock_write_func + self.exporter = SymLinkStepExporter(writer=self.mock_writer) @pytest.mark.parametrize("source", ["name", "uuid"]) def test_get_ensemble_id(self, source, request): @@ -114,7 +114,8 @@ def test_get_ensemble_id(self, source, request): def test_get_writer(self, source): if source == "obj": mock_writer = Mock() - exporter = SymLinkStepExporter(ext="dat", writer=mock_writer) + mock_writer.ext = "dat" + exporter = SymLinkStepExporter(writer=mock_writer) sample = Mock() sample.trajectory = [] @@ -122,7 +123,7 @@ def test_get_writer(self, source): assert result is mock_writer elif source == "most_common": - exporter = SymLinkStepExporter(ext="dat", writer=None) + exporter = SymLinkStepExporter(writer=None) traj1 = make_1d_traj([1.0]) traj2 = make_1d_traj([2.0, 3.0]) @@ -137,10 +138,11 @@ def test_get_writer(self, source): sample.trajectory = combined_traj result = exporter._get_writer(sample) - assert result == engine2.export_trajectory + default_writer = engine2._default_trajectory_writer() + assert result.__class__ is default_writer.__class__ def test_substitution_dict(self, shooting_step): - exporter = SymLinkStepExporter(ext="dat") + exporter = SymLinkStepExporter(writer=self.mock_writer) step = shooting_step sample = step.change.trials[0] @@ -150,14 +152,14 @@ def test_substitution_dict(self, shooting_step): assert subs["step"] is step assert subs["sample"] is sample assert subs["ensemble_id"] == exporter._get_ensemble_id(sample) - assert subs["ext"] == "dat" + assert subs["ext"] == self.mock_writer.ext def test_export_trial_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.change.trials[0] exporter = SymLinkStepExporter( - ext="db", base_dir=tmp_path, writer=self.mock_writer + base_dir=tmp_path, writer=self.mock_writer ) assert tmp_path.exists() assert tmp_path.is_dir() @@ -187,7 +189,7 @@ def test_export_active_sample(self, shooting_step, tmp_path): sample = step.active[0] exporter = SymLinkStepExporter( - ext="db", base_dir=tmp_path, writer=self.mock_writer + base_dir=tmp_path, writer=self.mock_writer ) original_cwd = os.getcwd() @@ -214,7 +216,7 @@ def test_export_raw_sample(self, shooting_step, tmp_path): sample = step.active[0] exporter = SymLinkStepExporter( - ext="db", base_dir=tmp_path, writer=self.mock_writer + base_dir=tmp_path, writer=self.mock_writer ) original_cwd = os.getcwd() @@ -245,22 +247,15 @@ def test_export_step(self, step_type, request, tmp_path): step = request.getfixturevalue(f"{step_type}_step") - all_trajectories = [] - for sample in step.change.trials: - all_trajectories.append(sample.trajectory) - for sample in step.active: - all_trajectories.append(sample.trajectory) - - unique_trajectory_uuids = set( - traj.__uuid__ for traj in all_trajectories - ) - expected_raw_files = len(unique_trajectory_uuids) + unique_trajectories = { + s.trajectory for s in step.change.trials + list(step.active) + } actual_trials = len(step.change.trials) expected_active = len(step.active) exporter = SymLinkStepExporter( - ext="db", base_dir=tmp_path, writer=self.mock_writer + base_dir=tmp_path, writer=self.mock_writer ) original_cwd = os.getcwd() @@ -271,25 +266,24 @@ def test_export_step(self, step_type, request, tmp_path): exporter.export_step(step) - if pathlib.Path("raw_data").exists(): - raw_files = list(pathlib.Path("raw_data").glob("*.db")) - assert len(raw_files) == expected_raw_files + raw_dir = pathlib.Path("raw_data") + assert raw_dir.exists() + raw_files = list(raw_dir.glob(f"*.{self.mock_writer.ext}")) + assert len(raw_files) == len(unique_trajectories) - trial_files = [] - if pathlib.Path(f"{step.mccycle}/trials").exists(): - trial_files = list( - pathlib.Path(f"{step.mccycle}/trials").glob("*.db") - ) + trials_dir = pathlib.Path(f"{step.mccycle}/trials") + assert trials_dir.exists() + trial_files = list(trials_dir.glob(f"*.{self.mock_writer.ext}")) assert len(trial_files) == actual_trials for trial_file in trial_files: assert trial_file.is_symlink() - active_files = [] - if pathlib.Path(f"{step.mccycle}/active").exists(): - active_files = list( - pathlib.Path(f"{step.mccycle}/active").glob("*.db") - ) + active_dir = pathlib.Path(f"{step.mccycle}/active") + assert active_dir.exists() + active_files = list( + active_dir.glob(f"*.{self.mock_writer.ext}") + ) assert len(active_files) == expected_active @@ -302,6 +296,7 @@ def test_export_step(self, step_type, request, tmp_path): def test_export_steps(all_steps, tmp_path): mock_writer = Mock() + mock_writer.ext = "db" def mock_write_func(trajectory, filename): pathlib.Path(filename).parent.mkdir(parents=True, exist_ok=True) pathlib.Path(filename).touch() @@ -311,10 +306,10 @@ def mock_write_func(trajectory, filename): os.chdir(tmp_path) try: - export_steps(all_steps, ext="db", writer=mock_writer) + export_steps(all_steps, writer=mock_writer) assert pathlib.Path("raw_data").exists() - raw_files = list(pathlib.Path("raw_data").glob("*.db")) + raw_files = list(pathlib.Path("raw_data").glob(f"*.{mock_writer.ext}")) assert len(raw_files) > 0 for step in all_steps: @@ -327,31 +322,29 @@ def mock_write_func(trajectory, filename): active_dir = step_dir / "active" assert active_dir.exists() - export_steps( - all_steps, - ext="db2", - writer=mock_writer, - export_trials=False - ) + writer_db2 = Mock() + writer_db2.ext = "db2" + writer_db2.side_effect = mock_write_func + + export_steps(all_steps, writer=writer_db2, export_trials=False) for step in all_steps: trials_dir = pathlib.Path(str(step.mccycle)) / "trials" - if trials_dir.exists(): - trial_files = list(trials_dir.glob("*.db2")) - assert len(trial_files) == 0 - - export_steps( - all_steps, - ext="db3", - writer=mock_writer, - export_active=False - ) + assert trials_dir.exists() + trial_files = list(trials_dir.glob(f"*.{writer_db2.ext}")) + assert len(trial_files) == 0 + + writer_db3 = Mock() + writer_db3.ext = "db3" + writer_db3.side_effect = mock_write_func + + export_steps(all_steps, writer=writer_db3, export_active=False) for step in all_steps: active_dir = pathlib.Path(str(step.mccycle)) / "active" - if active_dir.exists(): - active_files = list(active_dir.glob("*.db3")) - assert len(active_files) == 0 + assert active_dir.exists() + active_files = list(active_dir.glob(f"*.{writer_db3.ext}")) + assert len(active_files) == 0 finally: os.chdir(original_cwd) diff --git a/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py index ca9e99106..f42e0342e 100644 --- a/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py +++ b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py @@ -10,7 +10,7 @@ def test_mdtraj_trajectory_writer(ad_trajectory, ad_grofile, tmp_path): import mdtraj as md outfile = tmp_path / "test.xtc" - writer = MDTrajTrajectoryWriter() + writer = MDTrajTrajectoryWriter(ext="xtc") assert not outfile.exists() writer(ad_trajectory, outfile) assert outfile.exists() @@ -31,7 +31,7 @@ def test_subtrajectory_selection(ad_trajectory, selection, tmp_path): import mdtraj as md outfile = tmp_path / "test.xtc" - writer = MDTrajTrajectoryWriter(mdtraj_selection=selection) + writer = MDTrajTrajectoryWriter(ext="xtc", mdtraj_selection=selection) assert not outfile.exists() writer(ad_trajectory, outfile) assert outfile.exists() @@ -58,6 +58,6 @@ def test_mdtraj_trajectory_writer_selection_error(ad_trajectory, tmp_path): if not HAS_MDTRAJ: pytest.skip("mdtraj is not available") - writer = MDTrajTrajectoryWriter(mdtraj_selection=object()) + writer = MDTrajTrajectoryWriter(ext="xtc", mdtraj_selection=object()) with pytest.raises(TypeError): writer(ad_trajectory, tmp_path / "test.xtc") From 34cce77b76994202cbeb29c84a8dbfd176cc4305 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 16 Feb 2026 18:17:34 -0600 Subject: [PATCH 50/70] More testing for exports --- .../steps/test_symlink_step_exporter.py | 40 ++++++++++++++++++- .../tests/exports/trajectories/test_core.py | 3 ++ .../test_mdtrajtrajectorywriter.py | 8 ++++ .../trajectories/test_trrtrajectorywriter.py | 8 ++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index ce1f00ff2..49a86b535 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -6,7 +6,7 @@ from openpathsampling.exports.steps.symlink_step_exporter import ( SymLinkStepExporter, - _DEFAULT_TRIAL_PATTERN, _DEFAULT_ACTIVE_PATTERN, + _DEFAULT_TRIAL_PATTERN, _DEFAULT_ACTIVE_PATTERN, _DEFAULT_RAW_DATA_PATTERN, export_steps, ) @@ -348,3 +348,41 @@ def mock_write_func(trajectory, filename): finally: os.chdir(original_cwd) + + +def test_default_raw_pattern_paths(shooting_step): + writer = Mock() + writer.ext = "dat" + exporter = SymLinkStepExporter(writer=writer) + + sample = shooting_step.change.trials[0] + subs = exporter._substitution_dict(shooting_step, sample) + + expected = f"raw_data/{sample.trajectory.__uuid__}.{writer.ext}" + assert _DEFAULT_RAW_DATA_PATTERN.format(**subs) == expected + + +def test_default_trial_pattern_paths(shooting_step): + writer = Mock() + writer.ext = "dat" + exporter = SymLinkStepExporter(writer=writer) + + sample = shooting_step.change.trials[0] + subs = exporter._substitution_dict(shooting_step, sample) + ensemble_id = exporter._get_ensemble_id(sample) + + expected = f"{shooting_step.mccycle}/trials/{ensemble_id}.{writer.ext}" + assert _DEFAULT_TRIAL_PATTERN.format(**subs) == expected + + +def test_default_active_pattern_paths(shooting_step): + writer = Mock() + writer.ext = "dat" + exporter = SymLinkStepExporter(writer=writer) + + sample = shooting_step.active[0] + subs = exporter._substitution_dict(shooting_step, sample) + ensemble_id = exporter._get_ensemble_id(sample) + + expected = f"{shooting_step.mccycle}/active/{ensemble_id}.{writer.ext}" + assert _DEFAULT_ACTIVE_PATTERN.format(**subs) == expected diff --git a/openpathsampling/tests/exports/trajectories/test_core.py b/openpathsampling/tests/exports/trajectories/test_core.py index 4e2825c92..7ec1d0e5d 100644 --- a/openpathsampling/tests/exports/trajectories/test_core.py +++ b/openpathsampling/tests/exports/trajectories/test_core.py @@ -103,3 +103,6 @@ def test_call_not_patched_fail(self, request, tmp_path): trajectory = request.getfixturevalue("ad_trajectory") with pytest.raises(RuntimeError, match="monkey-patch"): self.writer(trajectory, tmp_path / "test.db") + + def test_ext(self): + assert self.writer.ext == "db" diff --git a/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py index f42e0342e..d0f8b90ea 100644 --- a/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py +++ b/openpathsampling/tests/exports/trajectories/test_mdtrajtrajectorywriter.py @@ -61,3 +61,11 @@ def test_mdtraj_trajectory_writer_selection_error(ad_trajectory, tmp_path): writer = MDTrajTrajectoryWriter(ext="xtc", mdtraj_selection=object()) with pytest.raises(TypeError): writer(ad_trajectory, tmp_path / "test.xtc") + + +def test_mdtraj_writer_ext(): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + writer = MDTrajTrajectoryWriter(ext="xtc") + assert writer.ext == "xtc" diff --git a/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py b/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py index db89b2391..48a1d378a 100644 --- a/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py +++ b/openpathsampling/tests/exports/trajectories/test_trrtrajectorywriter.py @@ -28,3 +28,11 @@ def test_trr_trajectory_writer(ad_trajectory, tmp_path): npt.assert_allclose(box, ad_trajectory.box_vectors) npt.assert_allclose(lambd, np.zeros(len(ad_trajectory))) npt.assert_allclose(time, np.zeros(len(ad_trajectory))) + + +def test_trr_writer_ext(): + if not HAS_MDTRAJ: + pytest.skip("mdtraj is not available") + + writer = TRRTrajectoryWriter() + assert writer.ext == "trr" From a6fee084b33fbba23fb5fce9e38f1a1df074bb56 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Feb 2026 14:56:47 -0600 Subject: [PATCH 51/70] Remove dependence on pkg_resources --- devtools/ci/push-docs-to-s3.py | 10 +++++----- docs/conf.py | 7 +++++-- openpathsampling/tests/test_helpers.py | 7 ++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/devtools/ci/push-docs-to-s3.py b/devtools/ci/push-docs-to-s3.py index d4fb2269c..87c6789d4 100755 --- a/devtools/ci/push-docs-to-s3.py +++ b/devtools/ci/push-docs-to-s3.py @@ -1,8 +1,8 @@ from __future__ import print_function import os -import pkg_resources import tempfile import subprocess +from importlib import metadata import openpathsampling.version import argparse @@ -23,10 +23,11 @@ # PREFIX = openpathsampling.version.short_version def is_s3cmd_installed(): - dists = pkg_resources.working_set - if not any(d.project_name == 's3cmd' for d in dists): + try: + metadata.distribution('s3cmd') + except metadata.PackageNotFoundError as exc: raise ImportError('The s3cmd package is required. ' - 'try $ pip install s3cmd') + 'try $ pip install s3cmd') from exc def run_cmd(template, config_filename, dry=False): cmd = template.format( @@ -73,4 +74,3 @@ def run_cmd(template, config_filename, dry=False): #config=f.name, #bucket=BUCKET_NAME) #return_val = subprocess.call(cmd.split()) - diff --git a/docs/conf.py b/docs/conf.py index 33df42326..9fb651f63 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -15,9 +15,9 @@ import sys import os import shutil +from importlib.metadata import PackageNotFoundError, version as get_version # we use these to get the version -import pkg_resources import packaging.version import openpathsampling @@ -136,7 +136,10 @@ def gen_cli_docs(config_file, stdout=False): # built documents. # # The full version, including alpha/beta/rc tags. -release = pkg_resources.get_distribution('openpathsampling').version +try: + release = get_version('openpathsampling') +except PackageNotFoundError: + release = openpathsampling.version.version # The short X.Y version. # version = packaging.version.Version(release).base_version version = release # prefer to have the .dev0 label on 'latest' diff --git a/openpathsampling/tests/test_helpers.py b/openpathsampling/tests/test_helpers.py index 3ed2ed199..29819f92e 100644 --- a/openpathsampling/tests/test_helpers.py +++ b/openpathsampling/tests/test_helpers.py @@ -5,8 +5,8 @@ @author David W.H. Swenson """ -import os from functools import wraps +from importlib.resources import files from openpathsampling.engines import NoEngine import numpy as np import numpy.testing as npt @@ -19,8 +19,6 @@ except ImportError: md = None -from pkg_resources import resource_filename - import openpathsampling as paths import openpathsampling.engines.openmm as peng import openpathsampling.engines.toy as toys @@ -178,8 +176,7 @@ def prepend_exception_message(e, failmsg): e.args = tuple([arg0] + list(e.args[1:])) def data_filename(fname, subdir='test_data'): - return resource_filename('openpathsampling', - os.path.join('tests', subdir, fname)) + return str(files('openpathsampling.tests').joinpath(subdir, fname)) def true_func(value, *args, **kwargs): return True From d39156479c4333945004a0d3c0cf9d0254fb13f0 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Feb 2026 21:27:51 -0600 Subject: [PATCH 52/70] Remove usage of `df.applymap` (pandas 3 support) --- openpathsampling/numerics/resampling_statistics.py | 2 +- openpathsampling/numerics/wham.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/openpathsampling/numerics/resampling_statistics.py b/openpathsampling/numerics/resampling_statistics.py index 115a196f1..3136e6067 100644 --- a/openpathsampling/numerics/resampling_statistics.py +++ b/openpathsampling/numerics/resampling_statistics.py @@ -66,7 +66,7 @@ def std_df(objects, mean_x=None): n_obj = float(len(objects)) sq = [o**2 for o in objects] variance = mean_df(sq) - mean_x**2 - return variance.applymap(np.sqrt) + return np.sqrt(variance) class ResamplingStatistics(object): """ diff --git a/openpathsampling/numerics/wham.py b/openpathsampling/numerics/wham.py index cb439f7f9..f08d310ed 100755 --- a/openpathsampling/numerics/wham.py +++ b/openpathsampling/numerics/wham.py @@ -188,10 +188,7 @@ def unweighting_tis(self, cleaned_df): pandas.DataFrame unweighting values for the input dataframe """ - unweighting = cleaned_df.copy().applymap( - lambda x: 1.0 if x > 0.0 else 0.0 - ) - return unweighting + return cleaned_df.gt(0.0).astype(float) def sum_k_Hk_Q(self, cleaned_df): r"""Sum over histograms for each histogram bin. Length is n_bins From 2b3d1bebd705870ce4fead1c88ed462b93c4a4be Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Feb 2026 21:54:13 -0600 Subject: [PATCH 53/70] fic numpy 2 issue collapsing len 1 array to scalar --- openpathsampling/volume.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openpathsampling/volume.py b/openpathsampling/volume.py index 2c8d3f888..ac58f354e 100644 --- a/openpathsampling/volume.py +++ b/openpathsampling/volume.py @@ -418,6 +418,17 @@ def _get_cv_float(self, snapshot): val = self.collectivevariable(snapshot) if self._cv_returns_iterable is None: self._cv_returns_iterable = self._is_iterable(val) + + # NumPy >= 2.0 (e.g., 2.4.2 in CI) no longer allows __float__ for + # non-0D ndarrays, so we explicitly scalarize only size-1 arrays here. + if isinstance(val, np.ndarray): + if val.size == 1: + return float(val.item()) + + raise TypeError( + "only 0-dimensional arrays can be converted to Python scalars" + ) + return val.__float__() def __call__(self, snapshot): From 001faf1ad49c2cc428adca18c310474f2ac9f059 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Feb 2026 22:14:34 -0600 Subject: [PATCH 54/70] Better handling of sqrt for resampling variance --- openpathsampling/numerics/resampling_statistics.py | 2 +- openpathsampling/tests/test_resampling_statistics.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/openpathsampling/numerics/resampling_statistics.py b/openpathsampling/numerics/resampling_statistics.py index 3136e6067..9583c3e3d 100644 --- a/openpathsampling/numerics/resampling_statistics.py +++ b/openpathsampling/numerics/resampling_statistics.py @@ -66,7 +66,7 @@ def std_df(objects, mean_x=None): n_obj = float(len(objects)) sq = [o**2 for o in objects] variance = mean_df(sq) - mean_x**2 - return np.sqrt(variance) + return variance.apply(lambda s: s.map(np.sqrt)) class ResamplingStatistics(object): """ diff --git a/openpathsampling/tests/test_resampling_statistics.py b/openpathsampling/tests/test_resampling_statistics.py index 8bac9f588..778b13d33 100644 --- a/openpathsampling/tests/test_resampling_statistics.py +++ b/openpathsampling/tests/test_resampling_statistics.py @@ -36,6 +36,16 @@ def test_std(self): ) assert_frame_equal(std_df(self.inputs), expected_std) + def test_std_object_dtype(self): + from openpathsampling.numerics.resampling_statistics import std_df + object_inputs = [df.astype(object) for df in self.inputs] + expected_std = pd.DataFrame( + [[0.17677669529663689, 0.17677669529663689], + [0.10606601717798207, 0.17677669529663689]], + columns=['A', 'B'], index=['A', 'B'] + ) + assert_frame_equal(std_df(object_inputs), expected_std) + def test_initialization(self): stats = paths.numerics.ResamplingStatistics( function=lambda x: x, From 2bb02e2c5648f140988b8b151a93db43244f8d58 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Feb 2026 22:33:21 -0600 Subject: [PATCH 55/70] Add test for uncovered `raise` --- openpathsampling/tests/test_volume.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openpathsampling/tests/test_volume.py b/openpathsampling/tests/test_volume.py index 1fc687796..1e74b901b 100644 --- a/openpathsampling/tests/test_volume.py +++ b/openpathsampling/tests/test_volume.py @@ -224,6 +224,14 @@ def test_get_cv_float(self, inp): expected = inp in ['float', 'array1'] assert isinstance(val, float) is expected + @pytest.mark.filterwarnings("ignore:The CV 'cv' returns an iterable") + def test_get_cv_float_array_raises(self): + snap = make_1d_traj([0.0])[0] + volume = self._vol_for_cv_type('array') + with pytest.raises(TypeError, + match="only 0-dimensional arrays can be converted"): + _ = volume._get_cv_float(snap) + class TestCVRangeVolumePeriodic(object): def setup_method(self): From 37212d2a9a8e216cfa7934cf33a12308f3ff521f Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 28 Feb 2026 21:20:11 -0600 Subject: [PATCH 56/70] Fixes for deploy_docs --- .github/workflows/deploy_docs.yml | 13 +++++++------ docs/conf.py | 6 +++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/deploy_docs.yml b/.github/workflows/deploy_docs.yml index 7bd9849c2..d6aa1fd04 100644 --- a/.github/workflows/deploy_docs.yml +++ b/.github/workflows/deploy_docs.yml @@ -15,24 +15,26 @@ jobs: docs: runs-on: ubuntu-latest name: "docs" + env: + CONDA_PY: "3.12" steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 - uses: conda-incubator/setup-miniconda@v3 with: auto-update-conda: true - python-version: "3.11" + python-version: "3.12" miniforge-version: latest - name: "Install requirements" run: source devtools/conda_install_reqs.sh - - name: "Install OPS" - run: | - python -m pip install --no-deps -e . - python -c "import openpathsampling" - name: "Install doc tools" run: | python -m pip install numpydoc s3cmd conda install -y -c conda-forge --file docs/requirements.txt + - name: "Install OPS" + run: | + python -m pip install --no-deps -e . + python -c "import openpathsampling" - name: "Versions" run: conda list - name: "Build docs" @@ -49,4 +51,3 @@ jobs: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - diff --git a/docs/conf.py b/docs/conf.py index 9fb651f63..a0d22975c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -20,6 +20,11 @@ # we use these to get the version import packaging.version +# Ensure local source tree is imported before any installed package. +DOCS_DIR = os.path.abspath(os.path.dirname(__file__)) +REPO_ROOT = os.path.abspath(os.path.join(DOCS_DIR, "..")) +sys.path.insert(0, REPO_ROOT) + import openpathsampling import sphinx_rtd_theme @@ -40,7 +45,6 @@ def gen_cli_docs(config_file, stdout=False): # documentation root, use os.path.abspath to make it absolute, like shown here. #sys.path.insert(0, os.path.abspath('.')) -sys.path.insert(0,os.path.abspath('../openpathsampling/')) #sys.path.append(os.path.abspath('_themes')) # -- Preparing the CLI docs ----------------------------------------------- From 4a41b8e6f2f0b8e20c7be2e370c6a4ee43367f69 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 1 Mar 2026 19:35:45 -0600 Subject: [PATCH 57/70] OpenMMEngine: Persist `platform` This allows OpenMM engines to persist their platform (if given at initialization). It also allow override of `openmm_properties` (and `platform`) in the `initialization()` method. --- openpathsampling/engines/openmm/engine.py | 116 +++++++++++++------ openpathsampling/tests/test_openmm_engine.py | 79 +++++++++++++ 2 files changed, 158 insertions(+), 37 deletions(-) diff --git a/openpathsampling/engines/openmm/engine.py b/openpathsampling/engines/openmm/engine.py index ab9ebb750..eecd5b23c 100644 --- a/openpathsampling/engines/openmm/engine.py +++ b/openpathsampling/engines/openmm/engine.py @@ -79,7 +79,8 @@ def __init__( system, integrator, openmm_properties=None, - options=None): + options=None, + platform=None): """ Parameters ---------- @@ -95,8 +96,6 @@ def __init__( keys include GPU floating point precision. Note that by default the engine selects the fastest currently available OpenMM platform. - If you want to specify the platform you will have to call - `engine.initialize(platform)` after creating the engine. options : dict a dictionary that provides additional settings for the OPS engine. Allowed are @@ -106,6 +105,11 @@ def __init__( 'n_frames_max' : int, default: 5000, the maximal number of frames allowed for a returned trajectory object + platform : str or :class:`openmm.Platform` or None + optional default platform for creating the OpenMM simulation. + This value is used by ``initialize`` and serialization. The + ``.platform`` property reflects the initialized simulation context + and remains ``None`` until initialization occurs. Notes ----- @@ -138,6 +142,8 @@ def __init__( openmm_properties = {} self.openmm_properties = openmm_properties + self._normalize_platform(platform) # validate serializability + self._platform = platform # set no cached snapshot self._current_snapshot = None @@ -202,7 +208,8 @@ def from_new_options( self.system, integrator, openmm_properties=openmm_properties, - options=new_options) + options=new_options, + platform=self._platform) if self._simulation is not None and \ integrator is self.integrator and \ @@ -220,7 +227,12 @@ def from_new_options( @property def platform(self): """ - str : Return the name of the currently used platform + str or None : Return the name of the currently used platform. + + This value is populated from the simulation context and is ``None`` + until the simulation has been initialized. Configured platform values + may exist before initialization for serialization, but are not exposed + through this property. """ if self._simulation is not None: @@ -228,6 +240,22 @@ def platform(self): else: return None + @staticmethod + def _normalize_platform(platform): + if platform is None: + return None + + if isinstance(platform, str): + return platform + + try: + return platform.getName() + except (AttributeError, TypeError) as err: + raise TypeError( + "platform must be None, a platform name string, or an " + "openmm.Platform instance" + ) from err + @property def simulation(self): if self._simulation is None: @@ -261,7 +289,7 @@ def unload_context(self): def _default_trajectory_writer(self): return TRRTrajectoryWriter() - def initialize(self, platform=None): + def initialize(self, platform=None, openmm_properties=None): """ Create the final OpenMMEngine @@ -270,6 +298,9 @@ def initialize(self, platform=None): platform : str or :class:`openmm.Platform` or None either a string with a name of the platform or a platform object if None it will default to the fastest currently available platform + openmm_properties : dict or None + optional platform properties for this initialize call. If ``None`` + the engine-stored ``openmm_properties`` are used. Notes ----- @@ -281,37 +312,45 @@ def initialize(self, platform=None): """ if self._simulation is None: - if type(platform) is str: - self._simulation = openmm.app.Simulation( - topology=self.topology.mdtraj.to_openmm(), - system=self.system, - integrator=self.integrator, - platform=openmm.Platform.getPlatformByName(platform), - platformProperties=self.openmm_properties - ) - elif platform is None: - # as of OpenMM 8.1, we can't give an empty props dict when - # platform is None. This will still raise the internal - # OpenMM error is platform is None and properties are - # provided. - openmm_props = self.openmm_properties - if openmm_props == {}: - openmm_props = None + if platform is None: + effective_platform = self._platform + else: + self._normalize_platform(platform) # validate serializability + effective_platform = platform - self._simulation = openmm.app.Simulation( - topology=self.topology.mdtraj.to_openmm(), - system=self.system, - integrator=self.integrator, - platformProperties=openmm_props, - ) + if openmm_properties is None: + effective_properties = self.openmm_properties else: - self._simulation = openmm.app.Simulation( - topology=self.topology.mdtraj.to_openmm(), - system=self.system, - integrator=self.integrator, - platform=platform, - platformProperties=self.openmm_properties - ) + effective_properties = openmm_properties + + simulation_kwargs = { + 'topology': self.topology.mdtraj.to_openmm(), + 'system': self.system, + 'integrator': self.integrator, + } + + if effective_platform is None: + if not effective_properties: + openmm_props = None + else: + openmm_props = effective_properties + raise ValueError( + "OpenMM platform-specific properties were provided, " + "but no platform was specified." + ) + simulation_kwargs['platformProperties'] = openmm_props + else: + if isinstance(effective_platform, str): + resolved_platform = openmm.Platform.getPlatformByName( + effective_platform + ) + else: + resolved_platform = effective_platform + + simulation_kwargs['platform'] = resolved_platform + simulation_kwargs['platformProperties'] = effective_properties + + self._simulation = openmm.app.Simulation(**simulation_kwargs) logger.info( 'Initialized OpenMM engine using platform `%s`' % @@ -333,7 +372,8 @@ def to_dict(self): 'integrator_xml': integrator_xml, 'topology': self.topology, 'options': self.options, - 'properties': self.openmm_properties + 'properties': self.openmm_properties, + 'platform': self._normalize_platform(self._platform) } @classmethod @@ -343,6 +383,7 @@ def from_dict(cls, dct): topology = dct['topology'] options = dct['options'] properties = dct['properties'] + platform = dct.get('platform', None) # we need to have str as keys properties = {str(key): str(value) @@ -355,7 +396,8 @@ def from_dict(cls, dct): system=openmm.XmlSerializer.deserialize(system_xml), integrator=integrator, options=options, - openmm_properties=properties + openmm_properties=properties, + platform=platform ) @property diff --git a/openpathsampling/tests/test_openmm_engine.py b/openpathsampling/tests/test_openmm_engine.py index ff6425e85..7c04924cd 100644 --- a/openpathsampling/tests/test_openmm_engine.py +++ b/openpathsampling/tests/test_openmm_engine.py @@ -83,6 +83,13 @@ def setup_method(self): ) integrator.setConstraintTolerance(0.00001) + uninitialized_integrator = mm.LangevinIntegrator( + 300*u.kelvin, + old_div(1.0, u.picoseconds), + 2.0*u.femtoseconds + ) + uninitialized_integrator.setConstraintTolerance(0.00001) + # Engine options options = { 'n_steps_per_frame': 2, @@ -96,6 +103,13 @@ def setup_method(self): options=options ) + self.uninitialized_engine = peng.Engine( + template.topology, + system, + uninitialized_integrator, + options=options + ) + self.engine.initialize('CPU') context = self.engine.simulation.context zero_array = np.zeros((template.topology.n_atoms, 3)) @@ -296,3 +310,68 @@ def test_export_trajectory(self, tmp_path): assert len(reloaded) == len(traj) npt.assert_allclose(reloaded.xyz, traj.xyz) + def test_serialization_cycle(self): + integrator = mm.LangevinIntegrator( + 300*u.kelvin, + old_div(1.0, u.picoseconds), + 2.0*u.femtoseconds + ) + integrator.setConstraintTolerance(0.00001) + engine = peng.Engine( + template.topology, + system, + integrator, + options={'n_steps_per_frame': 2, 'n_frames_max': 5}, + platform='CPU' + ) + + dct = engine.to_dict() + rebuilt = peng.Engine.from_dict(dct) + dct2 = rebuilt.to_dict() + assert dct == dct2 + assert dct2['platform'] == 'CPU' + + def test_initialize_uses_exact_platform_object(self): + platform_obj = mm.Platform.getPlatformByName('CPU') + self.uninitialized_engine.initialize(platform=platform_obj) + + assert self.uninitialized_engine.platform == 'CPU' + assert self.uninitialized_engine._platform is None + assert self.uninitialized_engine.to_dict()['platform'] is None + + def test_from_dict_without_platform_key_is_backward_compatible(self): + serialized = self.engine.to_dict() + del serialized['platform'] + + restored = peng.Engine.from_dict(serialized) + assert restored is not None + assert restored.platform is None + + def test_initialize_uses_stored_properties_when_override_is_none( + self): + self.uninitialized_engine.openmm_properties = {'__ops_bad__': '1'} + with pytest.raises(Exception, match='Illegal property name'): + self.uninitialized_engine.initialize( + platform='CPU', + openmm_properties=None + ) + + def test_initialize_explicit_properties_override_stored(self): + self.uninitialized_engine.openmm_properties = {'__ops_bad__': '1'} + self.uninitialized_engine.initialize( + platform='CPU', + openmm_properties={} + ) + assert isinstance(self.uninitialized_engine.simulation, mm.app.Simulation) + assert self.uninitialized_engine.platform == 'CPU' + + def test_initialize_nonempty_properties_without_platform_raises(self): + self.uninitialized_engine.openmm_properties = {'Threads': '1'} + with pytest.raises(ValueError, match='no platform was specified'): + self.uninitialized_engine.initialize() + + def test_initialize_empty_properties_without_platform_is_valid(self): + self.uninitialized_engine.openmm_properties = {} + self.uninitialized_engine.initialize() + assert isinstance(self.uninitialized_engine.simulation, mm.app.Simulation) + assert self.uninitialized_engine.platform is not None From 69deb9c9121fe66a0740f8baa8406cecc0a850ef Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 1 Mar 2026 20:02:44 -0600 Subject: [PATCH 58/70] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- openpathsampling/engines/openmm/engine.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/openpathsampling/engines/openmm/engine.py b/openpathsampling/engines/openmm/engine.py index eecd5b23c..c0ca7f798 100644 --- a/openpathsampling/engines/openmm/engine.py +++ b/openpathsampling/engines/openmm/engine.py @@ -330,15 +330,12 @@ def initialize(self, platform=None, openmm_properties=None): } if effective_platform is None: - if not effective_properties: - openmm_props = None - else: - openmm_props = effective_properties + if effective_properties: raise ValueError( "OpenMM platform-specific properties were provided, " "but no platform was specified." ) - simulation_kwargs['platformProperties'] = openmm_props + simulation_kwargs['platformProperties'] = None else: if isinstance(effective_platform, str): resolved_platform = openmm.Platform.getPlatformByName( From 87c952563dc4f4dd71f3e491d12309a3a2b86606 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 1 Mar 2026 21:37:01 -0600 Subject: [PATCH 59/70] Modernize conf.py: pathlib and contextlib! --- docs/conf.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index a0d22975c..a343f45fd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -13,17 +13,18 @@ # serve to show the default. import sys -import os import shutil +from contextlib import chdir +from pathlib import Path from importlib.metadata import PackageNotFoundError, version as get_version # we use these to get the version import packaging.version # Ensure local source tree is imported before any installed package. -DOCS_DIR = os.path.abspath(os.path.dirname(__file__)) -REPO_ROOT = os.path.abspath(os.path.join(DOCS_DIR, "..")) -sys.path.insert(0, REPO_ROOT) +DOCS_DIR = Path(__file__).resolve().parent +REPO_ROOT = DOCS_DIR.parent +sys.path.insert(0, str(REPO_ROOT)) import openpathsampling @@ -41,26 +42,19 @@ def gen_cli_docs(config_file, stdout=False): gen_cli_docs = paths_cli.compiling.gendocs.do_main # If extensions (or modules to document with autodoc) are in another directory, -# add these directories to sys.path here. If the directory is relative to the -# documentation root, use os.path.abspath to make it absolute, like shown here. +# add these directories to sys.path here. -#sys.path.insert(0, os.path.abspath('.')) -#sys.path.append(os.path.abspath('_themes')) +#sys.path.insert(0, str(DOCS_DIR)) +#sys.path.append(str(DOCS_DIR / '_themes')) # -- Preparing the CLI docs ----------------------------------------------- -orig_cwd = os.getcwd() -try: - mydir = os.path.dirname(__file__) - cli_inp_dir = os.path.abspath(os.path.join(mydir, "cli/compile/input/")) - os.chdir(cli_inp_dir) +with chdir(DOCS_DIR / "cli" / "compile" / "input"): gen_cli_docs("categories.yml", stdout=False) -finally: - os.chdir(orig_cwd) # -- Copying examples over into the docs/examples ------------------------- try: - shutil.copytree(os.path.abspath("../examples/alanine_dipeptide_tps"), - os.path.abspath("examples/alanine_dipeptide_tps")) + shutil.copytree(REPO_ROOT / "examples" / "alanine_dipeptide_tps", + DOCS_DIR / "examples" / "alanine_dipeptide_tps") except OSError: pass # there should be a backup here.... @@ -112,7 +106,7 @@ def gen_cli_docs(config_file, stdout=False): autosummary_generate = True autodoc_default_flags = ['members', 'inherited-members', 'imported-members'] -sys.path.insert(0, os.path.abspath('sphinxext')) +sys.path.insert(0, str(DOCS_DIR / 'sphinxext')) extensions.append('notebook_sphinxext') extensions.append('pandoc_sphinxext') From 98d2ec7e11f8b0eb6160a4feb74856ae50f49af4 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 2 Mar 2026 00:14:29 -0600 Subject: [PATCH 60/70] test to cover error case in _normalize_platform --- openpathsampling/tests/test_openmm_engine.py | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/openpathsampling/tests/test_openmm_engine.py b/openpathsampling/tests/test_openmm_engine.py index 7c04924cd..0fbc7cfde 100644 --- a/openpathsampling/tests/test_openmm_engine.py +++ b/openpathsampling/tests/test_openmm_engine.py @@ -339,6 +339,27 @@ def test_initialize_uses_exact_platform_object(self): assert self.uninitialized_engine._platform is None assert self.uninitialized_engine.to_dict()['platform'] is None + def test_bad_platform_get_name_raises_typeerror(self): + class BadPlatform(object): + def getName(self): + raise TypeError("bad getName") + + integrator = mm.LangevinIntegrator( + 300*u.kelvin, + old_div(1.0, u.picoseconds), + 2.0*u.femtoseconds + ) + integrator.setConstraintTolerance(0.00001) + + with pytest.raises(TypeError, match='platform must be None'): + peng.Engine( + template.topology, + system, + integrator, + options={'n_steps_per_frame': 2, 'n_frames_max': 5}, + platform=BadPlatform() + ) + def test_from_dict_without_platform_key_is_backward_compatible(self): serialized = self.engine.to_dict() del serialized['platform'] From 1c40ae7a3a59efa753e5480ec23e7d811aa79d7a Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Mar 2026 13:38:04 -0500 Subject: [PATCH 61/70] Fix problems with scalarize singletons in numpy 2 --- .../experimental/simstore/storable_functions.py | 6 ++++-- .../experimental/simstore/test_storable_function.py | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/openpathsampling/experimental/simstore/storable_functions.py b/openpathsampling/experimental/simstore/storable_functions.py index f72a2bd1c..1749e1f40 100644 --- a/openpathsampling/experimental/simstore/storable_functions.py +++ b/openpathsampling/experimental/simstore/storable_functions.py @@ -56,10 +56,12 @@ def _scalarize_singletons(values): """ if isinstance(values, np.ndarray): shape = tuple(n for n in values.shape if n != 1) + # NumPy >= 2.0 rejects float() for non-0D ndarrays, so use item() + # when singleton axes collapse to a scalar. if shape == tuple(): - values = values.__float__() + values = float(values.item()) else: - values.shape = shape + values = np.reshape(values, shape) # shape = values.shape # if len(shape) > 1 and shape[1] == 1: diff --git a/openpathsampling/experimental/simstore/test_storable_function.py b/openpathsampling/experimental/simstore/test_storable_function.py index 78fccb97a..d48a82d02 100644 --- a/openpathsampling/experimental/simstore/test_storable_function.py +++ b/openpathsampling/experimental/simstore/test_storable_function.py @@ -66,6 +66,11 @@ def test_scalarize_singletons_to_float(): assert not isinstance(scalarized, np.ndarray) assert isinstance(scalarized, float) +def test_scalarize_singletons_non_0d_singleton_to_float(): + scalarized = scalarize_singletons(np.array([[1.0]])) + assert not isinstance(scalarized, np.ndarray) + assert isinstance(scalarized, float) + def test_wrap_numpy(): for inp in [1, [1, 2]]: assert isinstance(wrap_numpy(inp), np.ndarray) From 3df245476f45ebda568464501825f085153f763c Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 22 Mar 2026 16:18:51 -0500 Subject: [PATCH 62/70] Improve test for test_storable_function Following Copilot review --- .../experimental/simstore/test_storable_function.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openpathsampling/experimental/simstore/test_storable_function.py b/openpathsampling/experimental/simstore/test_storable_function.py index d48a82d02..f9650793c 100644 --- a/openpathsampling/experimental/simstore/test_storable_function.py +++ b/openpathsampling/experimental/simstore/test_storable_function.py @@ -65,11 +65,19 @@ def test_scalarize_singletons_to_float(): scalarized = scalarize_singletons(arr) assert not isinstance(scalarized, np.ndarray) assert isinstance(scalarized, float) + assert scalarized == 1.0 + +def test_scalarize_singletons_1d_singleton_to_float(): + scalarized = scalarize_singletons(np.array([1.0])) + assert not isinstance(scalarized, np.ndarray) + assert isinstance(scalarized, float) + assert scalarized == 1.0 def test_scalarize_singletons_non_0d_singleton_to_float(): scalarized = scalarize_singletons(np.array([[1.0]])) assert not isinstance(scalarized, np.ndarray) assert isinstance(scalarized, float) + assert scalarized == 1.0 def test_wrap_numpy(): for inp in [1, [1, 2]]: From 2e318c64a559ea22c4d5f313af804ada83bc7089 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 28 Mar 2026 12:47:35 -0500 Subject: [PATCH 63/70] Switch _normalize_platform to match/case Also adjust test to more clearly test against isinstance --- openpathsampling/engines/openmm/engine.py | 26 ++++++++++---------- openpathsampling/tests/test_openmm_engine.py | 8 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/openpathsampling/engines/openmm/engine.py b/openpathsampling/engines/openmm/engine.py index c0ca7f798..95b747b07 100644 --- a/openpathsampling/engines/openmm/engine.py +++ b/openpathsampling/engines/openmm/engine.py @@ -242,19 +242,19 @@ def platform(self): @staticmethod def _normalize_platform(platform): - if platform is None: - return None - - if isinstance(platform, str): - return platform - - try: - return platform.getName() - except (AttributeError, TypeError) as err: - raise TypeError( - "platform must be None, a platform name string, or an " - "openmm.Platform instance" - ) from err + match platform: + case None: + return None + case str() as name: + return name + case candidate if isinstance(candidate, openmm.Platform): + return candidate.getName() + case _: + raise TypeError( + "platform must be None, a platform name string, or an " + "openmm.Platform instance. " + f"Got {type(platform)} instead" + ) @property def simulation(self): diff --git a/openpathsampling/tests/test_openmm_engine.py b/openpathsampling/tests/test_openmm_engine.py index 0fbc7cfde..7f3857f7e 100644 --- a/openpathsampling/tests/test_openmm_engine.py +++ b/openpathsampling/tests/test_openmm_engine.py @@ -339,10 +339,10 @@ def test_initialize_uses_exact_platform_object(self): assert self.uninitialized_engine._platform is None assert self.uninitialized_engine.to_dict()['platform'] is None - def test_bad_platform_get_name_raises_typeerror(self): - class BadPlatform(object): + def test_duck_typed_platform_like_object_raises_typeerror(self): + class PlatformLikeObject(object): def getName(self): - raise TypeError("bad getName") + return 'CPU' integrator = mm.LangevinIntegrator( 300*u.kelvin, @@ -357,7 +357,7 @@ def getName(self): system, integrator, options={'n_steps_per_frame': 2, 'n_frames_max': 5}, - platform=BadPlatform() + platform=PlatformLikeObject() ) def test_from_dict_without_platform_key_is_backward_compatible(self): From 637e3861569b5febbade9245326c71449f051481 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 10 May 2026 17:50:31 +0200 Subject: [PATCH 64/70] Update openpathsampling/tests/test_openmm_engine.py Co-authored-by: Sander Roet --- openpathsampling/tests/test_openmm_engine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpathsampling/tests/test_openmm_engine.py b/openpathsampling/tests/test_openmm_engine.py index 7f3857f7e..76ec39ed8 100644 --- a/openpathsampling/tests/test_openmm_engine.py +++ b/openpathsampling/tests/test_openmm_engine.py @@ -351,7 +351,7 @@ def getName(self): ) integrator.setConstraintTolerance(0.00001) - with pytest.raises(TypeError, match='platform must be None'): + with pytest.raises(TypeError, match=r'platform must be None.*PlatformLikeObject'): peng.Engine( template.topology, system, From 1645ca32d90b9fcee7bd891fef87ea9d9f1d212b Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 11 May 2026 13:52:18 +0200 Subject: [PATCH 65/70] Update openpathsampling/exports/steps/symlink_step_exporter.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- openpathsampling/exports/steps/symlink_step_exporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpathsampling/exports/steps/symlink_step_exporter.py b/openpathsampling/exports/steps/symlink_step_exporter.py index ebab81d17..68d176d65 100644 --- a/openpathsampling/exports/steps/symlink_step_exporter.py +++ b/openpathsampling/exports/steps/symlink_step_exporter.py @@ -112,7 +112,7 @@ def export_trial_sample(self, step, sample): ---------- step : Step The step containing the sample. - sample : Sample + sample : Sample The trial sample to export. """ self._export_sample_symlink(self.trial_pattern, step, sample) From 354545038218faae977c56c9671de74f0074f1c3 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 14 May 2026 09:09:40 +0200 Subject: [PATCH 66/70] Specialize error for empty trajectory case Assisted-by: Codex.app:GPT-5.4 --- .../exports/steps/symlink_step_exporter.py | 4 ++++ .../tests/exports/steps/test_symlink_step_exporter.py | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/openpathsampling/exports/steps/symlink_step_exporter.py b/openpathsampling/exports/steps/symlink_step_exporter.py index 68d176d65..d1c976ed9 100644 --- a/openpathsampling/exports/steps/symlink_step_exporter.py +++ b/openpathsampling/exports/steps/symlink_step_exporter.py @@ -71,6 +71,10 @@ def _get_writer(self, sample): if self.writer is not None: writer = self.writer else: + if len(sample.trajectory) == 0: + raise ValueError( + "Cannot determine writer from an empty trajectory" + ) engines = collections.Counter([s.engine for s in sample.trajectory]) engine = engines.most_common(1)[0][0] diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index 49a86b535..4190b74ee 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -141,6 +141,17 @@ def test_get_writer(self, source): default_writer = engine2._default_trajectory_writer() assert result.__class__ is default_writer.__class__ + def test_get_writer_empty_trajectory_error(self): + exporter = SymLinkStepExporter(writer=None) + sample = Mock() + sample.trajectory = [] + + with pytest.raises( + ValueError, + match="Cannot determine writer from an empty trajectory" + ): + exporter._get_writer(sample) + def test_substitution_dict(self, shooting_step): exporter = SymLinkStepExporter(writer=self.mock_writer) From fe0052320fed155aef19fc77d44b08d196e76f70 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 20 May 2026 07:20:32 -0500 Subject: [PATCH 67/70] Implement review suggestions on contexlib.chdir Also formally update Python to 3.11+ everywhere (CI already was) Assisted-by: Codex.app:GPT-5.4 --- .../steps/test_symlink_step_exporter.py | 42 +++---------------- setup.cfg | 6 +-- 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py index 4190b74ee..281f04945 100644 --- a/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py +++ b/openpathsampling/tests/exports/steps/test_symlink_step_exporter.py @@ -1,6 +1,6 @@ import pytest -import os import pathlib +import contextlib import openpathsampling as paths from unittest.mock import Mock @@ -176,10 +176,7 @@ def test_export_trial_sample(self, shooting_step, tmp_path): assert tmp_path.is_dir() assert len(list(tmp_path.iterdir())) == 0 - original_cwd = os.getcwd() - os.chdir(tmp_path) - - try: + with contextlib.chdir(tmp_path): exporter.export_trial_sample(step, sample) subs_dict = exporter._substitution_dict(step, sample) @@ -192,9 +189,6 @@ def test_export_trial_sample(self, shooting_step, tmp_path): assert pathlib.Path(raw_data_path).samefile(trial_path) - finally: - os.chdir(original_cwd) - def test_export_active_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.active[0] @@ -203,10 +197,7 @@ def test_export_active_sample(self, shooting_step, tmp_path): base_dir=tmp_path, writer=self.mock_writer ) - original_cwd = os.getcwd() - os.chdir(tmp_path) - - try: + with contextlib.chdir(tmp_path): exporter.export_active_sample(step, sample) subs_dict = exporter._substitution_dict(step, sample) @@ -219,9 +210,6 @@ def test_export_active_sample(self, shooting_step, tmp_path): assert pathlib.Path(raw_data_path).samefile(active_path) - finally: - os.chdir(original_cwd) - def test_export_raw_sample(self, shooting_step, tmp_path): step = shooting_step sample = step.active[0] @@ -230,10 +218,7 @@ def test_export_raw_sample(self, shooting_step, tmp_path): base_dir=tmp_path, writer=self.mock_writer ) - original_cwd = os.getcwd() - os.chdir(tmp_path) - - try: + with contextlib.chdir(tmp_path): exporter.export_raw_sample(step, sample) subs_dict = exporter._substitution_dict(step, sample) @@ -243,9 +228,6 @@ def test_export_raw_sample(self, shooting_step, tmp_path): assert pathlib.Path(raw_data_path).is_file() assert not pathlib.Path(raw_data_path).is_symlink() - finally: - os.chdir(original_cwd) - @pytest.mark.parametrize("step_type", ["shooting", "repex", "pathreversal"]) def test_export_step(self, step_type, request, tmp_path): @@ -269,10 +251,7 @@ def test_export_step(self, step_type, request, tmp_path): base_dir=tmp_path, writer=self.mock_writer ) - original_cwd = os.getcwd() - os.chdir(tmp_path) - - try: + with contextlib.chdir(tmp_path): assert actual_trials == expected_trials exporter.export_step(step) @@ -301,9 +280,6 @@ def test_export_step(self, step_type, request, tmp_path): for active_file in active_files: assert active_file.is_symlink() - finally: - os.chdir(original_cwd) - def test_export_steps(all_steps, tmp_path): mock_writer = Mock() @@ -313,10 +289,7 @@ def mock_write_func(trajectory, filename): pathlib.Path(filename).touch() mock_writer.side_effect = mock_write_func - original_cwd = os.getcwd() - os.chdir(tmp_path) - - try: + with contextlib.chdir(tmp_path): export_steps(all_steps, writer=mock_writer) assert pathlib.Path("raw_data").exists() @@ -357,9 +330,6 @@ def mock_write_func(trajectory, filename): active_files = list(active_dir.glob(f"*.{writer_db3.ext}")) assert len(active_files) == 0 - finally: - os.chdir(original_cwd) - def test_default_raw_pattern_paths(shooting_step): writer = Mock() diff --git a/setup.cfg b/setup.cfg index febad0f66..b1fe47cb1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -14,9 +14,9 @@ classifiers = Intended Audience :: Developers License :: OSI Approved :: MIT License Programming Language :: Python - Programming Language :: Python :: 3.9 - Programming Language :: Python :: 3.10 Programming Language :: Python :: 3.11 + Programming Language :: Python :: 3.12 + Programming Language :: Python :: 3.13 Topic :: Scientific/Engineering :: Bio-Informatics Topic :: Scientific/Engineering :: Chemistry Topic :: Scientific/Engineering :: Physics @@ -26,7 +26,7 @@ classifiers = [options] include_package_data = True -python_requires = >=3.10 +python_requires = >=3.11 install_requires = future psutil From 71b640e0e1d0ca1de42a670845af6a9337be45d8 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 31 May 2026 12:23:35 -0500 Subject: [PATCH 68/70] Fix OpenMM RC test workflow Looks like this is failing for us due to some issues with an action no longer providing pip. This is intended to bring the OpenMM RC workflow more into line with our main testing workflow. Assisted-by: Codex.app:GPT-5.4 --- .github/workflows/test-openmm-rc.yml | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test-openmm-rc.yml b/.github/workflows/test-openmm-rc.yml index 5e27b7fac..86979f5d2 100644 --- a/.github/workflows/test-openmm-rc.yml +++ b/.github/workflows/test-openmm-rc.yml @@ -16,21 +16,24 @@ jobs: tests: runs-on: ubuntu-latest name: "Tests" + strategy: + matrix: + CONDA_PY: + - "3.11" steps: - uses: actions/checkout@v2 with: fetch-depth: 2 - uses: actions/setup-python@v2 - - uses: conda-incubator/setup-miniconda@v2 - with: + - uses: conda-incubator/setup-miniconda@v3 + with: auto-update-conda: true + python-version: ${{ matrix.CONDA_PY }} + miniforge-version: latest - name: "Install requirements" + env: + CONDA_PY: ${{ matrix.CONDA_PY }} run: | - # we'd rather use the default Python version, but for now need to - # pin to 3.9 (see openpathsampling/openpathsampling#1093) - #CONDA_PY=$(python -c "import sys; print('.'.join(str(s) for s in sys.version_info[:2]))") - export CONDA_PY="3.11" - echo "Python version: ${CONDA_PY}" source devtools/conda_install_reqs.sh - name: "Install OpenMM RC" run: | @@ -38,7 +41,9 @@ jobs: - name: "Install" run: | python -m pip install --no-deps -e . - python -c "import openpathsampling" + - name: "Check installation" + run: | + python -c "import openpathsampling; print(openpathsampling.version.full_version)" - name: "Versions" run: conda list - name: "Unit Tests" @@ -46,7 +51,6 @@ jobs: PY_COLORS: "1" run: py.test -vv -s --cov --cov-report xml - name: "Tests: Experimental" - if: matrix.MINIMAL == '' && matrix.CONDA_PY != '2.7' run: py.test openpathsampling/experimental/ -vv -s - name: "Notebook tests" run: | From c22b84af65bc34c44f4a1f1efea7beeea331e8ac Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 31 May 2026 12:31:26 -0500 Subject: [PATCH 69/70] Enable in PRs for testing --- .github/workflows/check-openmm-rc.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-openmm-rc.yml b/.github/workflows/check-openmm-rc.yml index 15aa6fcc5..9034d0bdf 100644 --- a/.github/workflows/check-openmm-rc.yml +++ b/.github/workflows/check-openmm-rc.yml @@ -7,8 +7,8 @@ on: schedule: - cron: "0 9 * * *" # use this for debugging - #pull_request: - #branch: master + pull_request: + branch: master jobs: check_rc: From 690882d79b349315661533c265b1facc87bd8c5a Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 31 May 2026 12:44:43 -0500 Subject: [PATCH 70/70] Revert "Enable in PRs for testing" This reverts commit c22b84af65bc34c44f4a1f1efea7beeea331e8ac. --- .github/workflows/check-openmm-rc.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-openmm-rc.yml b/.github/workflows/check-openmm-rc.yml index 9034d0bdf..15aa6fcc5 100644 --- a/.github/workflows/check-openmm-rc.yml +++ b/.github/workflows/check-openmm-rc.yml @@ -7,8 +7,8 @@ on: schedule: - cron: "0 9 * * *" # use this for debugging - pull_request: - branch: master + #pull_request: + #branch: master jobs: check_rc: