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 01/18] 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 02/18] 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 03/18] 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 84c984d33201e7fad318d2acc83d50c90902519d Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 14 Dec 2025 14:55:36 -0600 Subject: [PATCH 04/18] 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 05/18] 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 4a41b8e6f2f0b8e20c7be2e370c6a4ee43367f69 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 1 Mar 2026 19:35:45 -0600 Subject: [PATCH 06/18] 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 07/18] 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 98d2ec7e11f8b0eb6160a4feb74856ae50f49af4 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Mon, 2 Mar 2026 00:14:29 -0600 Subject: [PATCH 08/18] 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 09/18] 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 10/18] 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 11/18] 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 12/18] 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 13/18] 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 14/18] 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 15/18] 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 16/18] 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 17/18] 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 18/18] 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: