From 1ea89d83f6e9bf7ef254ec33de1eaf5bd86ffc21 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Fri, 22 Sep 2023 10:32:57 +1000 Subject: [PATCH 1/3] Refactor set_counter() and some bug fixes - Fix to payu crashing with empty restart files/dirs - Add default values of LD_LIBRARY_PATHS - Raise exceptions for missing model executables --- payu/experiment.py | 72 ++++++++++++++++++++++---------------------- payu/models/model.py | 6 ++++ 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/payu/experiment.py b/payu/experiment.py index 970a1f48..bfc6d92d 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -13,6 +13,7 @@ import errno import getpass import os +import re import resource import sys import shlex @@ -172,41 +173,39 @@ def set_counters(self): # Initialize counter if unset if self.counter is None: - # TODO: this logic can probably be streamlined - try: - restart_dirs = [d for d in os.listdir(self.archive_path) - if d.startswith('restart')] - except EnvironmentError as exc: - if exc.errno == errno.ENOENT: - restart_dirs = None - else: - raise - - # First test for restarts - if restart_dirs: - self.counter = 1 + max([int(d.lstrip('restart')) - for d in restart_dirs - if d.startswith('restart')]) + # Check for restart index + max_restart_index = self.max_output_index(output_type="restart") + if max_restart_index: + self.counter = 1 + max_restart_index else: - # repeat runs do not generate restart files, so check outputs - try: - output_dirs = [d for d in os.listdir(self.archive_path) - if d.startswith('output')] - except EnvironmentError as exc: - if exc.errno == errno.ENOENT: - output_dirs = None - else: - raise - - # First test for restarts - # Now look for output directories - if output_dirs: - self.counter = 1 + max([int(d.lstrip('output')) - for d in output_dirs - if d.startswith('output')]) + # Now look for output directories, + # as repeat runs do not generate restart files. + max_output_index = self.max_output_index() + if max_output_index: + self.counter = 1 + max_output_index else: self.counter = 0 + def max_output_index(self, output_type="output"): + """Given a output directory type (output or restart), + return the maximum index of output directories found""" + try: + output_dirs = self.list_output_dirs(output_type) + except EnvironmentError as exc: + if exc.errno == errno.ENOENT: + output_dirs = None + else: + raise + + if output_dirs and len(output_dirs): + return max([int(d.lstrip(output_type)) for d in output_dirs]) + + def list_output_dirs(self, output_type="output"): + """Return a list of restart or output directories in archive""" + naming_pattern = re.compile(fr"^{output_type}[0-9][0-9][0-9]$") + return [d for d in os.listdir(self.archive_path) + if naming_pattern.match(d)] + def set_stacksize(self, stacksize): if stacksize == 'unlimited': @@ -749,8 +748,7 @@ def archive(self): default_restart_history) # Remove any outdated restart files - prior_restart_dirs = [d for d in os.listdir(self.archive_path) - if d.startswith('restart')] + prior_restart_dirs = self.list_output_dirs(output_type="restart") for res_dir in prior_restart_dirs: @@ -766,10 +764,12 @@ def archive(self): shutil.rmtree(res_path) # Ensure dynamic library support for subsequent python calls - ld_libpaths = os.environ['LD_LIBRARY_PATH'] + ld_libpaths = os.environ.get('LD_LIBRARY_PATH', None) py_libpath = sysconfig.get_config_var('LIBDIR') - if py_libpath not in ld_libpaths.split(':'): - os.environ['LD_LIBRARY_PATH'] = ':'.join([py_libpath, ld_libpaths]) + if ld_libpaths is None: + os.environ['LD_LIBRARY_PATH'] = py_libpath + elif py_libpath not in ld_libpaths.split(':'): + os.environ['LD_LIBRARY_PATH'] = f'{py_libpath}:{ld_libpaths}' collate_config = self.config.get('collate', {}) collating = collate_config.get('enable', True) diff --git a/payu/models/model.py b/payu/models/model.py index ac6643d2..c7aec243 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -286,6 +286,12 @@ def setup(self): # If have exe manifest this implies exe reproduce is True. Do not # want to overwrite exe manifest in this case if not self.expt.manifest.have_manifest['exe']: + # Check whether executable path exists + if not os.path.isfile(self.exec_path): + raise FileNotFoundError( + f'Executable for {self.name} model ' + f'not found on path: {self.exec_path}') + # Add to exe manifest (this is always done so any change in exe # path will be picked up) self.expt.manifest.add_filepath( From 0ba1b0f0e9e3b53e18721fb32aee4a2251a3d1d2 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Tue, 3 Oct 2023 11:08:35 +1100 Subject: [PATCH 2/3] add check if model executable path has executable permission --- payu/experiment.py | 2 +- payu/models/model.py | 20 +++++++++++++------- test/common.py | 6 ++++-- test/test_manifest.py | 18 ------------------ 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/payu/experiment.py b/payu/experiment.py index bfc6d92d..591d26b8 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -204,7 +204,7 @@ def list_output_dirs(self, output_type="output"): """Return a list of restart or output directories in archive""" naming_pattern = re.compile(fr"^{output_type}[0-9][0-9][0-9]$") return [d for d in os.listdir(self.archive_path) - if naming_pattern.match(d)] + if naming_pattern.match(d)] def set_stacksize(self, stacksize): diff --git a/payu/models/model.py b/payu/models/model.py index c7aec243..8c6b0875 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -283,15 +283,21 @@ def setup(self): # Make symlink to executable in work directory if self.exec_path: + # Check whether executable path exists + if not os.path.isfile(self.exec_path): + raise FileNotFoundError( + f'Executable for {self.name} model ' + f'not found on path: {self.exec_path}') + + # Check whether executable has executable permission + if not os.access(self.exec_path, os.X_OK): + raise PermissionError( + f'Executable for {self.name} model ' + f'is not executable: {self.exec_path}') + # If have exe manifest this implies exe reproduce is True. Do not # want to overwrite exe manifest in this case if not self.expt.manifest.have_manifest['exe']: - # Check whether executable path exists - if not os.path.isfile(self.exec_path): - raise FileNotFoundError( - f'Executable for {self.name} model ' - f'not found on path: {self.exec_path}') - # Add to exe manifest (this is always done so any change in exe # path will be picked up) self.expt.manifest.add_filepath( @@ -299,7 +305,7 @@ def setup(self): self.exec_path_local, self.exec_path ) - + # Populate information about required dynamically loaded libraries self.required_libs = required_libs(self.exec_path) diff --git a/test/common.py b/test/common.py index 7b185c01..639bb16a 100644 --- a/test/common.py +++ b/test/common.py @@ -1,5 +1,6 @@ from contextlib import contextmanager import os +import stat from pathlib import Path import yaml @@ -120,9 +121,10 @@ def make_exe(): # Create a fake executable file bindir = labdir / 'bin' bindir.mkdir(parents=True, exist_ok=True) - exe = config['exe'] + exe_path = bindir / config['exe'] exe_size = 199 - make_random_file(bindir/exe, exe_size) + make_random_file(exe_path, exe_size) + exe_path.chmod(exe_path.stat().st_mode | stat.S_IEXEC) def make_payu_exe(): diff --git a/test/test_manifest.py b/test/test_manifest.py index 208c9aae..b51bd359 100644 --- a/test/test_manifest.py +++ b/test/test_manifest.py @@ -213,24 +213,6 @@ def test_exe_reproduce(): # Check manifests have changed as expected assert(not manifests == get_manifests(ctrldir/'manifests')) - # Reset manifests "truth" - manifests = get_manifests(ctrldir/'manifests') - - # Make exe in config.yaml unfindable by giving it a non-existent - # path but crucially the same name as the proper executable - config['exe'] = '/bogus/test.exe' - - # Change reproduce exe back to True - config['manifest']['reproduce']['exe'] = True - - write_config(config) - - # Run setup with changed exe but reproduce exe set to True. Should - # work fine as the exe path is in the manifest - payu_setup(lab_path=str(labdir)) - - assert(manifests == get_manifests(ctrldir/'manifests')) - def test_input_reproduce(): From b5c0c303ca2f1909ab7b2ce541bbcc6b5d24bff6 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Tue, 3 Oct 2023 11:33:53 +1100 Subject: [PATCH 3/3] add profiler runscripts to mpi-run command --- payu/experiment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/payu/experiment.py b/payu/experiment.py index 591d26b8..6b631a8e 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -561,7 +561,7 @@ def run(self, *user_flags): for prof in self.profilers: if prof.runscript: - model_prog = model_prog.append(prof.runscript) + model_prog.append(prof.runscript) model_prog.append(model.exec_prefix)