Additional CMake changes

[SeanBryan51]

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) 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* (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

---

📚 Documentation preview 📚: https://cable--216.org.readthedocs.build/en/216/

[mcuntz]

Dear @SeanBryan51 ,

very good that the build system got an update.

Being a community model, it would be good if it would work on more than one computing system and with more than one compiler
I looked into build.bash and CMakeLIsts.txt and it is not obvious to me to add a new compiler system.
I am running CABLE on three different clusters and on my local Mac. Appart from my strong conviction that one should be compiling and running its computer code with different compilers, new Macs (with ARM processors) simply do not have access to Intel compilers anymore, but they can use gfortran, for example.

I would be good if you included the possibility to compile CABLE with gfortran on Gadi. That would help to port the script to other systems.

I also think that it would be good to use less environment variables. The build script should help me and not make me have to remember various environment variables.
So it would be good to have a parallel compilation switch, for example, with the build.script, starting it like ./build.bash -j 8.
And the debug mode could be switched on with ./build.bash -j 8 -d, for example.

Lastly, I do not think that it is a good idea to remove the --mpi switch. Sometimes an MPI library gets loaded due to other modules, such as a Python environment that uses mpi4py. But I still want to compile the serial version.

Kind regards,
Matthias

[SeanBryan51]

@mcuntz, thank you for the feedback.

• We can definitely add a case using gfortran. I can look into adding a flag to specify the compiler (e.g. ./build.bash --compiler=gnu) when invoking build.bash.
• I agree that an environment variable for the number of parallel jobs is a bit inconvenient. We can wrap this around a flag.
• Currently build.bash supports CMake flags so we don't need to define flags which have a CMake analogue. I have included some useful flags / CMake specifics in the help of build.bash.
• I agree with your comment around MPI. CMake supports a --target option to select a target to build, this can be used to build either the MPI target or serial target. I can look into adding support for this to build.bash.

[mcuntz]

Dear @SeanBryan51 ,

I hence tried to port the new build script to my Mac mini with ARM processor, which has the short hostname mcmini. Attached is the script that worked in a basic version. I don't know how to do the fancy editing directly so I attach the file to this comment. (I had to add .txt to be able to attach it)
build.bash.txt

Here are several remarks:

1. Could you use the shebang line #!/usr/bin/env bash rather than #!/bin/bash, please. My bash is at /opt/homebrew/bin/bash, for example.
   This is the preferred way for all scripting languages, also for python, perl, etc. With these languages, one immediately understands the reason.
2. prepend_path is part of the module's perl script. No module, no prepend_path. I put a version iprepend_path into the build script, which I use in my .bash_profile, and use this with the non-module Mac.
3. I added the config for mcmini. I do not know if I need the two PKG_CONFIG_PATH but it is to show you that netcdf-c and netcdf-fortran are in very different places. One of the cluster I am using has a weird, non-homogenous architecture and there a static build is preferred. There I might need the second addition to PKG_CONFIG_PATH.
4. You can see that I had to add the Fortran compiler for CMake because it found the NAG compiler first.
5. With all this, it started compiling but failed with landuse3.f90. This is because mland is a module variable and defined as a local variable in the subroutine landuse_allocate_mland. So I renamed mland into mmland in this routine (lines 235-487) and then it compiled.
6. I have not put in compiler flags for gfortran and neither had you. Did you try the setup with the GNU compiler?
7. Have you looked into build.ksh of the branch CABLE-POP_TRENDY? We use more compiler flags also on GADI, for example, -O3 instead of -O2, or the optimisation for the CPUs. And we use much, much more flags for the debug version for it to be useful.
8. I find it a bit ugly that the script searches for MPI even if one does not want to do an MPI run. It tells me three lines about "could not find".
9. And I think I had told before that I think the logic is flawed to compile the MPI version if the MPI compiler is found. This can be to several reasons. Just as on my system where it finds the NAG compiler and tries with this.

Kind regards,
Matthias

[SeanBryan51]

@mcuntz thanks for testing out the build system!

Could you use the shebang line #!/usr/bin/env bash rather than #!/bin/bash, please.

Done.

prepend_path is part of the module's perl script. No module, no prepend_path. I put a version iprepend_path into the build script, which I use in my .bash_profile, and use this with the non-module Mac.

I would like to avoid defining helper functions in build.bash to keep it minimal. If possible, please source the relevant scripts in your mcmini section.

I added the config for mcmini. I do not know if I need the two PKG_CONFIG_PATH but it is to show you that netcdf-c and netcdf-fortran are in very different places. One of the cluster I am using has a weird, non-homogenous architecture and there a static build is preferred. There I might need the second addition to PKG_CONFIG_PATH.

I recommend trying the build without modifying PKG_CONFIG_PATH to see if it is necessary. You can also try pkg-config --path netcdf-fortran to see if the correct netcdf-fortran path is being used.

Static linking is possible but it will require modifying CMakeLists.txt. Static linking via the FindPkgConfig module also isn't that well supported (see https://gitlab.kitware.com/cmake/cmake/-/issues/21714). It does look like there are work-arounds by using the NETCDF_STATIC_* variables and target_link_options. I'm happy to help with this.

I have not put in compiler flags for gfortran and neither had you. Did you try the setup with the GNU compiler?

Compiler flags for the release and debug configurations are defined in CMakeLists.txt. Currently this has only been done for the Intel compiler:

CABLE/CMakeLists.txt

Lines 14 to 24 in 9cc7be6

	set(CABLE_INTEL_Fortran_FLAGS -fp-model precise)
	set(CABLE_INTEL_Fortran_FLAGS_DEBUG -O0 -g -traceback -fpe0)
	set(CABLE_INTEL_Fortran_FLAGS_RELEASE -O2)
	if(CMAKE_Fortran_COMPILER_ID MATCHES "Intel")
	set(
	CABLE_Fortran_FLAGS
	${CABLE_INTEL_Fortran_FLAGS}
	"$<$<CONFIG:Release>:${CABLE_INTEL_Fortran_FLAGS_RELEASE}>"
	"$<$<CONFIG:Debug>:${CABLE_INTEL_Fortran_FLAGS_DEBUG}>"
	)
	endif()

I would be happy to add the flags used in your TRENDY branch for the GNU case.

I find it a bit ugly that the script searches for MPI even if one does not want to do an MPI run. It tells me three lines about "could not find". And I think I had told before that I think the logic is flawed to compile the MPI version if the MPI compiler is found. This can be to several reasons. Just as on my system where it finds the NAG compiler and tries with this.

I see now that an explicit switch for MPI would be useful. Currently the default is to build both serial and MPI applications simultaneously. I will change the default to build only the serial application and add back --mpi to enable the MPI compilation. I previously had a CABLE_MPI flag in CMakeLists.txt, we can use this to guard the build from MPI specifics.

[SeanBryan51]

Have you looked into build.ksh of the branch CABLE-POP_TRENDY? We use more compiler flags also on GADI, for example, -O3 instead of -O2, or the optimisation for the CPUs. And we use much, much more flags for the debug version for it to be useful.

@mcuntz I see here you use different flags for the release build as well as using intel-compiler-llvm instead of the regular intel-compiler. I think it is best we discuss these changes in a separate issue as optimisation improvements are outside the scope of this pull request. This pull request is focussed on putting the foundations in place for a build system using CMake.

[ccarouge]

I have one comment on the documentation but otherwise I'm fine with the current version.

[SeanBryan51]

Hi @mcuntz, have your requested changes been addressed? This pull request requires your approval to be merged.

[mcuntz]

Hey @SeanBryan51 ,

that looks really nice. I could simply get it to work on my Mac by adding the following little code after line 98 of build.bash:

elif hostname -f | grep -E '(mc16|mcmini)' > /dev/null; then
    : "${compiler:=gnu}"

    case ${compiler} in
        gnu)
	    export PKG_CONFIG_PATH=/usr/local/netcdf-fortran-4.6.1-gfortran/lib/pkgconfig:${PKG_CONFIG_PATH}
	    export PKG_CONFIG_PATH=${HOMEBREW_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}
	    cmake_args+=(-DCMAKE_Fortran_COMPILER=gfortran)
	    ;;
	
        ?*)
            echo -e "\nError: compiler ${compiler} is not supported.\n"
            exit 1
	    ;;
    esac

That's short :-) You might want to add it as an example for other people. Or not, as you like.

You can also see that I need to pass the argument -DCMAKE_Fortran_COMPILER=gfortran to cmake otherwise it would pick up the NAG compiler. So I was wondering if this could/should be also in the Gadi section so that people knwo if they ever port the code to another machine.

One little demand: could you make the compiler names either lowercase of case-insensitive, please. I can never remember Intel/intel/ifort or GNU/gnu/gcc/gfortran.

I was just wondering if you actually compiled the code with the GNU compiler? Because I had lots of (small) errors. The one I told you before (in landuse3.F90) in release mode but plenty more in debug mode.
I guess you think that this PR is not the right place. So I might try to do a PR for the small edits. I am just not working with the main branch and I had corrected these errors years ago in our branch.

[SeanBryan51]

@mcuntz thank you for reviewing. I'm happy to add the configuration you attached as an example.

You can also see that I need to pass the argument -DCMAKE_Fortran_COMPILER=gfortran to cmake otherwise it would pick up the NAG compiler.

CMake also recognises "standard" environment variables like FC for the fortran compiler - this is probably how CMake finds the right compiler on Gadi as the FC variable is set when module loading the Intel or GNU compilers.

One little demand: could you make the compiler names either lowercase of case-insensitive, please. I can never remember Intel/intel/ifort or GNU/gnu/gcc/gfortran.

Done.

I was just wondering if you actually compiled the code with the GNU compiler? Because I had lots of (small) errors. The one I told you before (in landuse3.F90) in release mode but plenty more in debug mode.

Yep I've tested out the GNU compiler on my local machine and had to work through the bug fixes you encountered to get it to work. As you guessed I would prefer this to be done in a separate pull request as it is a bit out of scope.