From 120f476852e332d3d310865f7a5479fc8bfe1639 Mon Sep 17 00:00:00 2001 From: Roy-Haolin-Du Date: Mon, 24 Feb 2025 11:31:15 +0000 Subject: [PATCH 1/4] bugfix replace-output-directory --- a3fe/_version.py | 2 +- a3fe/run/leg.py | 6 ++---- a3fe/tests/test_run.py | 41 +++++++++++++++++++++++++++++++++++++++++ docs/CHANGELOG.rst | 1 + 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/a3fe/_version.py b/a3fe/_version.py index 260c070..f9aa3e1 100644 --- a/a3fe/_version.py +++ b/a3fe/_version.py @@ -1 +1 @@ -__version__ = "0.3.1" +__version__ = "0.3.2" diff --git a/a3fe/run/leg.py b/a3fe/run/leg.py index 8c22fb1..97f26cc 100644 --- a/a3fe/run/leg.py +++ b/a3fe/run/leg.py @@ -259,11 +259,9 @@ def setup( relative_simulation_cost=self.relative_simulation_cost, ensemble_size=self.ensemble_size, lambda_values=cfg.lambda_values[self.leg_type][stage_type], - base_dir=self.stage_input_dirs[stage_type].replace("/input", ""), + base_dir=self.stage_input_dirs[stage_type].rsplit("/input", 1)[0], input_dir=self.stage_input_dirs[stage_type], - output_dir=self.stage_input_dirs[stage_type].replace( - "input", "output" - ), + output_dir="/".join(self.stage_input_dirs[stage_type].split("/")[:-1]) + "/output", stream_log_level=self.stream_log_level, ) ) diff --git a/a3fe/tests/test_run.py b/a3fe/tests/test_run.py index c2e19e2..b0e9399 100644 --- a/a3fe/tests/test_run.py +++ b/a3fe/tests/test_run.py @@ -12,6 +12,7 @@ import BioSimSpace.Sandpit.Exscientia as BSS import numpy as np import pytest +import shutil import a3fe as a3 from a3fe.analyse.detect_equil import dummy_check_equil_multiwindow @@ -348,6 +349,38 @@ def setup_calc(mock_run_process): setup_calc.setup(bound_leg_sysprep_config=cfg, free_leg_sysprep_config=cfg) yield setup_calc + @pytest.fixture + def setup_calc_with_input_path(self, setup_calc): + """Based on the setup_calc fixture, + create a new calculation with a temporary directory containing "input".""" + temp_parent_dir = "input_temp_a3fe" + os.makedirs(temp_parent_dir, exist_ok=True) + + with TemporaryDirectory(prefix="temp_input_", dir=temp_parent_dir) as dirname: + # Copy the example input directory to the temporary directory + subprocess.run( + [ + "cp", + "-r", + "a3fe/data/example_run_dir/input", + f"{dirname}/input", + ] + ) + + calc = a3.Calculation( + base_dir=dirname, + input_dir=f"{dirname}/input", + ensemble_size=1, + stream_log_level=logging.CRITICAL, + ) + cfg = SystemPreparationConfig() + cfg.slurm = False + calc.setup(bound_leg_sysprep_config=cfg, free_leg_sysprep_config=cfg) + yield calc + + if os.path.exists(temp_parent_dir): + shutil.rmtree(temp_parent_dir, ignore_errors=True) + def test_setup_calc_overall(self, setup_calc, mock_run_process): """Test that setting up the calculation was successful at a high level.""" assert setup_calc.setup_complete @@ -463,6 +496,14 @@ def test_setup_calc_sims(self, setup_calc): base_dir_files = set(os.listdir(sim.base_dir)) assert base_dir_files == expected_base_files + def test_stage_output_path_replacement(self, setup_calc_with_input_path): + """Test that stage output paths are correctly derived from input paths by only + replacing the last 'input' with 'output'.""" + for leg in setup_calc_with_input_path.legs: + for stage in leg.stages: + if leg.leg_type == a3.LegType.BOUND: + expected_output_dir = "output".join(stage.input_dir.rsplit("input", 1)) + assert stage.output_dir == expected_output_dir ######################## Tests Requiring SLURM ######################## diff --git a/docs/CHANGELOG.rst b/docs/CHANGELOG.rst index 4ea289b..337b3dc 100644 --- a/docs/CHANGELOG.rst +++ b/docs/CHANGELOG.rst @@ -5,6 +5,7 @@ Change Log 0.3.2 ==================== - Fix bug which caused somd.rst7 files in the ensemble equilibration directories to be incorrectly numbered in some cases. +- Fix bug which caused the output directory to be incorrectly replaced with "output" in some cases. 0.3.1 ==================== From 608e9d49d4fb42ec8bfcdf4d3aa6951edc99c704 Mon Sep 17 00:00:00 2001 From: Roy-Haolin-Du Date: Mon, 24 Feb 2025 11:47:42 +0000 Subject: [PATCH 2/4] Standardize formatting with ruff format --- a3fe/run/leg.py | 5 ++++- a3fe/tests/test_run.py | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/a3fe/run/leg.py b/a3fe/run/leg.py index 97f26cc..577cd46 100644 --- a/a3fe/run/leg.py +++ b/a3fe/run/leg.py @@ -261,7 +261,10 @@ def setup( lambda_values=cfg.lambda_values[self.leg_type][stage_type], base_dir=self.stage_input_dirs[stage_type].rsplit("/input", 1)[0], input_dir=self.stage_input_dirs[stage_type], - output_dir="/".join(self.stage_input_dirs[stage_type].split("/")[:-1]) + "/output", + output_dir="/".join( + self.stage_input_dirs[stage_type].split("/")[:-1] + ) + + "/output", stream_log_level=self.stream_log_level, ) ) diff --git a/a3fe/tests/test_run.py b/a3fe/tests/test_run.py index b0e9399..9b98a2e 100644 --- a/a3fe/tests/test_run.py +++ b/a3fe/tests/test_run.py @@ -351,11 +351,11 @@ def setup_calc(mock_run_process): @pytest.fixture def setup_calc_with_input_path(self, setup_calc): - """Based on the setup_calc fixture, + """Based on the setup_calc fixture, create a new calculation with a temporary directory containing "input".""" temp_parent_dir = "input_temp_a3fe" os.makedirs(temp_parent_dir, exist_ok=True) - + with TemporaryDirectory(prefix="temp_input_", dir=temp_parent_dir) as dirname: # Copy the example input directory to the temporary directory subprocess.run( @@ -502,9 +502,12 @@ def test_stage_output_path_replacement(self, setup_calc_with_input_path): for leg in setup_calc_with_input_path.legs: for stage in leg.stages: if leg.leg_type == a3.LegType.BOUND: - expected_output_dir = "output".join(stage.input_dir.rsplit("input", 1)) + expected_output_dir = "output".join( + stage.input_dir.rsplit("input", 1) + ) assert stage.output_dir == expected_output_dir + ######################## Tests Requiring SLURM ######################## From 621eb365642d7b14cd340337ed4623e2e0eca3db Mon Sep 17 00:00:00 2001 From: Roy-Haolin-Du Date: Tue, 25 Feb 2025 20:35:16 +0000 Subject: [PATCH 3/4] Make sure the last part of the output path is changed --- a3fe/run/leg.py | 11 ++++++----- a3fe/tests/test_run.py | 43 +++++------------------------------------- 2 files changed, 11 insertions(+), 43 deletions(-) diff --git a/a3fe/run/leg.py b/a3fe/run/leg.py index 577cd46..6d60172 100644 --- a/a3fe/run/leg.py +++ b/a3fe/run/leg.py @@ -259,12 +259,13 @@ def setup( relative_simulation_cost=self.relative_simulation_cost, ensemble_size=self.ensemble_size, lambda_values=cfg.lambda_values[self.leg_type][stage_type], - base_dir=self.stage_input_dirs[stage_type].rsplit("/input", 1)[0], + base_dir=_os.path.dirname( + _os.path.dirname(self.stage_input_dirs[stage_type]) + ), input_dir=self.stage_input_dirs[stage_type], - output_dir="/".join( - self.stage_input_dirs[stage_type].split("/")[:-1] - ) - + "/output", + output_dir=_os.path.join( + _os.path.dirname(self.stage_input_dirs[stage_type]), "output" + ), stream_log_level=self.stream_log_level, ) ) diff --git a/a3fe/tests/test_run.py b/a3fe/tests/test_run.py index 9b98a2e..18850e5 100644 --- a/a3fe/tests/test_run.py +++ b/a3fe/tests/test_run.py @@ -12,7 +12,6 @@ import BioSimSpace.Sandpit.Exscientia as BSS import numpy as np import pytest -import shutil import a3fe as a3 from a3fe.analyse.detect_equil import dummy_check_equil_multiwindow @@ -322,7 +321,7 @@ def _mock_run_process_inner( @pytest.fixture(scope="class") def setup_calc(mock_run_process): """Set up a calculation object completely from input files.""" - with TemporaryDirectory() as dirname: + with TemporaryDirectory(prefix="temp_input_") as dirname: # Copy the example input directory to the temporary directory # as we'll create some new files there subprocess.run( @@ -349,38 +348,6 @@ def setup_calc(mock_run_process): setup_calc.setup(bound_leg_sysprep_config=cfg, free_leg_sysprep_config=cfg) yield setup_calc - @pytest.fixture - def setup_calc_with_input_path(self, setup_calc): - """Based on the setup_calc fixture, - create a new calculation with a temporary directory containing "input".""" - temp_parent_dir = "input_temp_a3fe" - os.makedirs(temp_parent_dir, exist_ok=True) - - with TemporaryDirectory(prefix="temp_input_", dir=temp_parent_dir) as dirname: - # Copy the example input directory to the temporary directory - subprocess.run( - [ - "cp", - "-r", - "a3fe/data/example_run_dir/input", - f"{dirname}/input", - ] - ) - - calc = a3.Calculation( - base_dir=dirname, - input_dir=f"{dirname}/input", - ensemble_size=1, - stream_log_level=logging.CRITICAL, - ) - cfg = SystemPreparationConfig() - cfg.slurm = False - calc.setup(bound_leg_sysprep_config=cfg, free_leg_sysprep_config=cfg) - yield calc - - if os.path.exists(temp_parent_dir): - shutil.rmtree(temp_parent_dir, ignore_errors=True) - def test_setup_calc_overall(self, setup_calc, mock_run_process): """Test that setting up the calculation was successful at a high level.""" assert setup_calc.setup_complete @@ -496,14 +463,14 @@ def test_setup_calc_sims(self, setup_calc): base_dir_files = set(os.listdir(sim.base_dir)) assert base_dir_files == expected_base_files - def test_stage_output_path_replacement(self, setup_calc_with_input_path): + def test_stage_output_path_replacement(self, setup_calc): """Test that stage output paths are correctly derived from input paths by only replacing the last 'input' with 'output'.""" - for leg in setup_calc_with_input_path.legs: + for leg in setup_calc.legs: for stage in leg.stages: if leg.leg_type == a3.LegType.BOUND: - expected_output_dir = "output".join( - stage.input_dir.rsplit("input", 1) + expected_output_dir = os.path.join( + os.path.dirname(stage.input_dir), "output" ) assert stage.output_dir == expected_output_dir From abca4caa48344287c3a7293f05590e98cb6c22b9 Mon Sep 17 00:00:00 2001 From: Roy-Haolin-Du Date: Wed, 26 Feb 2025 21:27:10 +0000 Subject: [PATCH 4/4] correct the right path for output --- a3fe/run/leg.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/a3fe/run/leg.py b/a3fe/run/leg.py index 6d60172..c46b4cf 100644 --- a/a3fe/run/leg.py +++ b/a3fe/run/leg.py @@ -259,9 +259,7 @@ def setup( relative_simulation_cost=self.relative_simulation_cost, ensemble_size=self.ensemble_size, lambda_values=cfg.lambda_values[self.leg_type][stage_type], - base_dir=_os.path.dirname( - _os.path.dirname(self.stage_input_dirs[stage_type]) - ), + base_dir=_os.path.dirname(self.stage_input_dirs[stage_type]), input_dir=self.stage_input_dirs[stage_type], output_dir=_os.path.join( _os.path.dirname(self.stage_input_dirs[stage_type]), "output"