search

CABLE-LSM / benchcab

Tool for evaluation of CABLE land surface model
https://benchcab.readthedocs.io/en/latest/
Apache License 2.0
2 stars 3 forks source link

Function `remove_module_lines` can produce an invalid bash script #249

Closed SeanBryan51 closed 6 months ago

SeanBryan51 commented 8 months ago

For compiling CABLE with a custom build script, benchcab will try to remove any lines that invoke the module command in the build script so that benchcab can load the modules specified in config.yaml when compiling the executables. This ensures that when we run CABLE we have the correct module dependencies loaded (see related issue: https://github.com/CABLE-LSM/benchcab/issues/94). This is done via the remove_module_lines function:

https://github.com/CABLE-LSM/benchcab/blob/dd9bc12e554b218b1090c31ffaa41cdafd10fc7d/benchcab/model.py#L154-L162

However, this approach of removing lines from the bash script is brittle and can easily produce a broken bash script. For example, a bash script snippet such as the following would break when applying the above function:

if condition; then
    module load foo
fi

I think it is best to avoid modifying the build script as much as possible.

We could instead disable module invocations by creating a wrapper around the module command which prevents any module load or module add statements from being executed. To invoke the wrapper instead of the actual command when calling subprocess.run, we can prepend the path to the wrapper to the PATH environment variable (see here for using python subprocess with a modified environment). This approach could be relevant for #244 if the user specifies a list of shell commands to compile CABLE instead of a build script path.

SeanBryan51 commented 7 months ago

I'm going to attempt to implement this, we'll see how I go.

SeanBryan51 commented 6 months ago

After discussing with @ccarouge, we have decided to add support for spack and to deprecate support for legacy CABLE build systems in benchcab. This adds the requirement that users will only be able to test benchcab on branches that can be built by spack. ACCESS-NRI will help with transitioning legacy build systems to use the latest build system compatible with spack.

Moving to spack will solve existing issues around supporting bespoke build scripts (e.g. #244, #249). This is the best way forward considering support for legacy build systems is a temporary that will be deprecated sooner or later.

Closing this issue.