Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update build in benchcab once the updated CABLE build is released #258

Closed
ccarouge opened this issue Mar 1, 2024 · 5 comments · Fixed by #278 · May be fixed by #275
Closed

Update build in benchcab once the updated CABLE build is released #258

ccarouge opened this issue Mar 1, 2024 · 5 comments · Fixed by #278 · May be fixed by #275
Assignees
Labels
priority:high High priority issues that should be included in the next release.

Comments

@ccarouge
Copy link
Collaborator

ccarouge commented Mar 1, 2024

Once the CABLE build is modified to use CMake and Spack, the build in benchcab will break. We will need to update the build in benchcab to reflect the changes.

We will have to see if benchcab should use Spack or CMake directly to build CABLE.

SeanBryan51 added a commit to CABLE-LSM/CABLE that referenced this issue Mar 15, 2024
This pull request should follow after the CMake implementation (see
#200). This change adds the following build system changes:

- List source files in alphabetical order in CMakeLists.txt. The order
of source files was previously chosen so that we could demonstrate
binary equivalence in executables between the CMake build and the
Makefile build.
- Use [`find_package(MPI
REQUIRED)`](https://cmake.org/cmake/help/latest/module/FindMPI.html)
instead of specifying `mpif90` when invoking `cmake` for MPI case. This
is done so that
1. We can create separate targets for the serial and MPI executable and
only link against the MPI libraries when needed without relying on the
MPI compiler wrapper. This lets us compile all executables (serial and
parallel) with a single invocation of CMake.
2. We can use object libraries to compile the object files common to
both serial and MPI builds. This saves compiling these object files for
each executable (serial and MPI), reducing compilation time.
- Portability fixes and improvements to build.bash.
- Add `--compiler` flag to build.bash and GNU compiler support.
- Add Matthias's configuration for example.

Regression tests using
[benchcab](https://github.com/CABLE-LSM/benchcab)* (bitwise comparison
of model output via nccmp) show that model output is bitwise identical
between the current branch and the main branch for serial and MPI model
runs.

*executables were built manually as benchcab does not yet support the
recent build system changes (see related issue:
CABLE-LSM/benchcab#258).

Fixes #215

<!-- readthedocs-preview cable start -->
----
📚 Documentation preview 📚:
https://cable--216.org.readthedocs.build/en/216/

<!-- readthedocs-preview cable end -->
@SeanBryan51 SeanBryan51 self-assigned this Mar 17, 2024
@SeanBryan51
Copy link
Collaborator

SeanBryan51 commented Mar 18, 2024

Use the build script to build each model

As mentioned in Slack, it might be a good to work only with the build script again as a lot of the reasons why I removed the build script dependence are no longer relevant (see #138). There are a couple issues with the current way of specifying a build script that should be addressed first:

Use spack to build each model

Benchcab could use spack to build different versions of CABLE. Benchcab could invoke the following spack commands:

# 1. build CABLE serial and MPI executables from source for branch <branch_name>
# Note: this command will probably run for a while the first time as no dependencies are installed.
spack install cable@<branch_name> <spec for compiler and dependencies>

# 2. load CABLE spack package and symlink to executables
spack load cable@<branch_name>
ln -s $(which cable) path/to/experiment/dir
# or copy path to executable in payu config for MPI case
spack unload --all

# 3. load CABLE spack package and run the model
spack load cable@<branch_name>
cable cable.nml
# or `payu run` for MPI case
spack unload --all

With spack:

  • Only versions of CABLE that support the new CMake build system will be able to use spack.
  • Use spack dependencies instead of environment module dependencies.
  • Use spack install cable@<branch_name> instead of benchcab checkout(git clone ...) and benchcab build.
  • Use spack load instead of module load.

The requirement on benchcab to build legacy versions of cable means we will probably have to support the current way of doing things as well as doing things the spack. Supporting both ways seem like it would be hard to maintain. However, if we removed the requirement on benchcab to build legacy versions of cable, then moving solely to the spack way of doing things would be a good approach as using a tool or package manager to install different versions of CABLE instead of doing this manually is more modular and easier to maintain from the developer perspective.

In summary, I think using a build script seems like the easiest approach for solving this issue. IMO using spack is only worth it if we deprecate support for building and running legacy versions of CABLE as it considerable change to how benchcab currently does things.

@ccarouge
Copy link
Collaborator Author

We can't remove support for legacy versions yet. We will need to run benchcab on the CABLE-POP_TRENDY branch as part of CABLE4 work. Once CABLE4 is released, and the BIOS3 to CABLE-POP_TRENDY has been done (this should happen before CABLE4), we should be in a position to drop the support for legacy versions. There are a few other legacy branches, but these are smaller features and it should be possible to get them to be compatible with CABLE main without benchcab.

This means using a build script for now.

@SeanBryan51 SeanBryan51 added the priority:high High priority issues that should be included in the next release. label Mar 21, 2024
@SeanBryan51
Copy link
Collaborator

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.

@SeanBryan51 SeanBryan51 linked a pull request Mar 28, 2024 that will close this issue
2 tasks
@SeanBryan51
Copy link
Collaborator

SeanBryan51 commented Apr 3, 2024

After scoping out the work necessary for supporting spack (#275), it is worth considering whether we should use spack or invoke CMake directly for building CABLE.

Pros and cons - spack

Pros:

  1. Model agnostic approach to building models.
  2. Improves portability of benchcab to other machines.
  3. Using a package manager like spack to install different versions of CABLE is more modular and easier to maintain from the developer perspective than doing this manually. For example, moving to spack will simplify code and remove complex abstractions - resolving issues like:
  4. Fixes Should we parallelise the compilations across the versions? #130 due to recent CMake changes.

Cons:

  1. Requires significant changes to how benchcab currently does things (see Transition to spack workflow #275 for a discussion around implementing this).
  2. It might not be the right time to make the transition as:
    1. We currently do not have a formal release process for CABLE with spack which may shape how we design things.
    2. This requires that spack is accessible to users on Gadi. There currently is no spack "module" that can be loaded on Gadi and requires manual installation.

Pros and cons - CMake

Pros:

  1. Easy to implement (copy functionality of existing build script) and should not take long. This would be beneficial for anyone currently doing CABLE development and would allow them to test their development branches using the latest build system.
  2. Fixes Should we parallelise the compilations across the versions? #130 due to recent CMake changes.

Cons:

  1. Duplicate code between the build script and benchcab.

@ccarouge I think we should go with invoking CMake in benchcab for now as it is low effort and leave the spack implementation for now.

@SeanBryan51 SeanBryan51 linked a pull request Apr 4, 2024 that will close this issue
@ccarouge
Copy link
Collaborator Author

ccarouge commented Apr 4, 2024

OK. Let's go with CMake for now.

SeanBryan51 added a commit that referenced this issue Apr 4, 2024
The CABLE build system was transitioned from Makefile to CMake: see
CABLE-LSM/CABLE#216,
CABLE-LSM/CABLE#200.

This change adds support for building CABLE via CMake in benchcab so
that CABLE versions which branch from the HEAD of main can be tested.

Closes #258
SeanBryan51 added a commit that referenced this issue Apr 4, 2024
The CABLE build system was transitioned from Makefile to CMake: see
CABLE-LSM/CABLE#216,
CABLE-LSM/CABLE#200.

This change adds support for building CABLE via CMake in benchcab so
that CABLE versions which branch from the HEAD of main can be tested.

Closes #258
SeanBryan51 added a commit that referenced this issue Apr 5, 2024
The CABLE build system was transitioned from Makefile to CMake: see
CABLE-LSM/CABLE#216,
CABLE-LSM/CABLE#200.

This change adds support for building CABLE via CMake in benchcab so
that CABLE versions which branch from the HEAD of main can be tested.

Closes #258
SeanBryan51 added a commit that referenced this issue Apr 5, 2024
The CABLE build system was transitioned from Makefile to CMake: see
CABLE-LSM/CABLE#216,
CABLE-LSM/CABLE#200.

This change adds support for building CABLE via CMake in benchcab so
that CABLE versions which branch from the HEAD of main can be tested.

Closes #258
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:high High priority issues that should be included in the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants