From b725b82a7d5cc719f761eaf2e9cab69f25237d7a Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Fri, 3 May 2024 15:47:57 +1000 Subject: [PATCH 1/7] Get module executables from path loaded by environment modules - Move setting executable paths in Model class to Experiment.setup() - Inspect user modules (specified in config.yaml) to changes to PATH - The modules are restricted to user module directories defined in config.yaml - If exe is not an absolute path, search along above paths --- payu/envmod.py | 65 +++++++++++++++++++++++++++++++++++++++++++- payu/experiment.py | 21 ++++++++++---- payu/models/model.py | 64 +++++++++++++++++++++++++++++-------------- 3 files changed, 122 insertions(+), 28 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index aa391ce3..b7ae4a65 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -109,4 +109,67 @@ def lib_update(required_libs, lib_name): return '{0}/{1}'.format(mod_name, mod_version) # If there are no libraries, return an empty string - return '' \ No newline at end of file + return '' + + +def paths_set_by_user_modules(user_modules, user_modulepaths): + # Orginal environment + previous_env = dict(os.environ) + previous_modulepath = os.environ['MODULEPATH'] + + # Set restrict module path to only user defined module paths + os.environ['MODULEPATH'] = ':'.join(user_modulepaths) + + # Note: Using subprocess shell to isolate changes to environment + paths = [] + try: + # Get $PATH paths with no modules loaded + init_paths = paths_post_module_commands(["purge"]) + for module in user_modules: + # Check if module is available + cmd = f'{os.environ['MODULESHOME']}/bin/modulecmd bash is-avail {module}' + if run_cmd(cmd).returncode != 0: + continue + + #TODO: Check if multiple modules are available.. + + try: + # Get $PATH paths post running module purge && module load + paths.extend(paths_post_module_commands(['purge', + f'load {module}'])) + except subprocess.CalledProcessError as e: + continue + finally: + os.environ['MODULEPATH'] = previous_modulepath + + if previous_env != os.environ: + print( + "Warning: Env vars changed when inspecting paths set by modules" + ) + + # Remove inital paths and convert into a set + return set(paths).difference(set(init_paths)) + + +def paths_post_module_commands(commands): + """Runs subprocess module command and parse out the resulting + PATH environment variable""" + # Use modulecmd as module command is not available on compute nodes + module_cmds = [ + f'eval `{os.environ['MODULESHOME']}/bin/modulecmd bash {c}`' + for c in commands + ] + command = ' && '.join(module_cmds) + ' && echo $PATH' + + # Run Command and check the ouput + output = run_cmd(command) + output.check_returncode() + + # Extact out the PATH value, and split the paths + path = output.stdout.strip().split('\n')[-1] + return path.split(':') + + +def run_cmd(command): + """Wrapper around subprocess command""" + return subprocess.run(command, shell=True, text=True, capture_output=True) diff --git a/payu/experiment.py b/payu/experiment.py index e2c5de68..9db892a3 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -253,8 +253,7 @@ def load_modules(self): envmod.module('load', mod) # User-defined modules - user_modules = self.config.get('modules', {}).get('load', []) - for mod in user_modules: + for mod in self.user_modules: envmod.module('load', mod) envmod.module('list') @@ -414,6 +413,19 @@ def setup(self, force_archive=False): make_symlink(self.work_path, self.work_sym_path) + # Get paths added by user-modules to $PATH + envmod.setup() + self.user_modulepaths = self.config.get('modules', {}).get('use', []) + self.user_modules = self.config.get('modules', {}).get('load', []) + module_paths = envmod.paths_set_by_user_modules( + user_modules=self.user_modules, + user_modulepaths=self.user_modulepaths + ) + + # Set up model executable paths + for model in self.models: + model.setup_executable_paths(module_paths) + # Set up all file manifests self.manifest.setup() @@ -453,11 +465,8 @@ def setup(self, force_archive=False): self.get_restarts_to_prune() def run(self, *user_flags): - # XXX: This was previously done in reversion - envmod.setup() - # Add any user-defined module dir(s) to MODULEPATH - for module_dir in self.config.get('modules', {}).get('use', []): + for module_dir in self.user_modulepaths: envmod.module('use', module_dir) self.load_modules() diff --git a/payu/models/model.py b/payu/models/model.py index 9d4365aa..bf933110 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -82,21 +82,6 @@ def set_model_pathnames(self): self.work_output_path = self.work_path self.work_init_path = self.work_path - self.exec_prefix = self.config.get('exe_prefix', '') - self.exec_name = self.config.get('exe', self.default_exec) - if self.exec_name: - # By default os.path.join will not prepend the lab bin_path - # to an absolute path - self.exec_path = os.path.join(self.expt.lab.bin_path, - self.exec_name) - else: - self.exec_path = None - if self.exec_path: - # Make exec_name consistent for models with fully qualified path. - # In all cases it will just be the name of the executable without a - # path - self.exec_name = os.path.basename(self.exec_path) - def set_local_pathnames(self): # This is the path relative to the control directory, required for @@ -129,12 +114,6 @@ def set_local_pathnames(self): os.path.relpath(self.work_init_path, self.expt.work_path) ) ) - if self.exec_path: - # Local path in work directory - self.exec_path_local = os.path.join( - self.work_path_local, - os.path.basename(self.exec_path) - ) def set_input_paths(self): if len(self.expt.models) == 1: @@ -198,6 +177,49 @@ def get_prior_restart_files(self): print("No prior restart files found: {error}".format(error=str(e))) return [] + def setup_executable_paths(self, module_added_paths): + """Set model executable paths""" + self.exec_prefix = self.config.get('exe_prefix', '') + self.exec_name = self.config.get('exe', self.default_exec) + self.exec_path = None + if self.exec_name: + if os.path.isabs(self.exec_name): + # Use absolute path + self.exec_path = self.exec_name + else: + # Check for executable inside paths added by user-modules + exec_paths = [] + for path in module_added_paths: + exec_path = os.path.join(path, self.exec_name) + if os.path.exists(exec_path): + exec_paths.append(exec_path) + + if len(exec_paths) > 1: + # Will this ever happen? + raise ValueError( + f"Executable: {self.exec_name} found in multiple " + + f"module paths: {exec_paths}" + ) + elif len(exec_paths) == 1: + self.exec_path = exec_paths[0] + print( + f"Expanded model exectuable path to: {self.exec_path}") + else: + # Prepend the lab bin path + self.exec_path = os.path.join(self.expt.lab.bin_path, + self.exec_name) + + # Make exec_name consistent for models with fully qualified path. + # In all cases it will just be the name of the executable without a + # path + self.exec_name = os.path.basename(self.exec_path) + + # Local path in work directory + self.exec_path_local = os.path.join( + self.work_path_local, + os.path.basename(self.exec_path) + ) + def setup_configuration_files(self): """Copy configuration and optional configuration files from control path to work path""" From 3bffb9ea4528a7e586c073ba92dc7c44f12bcc7f Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Fri, 3 May 2024 16:05:00 +1000 Subject: [PATCH 2/7] Fix codestyle --- payu/envmod.py | 17 ++++++++--------- payu/models/model.py | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index b7ae4a65..84736048 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -113,7 +113,7 @@ def lib_update(required_libs, lib_name): def paths_set_by_user_modules(user_modules, user_modulepaths): - # Orginal environment + # Orginal environment previous_env = dict(os.environ) previous_modulepath = os.environ['MODULEPATH'] @@ -127,12 +127,11 @@ def paths_set_by_user_modules(user_modules, user_modulepaths): init_paths = paths_post_module_commands(["purge"]) for module in user_modules: # Check if module is available - cmd = f'{os.environ['MODULESHOME']}/bin/modulecmd bash is-avail {module}' + module_cmd = "{os.environ['MODULESHOME']}/bin/modulecmd bash" + cmd = f"{module_cmd} is-avail {module}" if run_cmd(cmd).returncode != 0: continue - - #TODO: Check if multiple modules are available.. - + # TODO: Check if multiple modules are available.. try: # Get $PATH paths post running module purge && module load paths.extend(paths_post_module_commands(['purge', @@ -147,16 +146,16 @@ def paths_set_by_user_modules(user_modules, user_modulepaths): "Warning: Env vars changed when inspecting paths set by modules" ) - # Remove inital paths and convert into a set + # Remove inital paths and convert into a set return set(paths).difference(set(init_paths)) def paths_post_module_commands(commands): - """Runs subprocess module command and parse out the resulting + """Runs subprocess module command and parse out the resulting PATH environment variable""" # Use modulecmd as module command is not available on compute nodes module_cmds = [ - f'eval `{os.environ['MODULESHOME']}/bin/modulecmd bash {c}`' + f"eval `{os.environ['MODULESHOME']}/bin/modulecmd bash {c}`" for c in commands ] command = ' && '.join(module_cmds) + ' && echo $PATH' @@ -165,7 +164,7 @@ def paths_post_module_commands(commands): output = run_cmd(command) output.check_returncode() - # Extact out the PATH value, and split the paths + # Extact out the PATH value, and split the paths path = output.stdout.strip().split('\n')[-1] return path.split(':') diff --git a/payu/models/model.py b/payu/models/model.py index bf933110..ba6a3ca1 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -205,7 +205,7 @@ def setup_executable_paths(self, module_added_paths): print( f"Expanded model exectuable path to: {self.exec_path}") else: - # Prepend the lab bin path + # Prepend the lab bin path self.exec_path = os.path.join(self.expt.lab.bin_path, self.exec_name) From 5694dc34f2eb61331bbd79b26e7ec507d3bcda57 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Fri, 3 May 2024 16:22:44 +1000 Subject: [PATCH 3/7] Skip searching module paths if no MODULESHOME --- payu/envmod.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/payu/envmod.py b/payu/envmod.py index 84736048..ce3d97e9 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -113,6 +113,14 @@ def lib_update(required_libs, lib_name): def paths_set_by_user_modules(user_modules, user_modulepaths): + """Search along changes PATH added by user defined modules + and return a set of paths added - this is used for + searching for the model executable""" + if 'MODULESHOME' not in os.environ: + print('payu: warning: No Environment Modules found; skipping ' + 'inspecting user module changes to PATH') + return set() + # Orginal environment previous_env = dict(os.environ) previous_modulepath = os.environ['MODULEPATH'] From 9d0e85d86843d46e3b634272cee541953a7a0346 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Mon, 6 May 2024 13:17:52 +1000 Subject: [PATCH 4/7] Expand model exexutable path for collate executable - Refactor experiment module and model expand executable path functions - Run module setup as part of collate - Ignore module paths when setting executable path in model.build --- payu/envmod.py | 2 +- payu/experiment.py | 49 ++++++++++++++++++++++-------------- payu/models/fms.py | 16 ++++++++---- payu/models/model.py | 59 +++++++++++++++++++++++++------------------- 4 files changed, 76 insertions(+), 50 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index ce3d97e9..5be75fd4 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -135,7 +135,7 @@ def paths_set_by_user_modules(user_modules, user_modulepaths): init_paths = paths_post_module_commands(["purge"]) for module in user_modules: # Check if module is available - module_cmd = "{os.environ['MODULESHOME']}/bin/modulecmd bash" + module_cmd = f"{os.environ['MODULESHOME']}/bin/modulecmd bash" cmd = f"{module_cmd} is-avail {module}" if run_cmd(cmd).returncode != 0: continue diff --git a/payu/experiment.py b/payu/experiment.py index 9db892a3..4b1b7be6 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -220,9 +220,29 @@ def set_stacksize(self, stacksize): resource.setrlimit(resource.RLIMIT_STACK, (stacksize, resource.RLIM_INFINITY)) - def load_modules(self): - # NOTE: This function is increasingly irrelevant, and may be removable. + def setup_modules(self): + """Setup modules and get paths added to $PATH by user-modules""" + envmod.setup() + + # Get user modules info from config + self.user_modulepaths = self.config.get('modules', {}).get('use', []) + self.user_modules = self.config.get('modules', {}).get('load', []) + + # Get paths added to $PATH by user-modules + self.user_modules_set_paths = envmod.paths_set_by_user_modules( + user_modules=self.user_modules, + user_modulepaths=self.user_modulepaths + ) + + def run_modules(self): + """Run module load + use commands""" + # Add any user-defined module dir(s) to MODULEPATH + for module_dir in self.user_modulepaths: + envmod.module('use', module_dir) + + self.load_modules() + def load_modules(self): # Scheduler sched_modname = self.config.get('scheduler', 'pbs') self.modules.add(sched_modname) @@ -413,18 +433,10 @@ def setup(self, force_archive=False): make_symlink(self.work_path, self.work_sym_path) - # Get paths added by user-modules to $PATH - envmod.setup() - self.user_modulepaths = self.config.get('modules', {}).get('use', []) - self.user_modules = self.config.get('modules', {}).get('load', []) - module_paths = envmod.paths_set_by_user_modules( - user_modules=self.user_modules, - user_modulepaths=self.user_modulepaths - ) - - # Set up model executable paths + # Set up executable paths - first search through paths added by modules + self.setup_modules() for model in self.models: - model.setup_executable_paths(module_paths) + model.setup_executable_paths() # Set up all file manifests self.manifest.setup() @@ -465,11 +477,8 @@ def setup(self, force_archive=False): self.get_restarts_to_prune() def run(self, *user_flags): - # Add any user-defined module dir(s) to MODULEPATH - for module_dir in self.user_modulepaths: - envmod.module('use', module_dir) - - self.load_modules() + # Run module use and load commands + self.run_modules() f_out = open(self.stdout_fname, 'w') f_err = open(self.stderr_fname, 'w') @@ -813,6 +822,10 @@ def archive(self, force_prune_restarts=False): self.postprocess() def collate(self): + # Search module added paths & run module use + load commands + self.setup_modules() + self.run_modules() + for model in self.models: model.collate() diff --git a/payu/models/fms.py b/payu/models/fms.py index f2f3e743..fb0247a8 100644 --- a/payu/models/fms.py +++ b/payu/models/fms.py @@ -76,9 +76,16 @@ def fms_collate(model): mpi = collate_config.get('mpi', False) if mpi: - # Must use envmod to be able to load mpi modules for collation - envmod.setup() - model.expt.load_modules() + # TODO: I've added module setup and load functions to + # Experiment.collate - as user modules needed to be use & loaded + # to expand executable paths. However, is loading mpi module + # for non-mpi collate jobs going to cause errors? + # If so, need to only use/load user modules in Experiment.collate + # and only load additional modules for mpi here + + # # Must use envmod to be able to load mpi modules for collation + # envmod.setup() + # model.expt.load_modules() default_exe = 'mppnccombine-fast' else: default_exe = 'mppnccombine' @@ -92,8 +99,7 @@ def fms_collate(model): mppnc_path = os.path.join(model.expt.lab.bin_path, f) break else: - if not os.path.isabs(mppnc_path): - mppnc_path = os.path.join(model.expt.lab.bin_path, mppnc_path) + mppnc_path = model.expand_executable_path(mppnc_path) assert mppnc_path, 'No mppnccombine program found' diff --git a/payu/models/model.py b/payu/models/model.py index ba6a3ca1..74c7562a 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -177,37 +177,42 @@ def get_prior_restart_files(self): print("No prior restart files found: {error}".format(error=str(e))) return [] - def setup_executable_paths(self, module_added_paths): + def expand_executable_path(self, exec, search_module_path=True): + """Given an executable, return the expanded executable path""" + # Check if exe is already an absolute path + if os.path.isabs(exec): + return exec + + if not search_module_path: + module_added_paths = set() + else: + module_added_paths = self.expt.user_modules_set_paths + + # Search for exe inside paths added to $PATH by user-defined modules + exec_paths = [] + for path in module_added_paths: + exec_path = os.path.join(path, exec) + if os.path.exists(exec_path): + exec_paths.append(exec_path) + + if len(exec_paths) > 1: + raise ValueError( + f"Executable {exec} found in multiple $PATH paths added by " + + f"user-defined modules in `config.yaml`. Paths: {exec_paths}") + elif len(exec_paths) == 1: + return exec_paths[0] + + # Else prepend the lab bin path to exec + return os.path.join(self.expt.lab.bin_path, exec) + + def setup_executable_paths(self, search_module_paths=True): """Set model executable paths""" self.exec_prefix = self.config.get('exe_prefix', '') self.exec_name = self.config.get('exe', self.default_exec) self.exec_path = None if self.exec_name: - if os.path.isabs(self.exec_name): - # Use absolute path - self.exec_path = self.exec_name - else: - # Check for executable inside paths added by user-modules - exec_paths = [] - for path in module_added_paths: - exec_path = os.path.join(path, self.exec_name) - if os.path.exists(exec_path): - exec_paths.append(exec_path) - - if len(exec_paths) > 1: - # Will this ever happen? - raise ValueError( - f"Executable: {self.exec_name} found in multiple " + - f"module paths: {exec_paths}" - ) - elif len(exec_paths) == 1: - self.exec_path = exec_paths[0] - print( - f"Expanded model exectuable path to: {self.exec_path}") - else: - # Prepend the lab bin path - self.exec_path = os.path.join(self.expt.lab.bin_path, - self.exec_name) + self.exec_path = self.expand_executable_path(self.exec_name, + search_module_paths) # Make exec_name consistent for models with fully qualified path. # In all cases it will just be the name of the executable without a @@ -361,6 +366,8 @@ def collate(self): raise NotImplementedError def build_model(self): + # Don't search user modules for executable paths + self.setup_executable_paths(search_module_paths=False) if not self.repo_url: return From 7b7db965e10529ef01703e1f7e5e8f6e912cafe1 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Wed, 8 May 2024 08:43:09 +1000 Subject: [PATCH 5/7] Add checks for user modules - Loading user modules and modulepaths in experiment.setup to check if multiple modules are available - In collate, only load user modules so load_modules() is run, only when mpi modules are needed - Changed inspecting path over all modulepaths (including the ones added using `module use`) to reflect the module that will actually be loaded --- payu/envmod.py | 134 +++++++++++++++++++++++++------------------ payu/experiment.py | 33 +++++------ payu/models/fms.py | 12 +--- payu/models/model.py | 4 +- 4 files changed, 94 insertions(+), 89 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index 5be75fd4..6061b183 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -17,6 +17,21 @@ DEFAULT_BASEPATH = '/opt/Modules' DEFAULT_VERSION = 'v4.3.0' +MODULE_NOT_FOUND_HELP = """ To fix module not being found: +- Check module name and version in config.yaml (listed under `modules: load:`) +- If module is found in a module directory, ensure this path is listed in +config.yaml under `modules: use:`, or run `module use` command prior to running +payu commands. +""" + +MULTIPLE_MODULES_HELP = """ To fix having multiple modules available: +- Add version to the module in config.yaml (under `modules: load:`) +- Modify module directories in config.yaml (under `modules: use:`) +- Or modify module directories in user environment by using module use/unuse +commands, e.g.: + $ module use dir # Add dir to $MODULEPATH + $ module unuse dir # Remove dir from $MODULEPATH +""" def setup(basepath=DEFAULT_BASEPATH): """Set the environment modules used by the Environment Module system.""" @@ -112,71 +127,76 @@ def lib_update(required_libs, lib_name): return '' -def paths_set_by_user_modules(user_modules, user_modulepaths): - """Search along changes PATH added by user defined modules - and return a set of paths added - this is used for - searching for the model executable""" - if 'MODULESHOME' not in os.environ: - print('payu: warning: No Environment Modules found; skipping ' - 'inspecting user module changes to PATH') - return set() +def setup_user_modules(user_modules, user_modulepaths): + """Run module use + load commands for user-defined modules""" - # Orginal environment - previous_env = dict(os.environ) - previous_modulepath = os.environ['MODULEPATH'] - - # Set restrict module path to only user defined module paths - os.environ['MODULEPATH'] = ':'.join(user_modulepaths) - - # Note: Using subprocess shell to isolate changes to environment - paths = [] - try: - # Get $PATH paths with no modules loaded - init_paths = paths_post_module_commands(["purge"]) - for module in user_modules: - # Check if module is available - module_cmd = f"{os.environ['MODULESHOME']}/bin/modulecmd bash" - cmd = f"{module_cmd} is-avail {module}" - if run_cmd(cmd).returncode != 0: - continue - # TODO: Check if multiple modules are available.. - try: - # Get $PATH paths post running module purge && module load - paths.extend(paths_post_module_commands(['purge', - f'load {module}'])) - except subprocess.CalledProcessError as e: - continue - finally: - os.environ['MODULEPATH'] = previous_modulepath - - if previous_env != os.environ: + if 'MODULESHOME' not in os.environ: print( - "Warning: Env vars changed when inspecting paths set by modules" - ) - - # Remove inital paths and convert into a set - return set(paths).difference(set(init_paths)) - + 'payu: warning: No Environment Modules found; ' + + 'skipping running module use/load commands for any module ' + + 'directories/modulefiles defined in config.yaml') + return -def paths_post_module_commands(commands): - """Runs subprocess module command and parse out the resulting - PATH environment variable""" - # Use modulecmd as module command is not available on compute nodes - module_cmds = [ - f"eval `{os.environ['MODULESHOME']}/bin/modulecmd bash {c}`" - for c in commands - ] - command = ' && '.join(module_cmds) + ' && echo $PATH' + # Add user-defined directories to MODULEPATH + for modulepath in user_modulepaths: + if not os.path.isdir(modulepath): + raise ValueError( + f"Module directory is not found: {modulepath}" + + "\n Check paths listed under `modules: use:` in config.yaml") + + module('use', modulepath) + + for modulefile in user_modules: + # Check module exists and there is not multiple available + module_subcommand = f"avail --terse {modulefile}" + output = run_cmd(module_cmd(module_subcommand)).stderr + + # Extract out the modulefiles available + modules = [line for line in output.strip().splitlines() + if not (line.startswith('/') and line.endswith(':'))] + + if len(modules) > 1: + # Modules are used for finding model executable paths - so check + # for unique module -TODO: Could be a warning rather than an error? + raise ValueError( + f"There are multiple modules available for {modulefile}:\n" + + f"{output}\n{MULTIPLE_MODULES_HELP}") + elif len(modules) == 0: + raise ValueError( + f"Module is not found: {modulefile}\n{MODULE_NOT_FOUND_HELP}" + ) + + # Load module + module('load', modulefile) + + +def env_var_set_by_modules(user_modules, env_var): + """Return an environment variable post loading only user-defined modules + - this is used for getting $PATH for searching for the model executable""" + if 'MODULESHOME' not in os.environ: + print('payu: warning: No Environment Modules found; skipping ' + f'inspecting user module changes to ${env_var}') + return [] - # Run Command and check the ouput + # Note: Using subprocess shell to isolate changes to environment + load_commands = [f'load {module}' for module in user_modules] + commands = ['purge'] + load_commands + module_cmds = [f"eval `{module_cmd(c)}`" for c in commands] + module_cmds += [f'echo ${env_var}'] + command = ' && '.join(module_cmds) output = run_cmd(command) + + # Extract out $env_var from output output.check_returncode() + lines = output.stdout.strip().split('\n') + return lines[-1] + - # Extact out the PATH value, and split the paths - path = output.stdout.strip().split('\n')[-1] - return path.split(':') +def module_cmd(command): + """Format module subcommand using modulecmd""" + return f"{os.environ['MODULESHOME']}/bin/modulecmd bash {command}" def run_cmd(command): - """Wrapper around subprocess command""" + """Wrapper around subprocess command that captures output""" return subprocess.run(command, shell=True, text=True, capture_output=True) diff --git a/payu/experiment.py b/payu/experiment.py index 4b1b7be6..7f826831 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -228,19 +228,17 @@ def setup_modules(self): self.user_modulepaths = self.config.get('modules', {}).get('use', []) self.user_modules = self.config.get('modules', {}).get('load', []) - # Get paths added to $PATH by user-modules - self.user_modules_set_paths = envmod.paths_set_by_user_modules( - user_modules=self.user_modules, - user_modulepaths=self.user_modulepaths - ) + # Run module use + load commands for user-defined modules + envmod.setup_user_modules(self.user_modules, self.user_modulepaths) - def run_modules(self): - """Run module load + use commands""" - # Add any user-defined module dir(s) to MODULEPATH - for module_dir in self.user_modulepaths: - envmod.module('use', module_dir) + # Get paths and loaded modules post loading only the user modules + self.user_modules_paths = envmod.env_var_set_by_modules( + self.user_modules, 'PATH' + ).split(':') - self.load_modules() + self.loaded_user_modules = envmod.env_var_set_by_modules( + self.user_modules, 'LOADEDMODULES' + ).split(':') def load_modules(self): # Scheduler @@ -265,17 +263,14 @@ def load_modules(self): if len(mod) > 0: print('mod '+mod) mod_base = mod.split('/')[0] - if mod_base not in core_modules: + if (mod_base not in core_modules and + mod not in self.loaded_user_modules): envmod.module('unload', mod) # Now load model-dependent modules for mod in self.modules: envmod.module('load', mod) - # User-defined modules - for mod in self.user_modules: - envmod.module('load', mod) - envmod.module('list') for prof in self.profilers: @@ -477,8 +472,7 @@ def setup(self, force_archive=False): self.get_restarts_to_prune() def run(self, *user_flags): - # Run module use and load commands - self.run_modules() + self.load_modules() f_out = open(self.stdout_fname, 'w') f_err = open(self.stderr_fname, 'w') @@ -822,9 +816,8 @@ def archive(self, force_prune_restarts=False): self.postprocess() def collate(self): - # Search module added paths & run module use + load commands + # Setup modules - load user-defined modules self.setup_modules() - self.run_modules() for model in self.models: model.collate() diff --git a/payu/models/fms.py b/payu/models/fms.py index fb0247a8..b5f9e35a 100644 --- a/payu/models/fms.py +++ b/payu/models/fms.py @@ -76,16 +76,8 @@ def fms_collate(model): mpi = collate_config.get('mpi', False) if mpi: - # TODO: I've added module setup and load functions to - # Experiment.collate - as user modules needed to be use & loaded - # to expand executable paths. However, is loading mpi module - # for non-mpi collate jobs going to cause errors? - # If so, need to only use/load user modules in Experiment.collate - # and only load additional modules for mpi here - - # # Must use envmod to be able to load mpi modules for collation - # envmod.setup() - # model.expt.load_modules() + # Load mpi modules for collation + model.expt.load_modules() default_exe = 'mppnccombine-fast' else: default_exe = 'mppnccombine' diff --git a/payu/models/model.py b/payu/models/model.py index 74c7562a..20e25c88 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -184,9 +184,9 @@ def expand_executable_path(self, exec, search_module_path=True): return exec if not search_module_path: - module_added_paths = set() + module_added_paths = [] else: - module_added_paths = self.expt.user_modules_set_paths + module_added_paths = self.expt.user_modules_paths # Search for exe inside paths added to $PATH by user-defined modules exec_paths = [] From 6ce00f9411320c9b501c7666898282d1261b4641 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Wed, 8 May 2024 09:29:15 +1000 Subject: [PATCH 6/7] Fix handling when PATH set by modules is not defined - When searching paths for executable, check if file is executable --- payu/envmod.py | 5 +++-- payu/experiment.py | 13 +++++++++---- payu/models/model.py | 22 +++++++++++++--------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index 6061b183..b7b62dae 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -33,6 +33,7 @@ $ module unuse dir # Remove dir from $MODULEPATH """ + def setup(basepath=DEFAULT_BASEPATH): """Set the environment modules used by the Environment Module system.""" @@ -153,7 +154,7 @@ def setup_user_modules(user_modules, user_modulepaths): # Extract out the modulefiles available modules = [line for line in output.strip().splitlines() - if not (line.startswith('/') and line.endswith(':'))] + if not (line.startswith('/') and line.endswith(':'))] if len(modules) > 1: # Modules are used for finding model executable paths - so check @@ -176,7 +177,7 @@ def env_var_set_by_modules(user_modules, env_var): if 'MODULESHOME' not in os.environ: print('payu: warning: No Environment Modules found; skipping ' f'inspecting user module changes to ${env_var}') - return [] + return # Note: Using subprocess shell to isolate changes to environment load_commands = [f'load {module}' for module in user_modules] diff --git a/payu/experiment.py b/payu/experiment.py index 7f826831..090b712e 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -132,6 +132,8 @@ def __init__(self, lab, reproduce=False, force=False): self.run_id = None + self.user_modules_path = None + def init_models(self): self.model_name = self.config.get('model') @@ -232,13 +234,16 @@ def setup_modules(self): envmod.setup_user_modules(self.user_modules, self.user_modulepaths) # Get paths and loaded modules post loading only the user modules - self.user_modules_paths = envmod.env_var_set_by_modules( + self.user_modules_path = envmod.env_var_set_by_modules( self.user_modules, 'PATH' - ).split(':') + ) + # Store list of all modules loaded by user-modules self.loaded_user_modules = envmod.env_var_set_by_modules( self.user_modules, 'LOADEDMODULES' - ).split(':') + ) + if self.loaded_user_modules is not None: + self.loaded_user_modules = self.loaded_user_modules.split(':') def load_modules(self): # Scheduler @@ -264,7 +269,7 @@ def load_modules(self): print('mod '+mod) mod_base = mod.split('/')[0] if (mod_base not in core_modules and - mod not in self.loaded_user_modules): + mod not in self.loaded_user_modules): envmod.module('unload', mod) # Now load model-dependent modules diff --git a/payu/models/model.py b/payu/models/model.py index 20e25c88..91838d69 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -177,22 +177,28 @@ def get_prior_restart_files(self): print("No prior restart files found: {error}".format(error=str(e))) return [] - def expand_executable_path(self, exec, search_module_path=True): + def expand_executable_path(self, exec): """Given an executable, return the expanded executable path""" # Check if exe is already an absolute path if os.path.isabs(exec): return exec - if not search_module_path: + # Check if path set by loading user modules has been defined + module_added_path = self.expt.user_modules_path + if module_added_path is None: + print("payu: warning: Skipping searching for model executable " + + "in $PATH set by user modules") + module_added_paths = [] + elif module_added_path == '': module_added_paths = [] else: - module_added_paths = self.expt.user_modules_paths + module_added_paths = module_added_path.split(':') # Search for exe inside paths added to $PATH by user-defined modules exec_paths = [] for path in module_added_paths: exec_path = os.path.join(path, exec) - if os.path.exists(exec_path): + if os.path.exists(exec_path) and os.access(exec_path, os.X_OK): exec_paths.append(exec_path) if len(exec_paths) > 1: @@ -205,14 +211,13 @@ def expand_executable_path(self, exec, search_module_path=True): # Else prepend the lab bin path to exec return os.path.join(self.expt.lab.bin_path, exec) - def setup_executable_paths(self, search_module_paths=True): + def setup_executable_paths(self): """Set model executable paths""" self.exec_prefix = self.config.get('exe_prefix', '') self.exec_name = self.config.get('exe', self.default_exec) self.exec_path = None if self.exec_name: - self.exec_path = self.expand_executable_path(self.exec_name, - search_module_paths) + self.exec_path = self.expand_executable_path(self.exec_name) # Make exec_name consistent for models with fully qualified path. # In all cases it will just be the name of the executable without a @@ -366,8 +371,7 @@ def collate(self): raise NotImplementedError def build_model(self): - # Don't search user modules for executable paths - self.setup_executable_paths(search_module_paths=False) + self.setup_executable_paths() if not self.repo_url: return From bbcd943a1a18d56c1c9be94a7534bfb8f09fc7a6 Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Tue, 14 May 2024 15:21:03 +1000 Subject: [PATCH 7/7] Update envmod.setup_user_modules to inspect PATH and LOADEDMODULES before/after loading modules --- payu/envmod.py | 63 ++++++++++++++++++++------------------------ payu/experiment.py | 28 +++++++------------- payu/models/model.py | 8 ++---- 3 files changed, 41 insertions(+), 58 deletions(-) diff --git a/payu/envmod.py b/payu/envmod.py index b7b62dae..cfa13e13 100644 --- a/payu/envmod.py +++ b/payu/envmod.py @@ -129,14 +129,17 @@ def lib_update(required_libs, lib_name): def setup_user_modules(user_modules, user_modulepaths): - """Run module use + load commands for user-defined modules""" + """Run module use + load commands for user-defined modules. Return + tuple containing a set of loaded modules and paths added to + LOADEDMODULES and PATH environment variable, as result of + loading user-modules""" if 'MODULESHOME' not in os.environ: print( 'payu: warning: No Environment Modules found; ' + 'skipping running module use/load commands for any module ' + 'directories/modulefiles defined in config.yaml') - return + return (None, None) # Add user-defined directories to MODULEPATH for modulepath in user_modulepaths: @@ -147,18 +150,26 @@ def setup_user_modules(user_modules, user_modulepaths): module('use', modulepath) + # First un-load all user modules, if they are loaded, so can store + # LOADEDMODULES and PATH to compare to later + for modulefile in user_modules: + if run_module_cmd("is-loaded", modulefile).returncode == 0: + module('unload', modulefile) + previous_loaded_modules = os.environ.get('LOADEDMODULES', '') + previous_path = os.environ.get('PATH', '') + for modulefile in user_modules: # Check module exists and there is not multiple available - module_subcommand = f"avail --terse {modulefile}" - output = run_cmd(module_cmd(module_subcommand)).stderr + output = run_module_cmd("avail --terse", modulefile).stderr - # Extract out the modulefiles available + # Extract out the modulefiles available - strip out lines like: + # /apps/Modules/modulefiles: modules = [line for line in output.strip().splitlines() if not (line.startswith('/') and line.endswith(':'))] + # Modules are used for finding model executable paths - so check + # for unique module if len(modules) > 1: - # Modules are used for finding model executable paths - so check - # for unique module -TODO: Could be a warning rather than an error? raise ValueError( f"There are multiple modules available for {modulefile}:\n" + f"{output}\n{MULTIPLE_MODULES_HELP}") @@ -170,34 +181,18 @@ def setup_user_modules(user_modules, user_modulepaths): # Load module module('load', modulefile) + # Create a set of paths and modules loaded by user modules + loaded_modules = os.environ.get('LOADEDMODULES', '') + path = os.environ.get('PATH', '') + loaded_modules = set(loaded_modules.split(':')).difference( + previous_loaded_modules.split(':')) + paths = set(path.split(':')).difference(set(previous_path.split(':'))) -def env_var_set_by_modules(user_modules, env_var): - """Return an environment variable post loading only user-defined modules - - this is used for getting $PATH for searching for the model executable""" - if 'MODULESHOME' not in os.environ: - print('payu: warning: No Environment Modules found; skipping ' - f'inspecting user module changes to ${env_var}') - return - - # Note: Using subprocess shell to isolate changes to environment - load_commands = [f'load {module}' for module in user_modules] - commands = ['purge'] + load_commands - module_cmds = [f"eval `{module_cmd(c)}`" for c in commands] - module_cmds += [f'echo ${env_var}'] - command = ' && '.join(module_cmds) - output = run_cmd(command) - - # Extract out $env_var from output - output.check_returncode() - lines = output.stdout.strip().split('\n') - return lines[-1] - - -def module_cmd(command): - """Format module subcommand using modulecmd""" - return f"{os.environ['MODULESHOME']}/bin/modulecmd bash {command}" + return (loaded_modules, paths) -def run_cmd(command): - """Wrapper around subprocess command that captures output""" +def run_module_cmd(subcommand, *args): + """Wrapper around subprocess module command that captures output""" + modulecmd = f"{os.environ['MODULESHOME']}/bin/modulecmd bash" + command = f"{modulecmd} {subcommand} {' '.join(args)}" return subprocess.run(command, shell=True, text=True, capture_output=True) diff --git a/payu/experiment.py b/payu/experiment.py index 090b712e..0106ea53 100644 --- a/payu/experiment.py +++ b/payu/experiment.py @@ -132,7 +132,7 @@ def __init__(self, lab, reproduce=False, force=False): self.run_id = None - self.user_modules_path = None + self.user_modules_paths = None def init_models(self): @@ -227,23 +227,15 @@ def setup_modules(self): envmod.setup() # Get user modules info from config - self.user_modulepaths = self.config.get('modules', {}).get('use', []) - self.user_modules = self.config.get('modules', {}).get('load', []) - - # Run module use + load commands for user-defined modules - envmod.setup_user_modules(self.user_modules, self.user_modulepaths) - - # Get paths and loaded modules post loading only the user modules - self.user_modules_path = envmod.env_var_set_by_modules( - self.user_modules, 'PATH' - ) - - # Store list of all modules loaded by user-modules - self.loaded_user_modules = envmod.env_var_set_by_modules( - self.user_modules, 'LOADEDMODULES' - ) - if self.loaded_user_modules is not None: - self.loaded_user_modules = self.loaded_user_modules.split(':') + user_modulepaths = self.config.get('modules', {}).get('use', []) + user_modules = self.config.get('modules', {}).get('load', []) + + # Run module use + load commands for user-defined modules, and + # get a set of paths and loaded modules added by loading the modules + loaded_mods, paths = envmod.setup_user_modules(user_modules, + user_modulepaths) + self.user_modules_paths = paths + self.loaded_user_modules = [] if loaded_mods is None else loaded_mods def load_modules(self): # Scheduler diff --git a/payu/models/model.py b/payu/models/model.py index 91838d69..43200eaf 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -184,15 +184,11 @@ def expand_executable_path(self, exec): return exec # Check if path set by loading user modules has been defined - module_added_path = self.expt.user_modules_path - if module_added_path is None: + module_added_paths = self.expt.user_modules_paths + if module_added_paths is None: print("payu: warning: Skipping searching for model executable " + "in $PATH set by user modules") module_added_paths = [] - elif module_added_path == '': - module_added_paths = [] - else: - module_added_paths = module_added_path.split(':') # Search for exe inside paths added to $PATH by user-defined modules exec_paths = []