From c29ab43cdb77c9c3c3b9b11e93ecd7ff24f66cbe Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Thu, 4 Jul 2024 12:41:33 -0400 Subject: [PATCH 01/10] New command: Load trajectory --- paths_cli/commands/load_trajectory.py | 51 +++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 paths_cli/commands/load_trajectory.py diff --git a/paths_cli/commands/load_trajectory.py b/paths_cli/commands/load_trajectory.py new file mode 100644 index 0000000..c9087ef --- /dev/null +++ b/paths_cli/commands/load_trajectory.py @@ -0,0 +1,51 @@ +import click + +import paths_cli.utils +from paths_cli.parameters import APPEND_FILE, ENGINE, MULTI_TAG +from paths_cli import OPSCommandPlugin + + +@click.command( + "load-trajectory", + short_help="Load an external trajectory file", +) +@click.argument('traj_file') +@click.option( + '--top', + help=( + "Topology information (typically PDB). Only for required " + "formats." + ), + default=None, +) +@APPEND_FILE.clicked(required=True) +@MULTI_TAG.clicked() +def load_trajectory(traj_file, top, append, tag): + """Load a trajectory from a file. + + This uses MDTraj under the hood, and can load any file format that + MDTraj can. NB: This stores in a format based on OpenMM snapshots. + Trajectories loaded this way will work with engines compatible with + that input (e.g., GROMACS). + """ + import mdtraj as md + from openpathsampling.engines.openmm.tools import ops_load_trajectory + traj = ops_load_trajectory(traj_file, top=top) + storage = append.get() + storage.save(traj) + for tag_name in tag: + storage.tags[tag_name] = traj + + + # tests: + # * check that we load PDB + # * check that if we load PDB with top as well works + # * check that we can load trr / xtc files (only with top) + # * maybe check for error if top doesn't match traj file? (atom numbers) + +PLUGIN = OPSCommandPlugin( + command=load_trajectory, + section="Miscellaneous", + requires_ops=(1, 6), + requires_cli=(0, 4), +) From 2df9112a49d6578d5954a348e6a88fd74ce38898 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sat, 6 Jul 2024 17:57:44 -0500 Subject: [PATCH 02/10] fix up load trajectory so it works --- paths_cli/commands/load_trajectory.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/paths_cli/commands/load_trajectory.py b/paths_cli/commands/load_trajectory.py index c9087ef..ca9b127 100644 --- a/paths_cli/commands/load_trajectory.py +++ b/paths_cli/commands/load_trajectory.py @@ -13,14 +13,14 @@ @click.option( '--top', help=( - "Topology information (typically PDB). Only for required " + "Topology file (typically PDB). Only for required " "formats." ), default=None, ) @APPEND_FILE.clicked(required=True) @MULTI_TAG.clicked() -def load_trajectory(traj_file, top, append, tag): +def load_trajectory(traj_file, top, append_file, tag): """Load a trajectory from a file. This uses MDTraj under the hood, and can load any file format that @@ -31,7 +31,7 @@ def load_trajectory(traj_file, top, append, tag): import mdtraj as md from openpathsampling.engines.openmm.tools import ops_load_trajectory traj = ops_load_trajectory(traj_file, top=top) - storage = append.get() + storage = APPEND_FILE.get(append_file) storage.save(traj) for tag_name in tag: storage.tags[tag_name] = traj From 41b79f45680fe68f8107af9f32c571b3228cc2c1 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 21 Aug 2024 17:45:41 -0500 Subject: [PATCH 03/10] some cleanup for load_trajectory --- paths_cli/commands/load_trajectory.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/paths_cli/commands/load_trajectory.py b/paths_cli/commands/load_trajectory.py index ca9b127..9b4567a 100644 --- a/paths_cli/commands/load_trajectory.py +++ b/paths_cli/commands/load_trajectory.py @@ -28,9 +28,12 @@ def load_trajectory(traj_file, top, append_file, tag): Trajectories loaded this way will work with engines compatible with that input (e.g., GROMACS). """ - import mdtraj as md from openpathsampling.engines.openmm.tools import ops_load_trajectory - traj = ops_load_trajectory(traj_file, top=top) + if top: + traj = ops_load_trajectory(traj_file, top=top) + else: + traj = ops_load_trajectory(traj_file) + storage = APPEND_FILE.get(append_file) storage.save(traj) for tag_name in tag: From b61f83123f5483bf15484c9421acd6e71c1bf622 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 21 Aug 2024 22:45:03 -0500 Subject: [PATCH 04/10] run load_trajectory from module --- paths_cli/commands/load_trajectory.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/paths_cli/commands/load_trajectory.py b/paths_cli/commands/load_trajectory.py index 9b4567a..6901a8a 100644 --- a/paths_cli/commands/load_trajectory.py +++ b/paths_cli/commands/load_trajectory.py @@ -52,3 +52,6 @@ def load_trajectory(traj_file, top, append_file, tag): requires_ops=(1, 6), requires_cli=(0, 4), ) + +if __name__ == "__main__": # -no-cov- + load_trajectory() From 128301dd714cad82b808a2bd0b2d201e4ebea822 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 06:15:43 +0200 Subject: [PATCH 05/10] Add tests for load_trajectory --- .../tests/commands/test_load_trajectory.py | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 paths_cli/tests/commands/test_load_trajectory.py diff --git a/paths_cli/tests/commands/test_load_trajectory.py b/paths_cli/tests/commands/test_load_trajectory.py new file mode 100644 index 0000000..db6dc29 --- /dev/null +++ b/paths_cli/tests/commands/test_load_trajectory.py @@ -0,0 +1,83 @@ +from click.testing import CliRunner +from contextlib import contextmanager +import pytest +from importlib import resources +from openpathsampling.tests.test_helpers import data_filename +import openpathsampling as paths + +from paths_cli.commands.load_trajectory import * + + +@contextmanager +def run_load_trajectory(args): + runner = CliRunner() + with runner.isolated_filesystem(): + storage = paths.Storage("setup.nc", 'w') + storage.close() + results = runner.invoke( + load_trajectory, + args + ) + assert results.exit_code == 0 + st = paths.Storage("setup.nc", mode='r') + assert len(st.trajectories) == 1 + yield st + + +@pytest.mark.parametrize("with_top", [True, False]) +@pytest.mark.parametrize("with_tag", [True, False]) +def test_load_trajectory_pdb(with_top, with_tag): + # test that we can load a PDB file with or without topology; also tests + # that the taging works correctly + pdb_path = data_filename("ala_small_traj.pdb") + out_file = "setup.nc" + args = [ + pdb_path, + '--append-file', out_file, + ] + if with_top: + args.extend(['--top', pdb_path]) + + if with_tag: + args.extend(['--tag', 'init_snap']) + + with run_load_trajectory(args) as st: + traj = st.trajectories[0] + assert len(traj) == 10 + if with_tag: + tagged = st.tags['init_snap'] + assert tagged == traj + +def test_load_trajectory_trr(): + trr = data_filename("gromacs_engine/project_trr/0000000.trr") + gro = data_filename("gromacs_engine/conf.gro") + out_file = "setup.nc" + args = [ + trr, + '--append-file', out_file, + '--top', gro, + ] + with run_load_trajectory(args) as st: + traj = st.trajectories[0] + assert len(traj) == 4 + +def test_load_trajectory_bad_topology(): + trr = data_filename("gromacs_engine/project_trr/0000000.trr") + pdb = data_filename("tip4p_water.pdb") + out_file = "setup.nc" + args = [ + trr, + '--append-file', out_file, + '--top', pdb, + ] + runner = CliRunner() + with runner.isolated_filesystem(): + storage = paths.Storage("setup.nc", 'w') + storage.close() + result = runner.invoke( + load_trajectory, + args + ) + assert result.exit_code == 1 + assert "topology" in str(result.exception) + assert "same atoms" in str(result.exception) From b5cb4a4f4a515e518516d837d8171da8c0a11043 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 06:16:05 +0200 Subject: [PATCH 06/10] remove test planning comment --- paths_cli/commands/load_trajectory.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/paths_cli/commands/load_trajectory.py b/paths_cli/commands/load_trajectory.py index 6901a8a..6025b47 100644 --- a/paths_cli/commands/load_trajectory.py +++ b/paths_cli/commands/load_trajectory.py @@ -40,12 +40,6 @@ def load_trajectory(traj_file, top, append_file, tag): storage.tags[tag_name] = traj - # tests: - # * check that we load PDB - # * check that if we load PDB with top as well works - # * check that we can load trr / xtc files (only with top) - # * maybe check for error if top doesn't match traj file? (atom numbers) - PLUGIN = OPSCommandPlugin( command=load_trajectory, section="Miscellaneous", From 9772eef01352d54539fadf2466456d333b571b26 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 06:21:26 +0200 Subject: [PATCH 07/10] skip tests if OpenMM/MDTraj not installed --- paths_cli/tests/commands/test_load_trajectory.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/paths_cli/tests/commands/test_load_trajectory.py b/paths_cli/tests/commands/test_load_trajectory.py index db6dc29..083b93f 100644 --- a/paths_cli/tests/commands/test_load_trajectory.py +++ b/paths_cli/tests/commands/test_load_trajectory.py @@ -29,6 +29,8 @@ def run_load_trajectory(args): def test_load_trajectory_pdb(with_top, with_tag): # test that we can load a PDB file with or without topology; also tests # that the taging works correctly + pytest.importorskip("openmm") + pytest.importorskip("mdtraj") pdb_path = data_filename("ala_small_traj.pdb") out_file = "setup.nc" args = [ @@ -49,6 +51,8 @@ def test_load_trajectory_pdb(with_top, with_tag): assert tagged == traj def test_load_trajectory_trr(): + pytest.importorskip("openmm") + pytest.importorskip("mdtraj") trr = data_filename("gromacs_engine/project_trr/0000000.trr") gro = data_filename("gromacs_engine/conf.gro") out_file = "setup.nc" @@ -62,6 +66,8 @@ def test_load_trajectory_trr(): assert len(traj) == 4 def test_load_trajectory_bad_topology(): + pytest.importorskip("openmm") + pytest.importorskip("mdtraj") trr = data_filename("gromacs_engine/project_trr/0000000.trr") pdb = data_filename("tip4p_water.pdb") out_file = "setup.nc" From d1c75390aa9d9279d0dc6a05ecd8b4b8d35d08a9 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 13:52:37 +0200 Subject: [PATCH 08/10] Convert output of INIT_CONDS parameter to trajs --- paths_cli/parameters.py | 26 +++++++++++++++++++++++++- paths_cli/tests/test_parameters.py | 13 ++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/paths_cli/parameters.py b/paths_cli/parameters.py index e8fbc58..0e04709 100644 --- a/paths_cli/parameters.py +++ b/paths_cli/parameters.py @@ -18,7 +18,31 @@ store='schemes', ) -INIT_CONDS = OPSStorageLoadMultiple( +class InitCondsLoader(OPSStorageLoadMultiple): + def _extract_trajectories(self, obj): + import openpathsampling as paths + if isinstance(obj, paths.SampleSet): + yield from (s.trajectory for s in obj) + elif isinstance(obj, paths.Sample): + yield obj.trajectory + elif isinstance(obj, paths.Trajectory): + yield obj + elif isinstance(obj, paths.BaseSnapshot): + yield paths.Trajectory([obj]) + elif isinstance(obj, list): + for o in obj: + yield from self._extract_trajectories(o) + else: + raise RuntimeError("Unknown initial conditions type: " + f"{obj} (type: {type(obj)}") + + def get(self, storage, names): + results = super().get(storage, names) + final_results = list(self._extract_trajectories(results)) + return final_results + + +INIT_CONDS = InitCondsLoader( param=Option('-t', '--init-conds', multiple=True, help=("identifier for initial conditions " + "(sample set or trajectory)" + HELP_MULTIPLE)), diff --git a/paths_cli/tests/test_parameters.py b/paths_cli/tests/test_parameters.py index b748fbe..c9531e1 100644 --- a/paths_cli/tests/test_parameters.py +++ b/paths_cli/tests/test_parameters.py @@ -243,8 +243,8 @@ def test_get(self, getter): storage = paths.Storage(filename, mode='r') get_type, getter_style = self._parse_getter(getter) expected = { - 'sset': self.sample_set, - 'traj': self.traj + 'sset': [s.trajectory for s in self.sample_set], + 'traj': [self.traj] }[get_type] get_arg = { 'name': 'traj', @@ -277,7 +277,14 @@ def test_get_none(self, num_in_file): st = paths.Storage(filename, mode='r') obj = INIT_CONDS.get(st, None) - assert obj == stored_things[num_in_file - 1] + # TODO: fix this for all being trajectories + expected = [ + [self.traj], + [s.trajectory for s in self.sample_set], + [s.trajectory for s in self.other_sample_set], + [s.trajectory for s in self.other_sample_set], + ] + assert obj == expected[num_in_file - 1] def test_get_multiple(self): filename = self.create_file('number-traj') From 06db15a379c853661e92451cfc5c31fec12385f5 Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 14:50:38 +0200 Subject: [PATCH 09/10] Cleanup after switching init conds behavior --- paths_cli/tests/commands/test_equilibrate.py | 8 +++++--- paths_cli/tests/commands/test_pathsampling.py | 5 +++-- paths_cli/tests/test_parameters.py | 1 - 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/paths_cli/tests/commands/test_equilibrate.py b/paths_cli/tests/commands/test_equilibrate.py index c57a8b8..01c3689 100644 --- a/paths_cli/tests/commands/test_equilibrate.py +++ b/paths_cli/tests/commands/test_equilibrate.py @@ -11,7 +11,7 @@ def print_test(output_storage, scheme, init_conds, multiplier, extra_steps): print(isinstance(output_storage, paths.Storage)) print(scheme.__uuid__) - print(init_conds.__uuid__) + print([o.__uuid__ for o in init_conds]) print(multiplier, extra_steps) @@ -31,8 +31,10 @@ def test_equilibrate(tps_fixture): ["setup.nc", "-o", "foo.nc"] ) out_str = "True\n{schemeid}\n{condsid}\n1 0\n" - expected_output = out_str.format(schemeid=scheme.__uuid__, - condsid=init_conds.__uuid__) + expected_output = out_str.format( + schemeid=scheme.__uuid__, + condsid=[o.trajectory.__uuid__ for o in init_conds], + ) assert results.exit_code == 0 assert results.output == expected_output diff --git a/paths_cli/tests/commands/test_pathsampling.py b/paths_cli/tests/commands/test_pathsampling.py index dc5034a..08d7db3 100644 --- a/paths_cli/tests/commands/test_pathsampling.py +++ b/paths_cli/tests/commands/test_pathsampling.py @@ -11,7 +11,7 @@ def print_test(output_storage, scheme, init_conds, n_steps): print(isinstance(output_storage, paths.Storage)) print(scheme.__uuid__) - print(init_conds.__uuid__) + print([traj.__uuid__ for traj in init_conds]) print(n_steps) @patch('paths_cli.commands.pathsampling.pathsampling_main', print_test) @@ -26,7 +26,8 @@ def test_pathsampling(tps_fixture): results = runner.invoke(pathsampling, ['setup.nc', '-o', 'foo.nc', '-n', '1000']) - expected_output = (f"True\n{scheme.__uuid__}\n{init_conds.__uuid__}" + initcondsid = [samp.trajectory.__uuid__ for samp in init_conds] + expected_output = (f"True\n{scheme.__uuid__}\n{initcondsid}" "\n1000\n") assert results.output == expected_output diff --git a/paths_cli/tests/test_parameters.py b/paths_cli/tests/test_parameters.py index c9531e1..ecc82a3 100644 --- a/paths_cli/tests/test_parameters.py +++ b/paths_cli/tests/test_parameters.py @@ -277,7 +277,6 @@ def test_get_none(self, num_in_file): st = paths.Storage(filename, mode='r') obj = INIT_CONDS.get(st, None) - # TODO: fix this for all being trajectories expected = [ [self.traj], [s.trajectory for s in self.sample_set], From ff15749584c829f86d4d482b56943ff124e79d8b Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Sun, 25 Aug 2024 16:32:15 +0200 Subject: [PATCH 10/10] add some test coverage --- paths_cli/parameters.py | 2 -- paths_cli/tests/test_parameters.py | 25 ++++++++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/paths_cli/parameters.py b/paths_cli/parameters.py index 0e04709..574b17d 100644 --- a/paths_cli/parameters.py +++ b/paths_cli/parameters.py @@ -27,8 +27,6 @@ def _extract_trajectories(self, obj): yield obj.trajectory elif isinstance(obj, paths.Trajectory): yield obj - elif isinstance(obj, paths.BaseSnapshot): - yield paths.Trajectory([obj]) elif isinstance(obj, list): for o in obj: yield from self._extract_trajectories(o) diff --git a/paths_cli/tests/test_parameters.py b/paths_cli/tests/test_parameters.py index ecc82a3..e4cc9f1 100644 --- a/paths_cli/tests/test_parameters.py +++ b/paths_cli/tests/test_parameters.py @@ -214,8 +214,12 @@ def create_file(self, getter): get_type, getter_style = self._parse_getter(getter) main, other = { 'traj': (self.traj, self.other_traj), - 'sset': (self.sample_set, self.other_sample_set) + 'sset': (self.sample_set, self.other_sample_set), + 'samp': (self.sample_set[0], self.other_sample_set[0]), }[get_type] + if get_type == 'samp': + storage.save(main) + storage.save(other) if get_type == 'sset': storage.save(self.sample_set) storage.save(self.other_sample_set) @@ -231,12 +235,14 @@ def create_file(self, getter): if other_tag: storage.tags[other_tag] = other + storage.close() return filename @pytest.mark.parametrize("getter", [ 'name-traj', 'number-traj', 'tag-final-traj', 'tag-initial-traj', - 'name-sset', 'number-sset', 'tag-final-sset', 'tag-initial-sset' + 'name-sset', 'number-sset', 'tag-final-sset', 'tag-initial-sset', + 'name-samp', 'number-samp', ]) def test_get(self, getter): filename = self.create_file(getter) @@ -244,7 +250,8 @@ def test_get(self, getter): get_type, getter_style = self._parse_getter(getter) expected = { 'sset': [s.trajectory for s in self.sample_set], - 'traj': [self.traj] + 'traj': [self.traj], + 'samp': [self.sample_set[0].trajectory], }[get_type] get_arg = { 'name': 'traj', @@ -303,6 +310,18 @@ def test_cannot_guess(self): with pytest.raises(RuntimeError): self.PARAMETER.get(storage, None) + def test_get_bad_name(self): + filename = self._filename("bad_tag") + storage = paths.Storage(filename, 'w') + storage.save(self.traj) + storage.save(self.other_traj) + storage.tags['bad_tag'] = "foo" + storage.close() + + storage = paths.Storage(filename, 'r') + with pytest.raises(RuntimeError, match="initial conditions type"): + self.PARAMETER.get(storage, "bad_tag") + class TestINIT_SNAP(ParamInstanceTest): PARAMETER = INIT_SNAP