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

Updating 219 from main. #426

Merged
merged 120 commits into from
Oct 18, 2024
Merged

Conversation

bschroeter
Copy link

@bschroeter bschroeter commented Oct 18, 2024

Updating branch 219 from main.


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

ccarouge and others added 30 commits November 28, 2023 15:22
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.

Note: since compiler flags are the same across all targets, this change
applies the flags globally to all targets in the current directory and
below via [`add_compile_options`][add_compile_options].

[add_compile_options]: https://cmake.org/cmake/help/latest/command/add_compile_options.html
Flags for release and debug configurations for the GNU compiler were
taken from build.ksh (CABLE-POP_TRENDY branch).
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 -->
Making CABLE available as a [spack](https://spack.io/) package allows us
to test the model compiles successfully inside a continuous integration
(CI) environment which gets triggered automatically on a pull request.
This change adds the model build CI workflow developed by the ACCESS-NRI
release team to compile CABLE offline via spack (see
[build-ci](https://github.com/ACCESS-NRI/build-ci) for more details).
This workflow compiles CABLE using the intel compiler. The CI for other
compilers will follow after this pull request.

Fixes #202

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

<!-- readthedocs-preview cable end -->
# CABLE ground water hydrology work of Mengyuan Mu

This pull request is part of integrating the ground water hydrology work
of Mengyuan Mu into CABLE, which will take place in a sequence of pull
requests. The actual integration project itself was completed back in
September 2023. The current work involves just a gradual merge into the
main branch.

## Description

1. A new SUBROUTINE GWspatialParameters is introduced in
**src/offline/cable_parameters.F90**
2. This subroutine will eventually be called only once, from
**src/offline/cable_input.F90**. I will add this call at the appropriate
time, but for now the subroutine is inactive
3. Dependencies are introduced in **src/offline/cable_parameters.F90**
and **src/offline/cable_define_types.F90** in order for the code to
compile successfully (confirmed)
4. This pull request involves only code additions, no deletions or
modifications of existing code were carried out (some minor typos I came
across in existing comments were fixed)
5. *PLEASE* do not delete any of my in-code comments (tagged rk4417) for
now. Once the integration of the ground water hydrology work is complete
and its functionality is confirmed to reproduce the results obtained
already outside of the repo, I will clean-up all of my comments in a
single pull request. Deleting any of my comments now will just make it
much more difficult for me to track down and fix any issues that might
arise during this process.


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

<!-- readthedocs-preview cable end -->
# CABLE

## Description

gfortran could not compile the CABLE main branch, neither in release
mode (-O3 -Wno-aggressive-loop-optimizations) nor in debug mode (-O -g
-pedantic-errors -Wall -W -Wno-maybe-uninitialized -fbacktrace
-ffpe-trap=zero,overflow,underflow -finit-real=nan).

The name `mland` was an ambiguous reference in the routine
`landuse_allocate_mland` in landuse3.F90 because it is used via
landuse_constant and defined in `landuse_allocate_mland`. I renamed it
locally to `imland` in the routine.

With this change, the release mode compiled. However there were plenty
of 'Nonconforming tab character' and 'continued character constant' in
the other files. I removed all that and now it compiles in release and
debug mode.

## Type of change

Please delete options that are not relevant.

- [x] Bug fix
- [ ] New or updated documentation

## Checklist

- [x] The new content is accessible and located in the appropriate
section.
- [x] I have checked that links are valid and point to the intended
content.
- [x] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
The build script currently does not support MPI builds with the GNU
compiler on Gadi. This is due to the `intel-mpi` module being loaded
when we should be using an MPI module that is compatible with the chosen
compiler. This change loads the `openmpi` module when GNU compilation is
enabled in the build script.

Fixes #239

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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

Fixes #(285)
Change Uber Quick guide instructions to run the testcase from
src/offline/ since for now CABLE needs the namelists in the run
directory.

## Type of change

Please delete options that are not relevant.

- [X] New or updated documentation

## Checklist

- [x] The new content is accessible and located in the appropriate
section.
- [x] I have checked that links are valid and point to the intended
content.
- [x] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
Case of mobile is not a major consideration here, comfort on a
computer is more important
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

Increase the overall width of the page. This removes the large left and
right margins and tend to improve the rendering of the tables throughout
the documentation.

You can link issues by using a supported keyword in the pull request's
description or in a commit message:

Fixes #(303)

## Type of change

Please delete options that are not relevant.

- [X] New or updated documentation

## Checklist

- [X] The new content is accessible and located in the appropriate
section.
- [X] I have checked that links are valid and point to the intended
content.
- [X] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
ccarouge and others added 29 commits August 8, 2024 11:57
# CABLE

## Description

Added a table of physical constants and a table of mathematical
constants to the user guide. Additional tables of constants can be added
following the same pattern.

Fixed missing/incorrect units in code and documentation.

**It might be a good time at this stage to decide on the numerical value
to use for the ice density: 910 from cable_soilparm.nml OR 921 from
cable_phys_constants_mod.F90 ?** Interestingly another well known code
uses 917.


- [X] New or updated documentation



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

<!-- readthedocs-preview cable end -->
Remove double summation of btran (#279)
Require both changes.
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

Fixes #351. 

Move the definition of sum_rad_gradis and sum_rad_rniso out of the NITER
loop in `define_canopy()` since these variables do not change in the
loop. Having a quick comparison between the outputs before and after
this commit [the effect is
negligeable](#352 (comment)).

Use `sum_rad_gradis` and `sum_rad_rniso` as calculated in
define_canopy() everywhere in define_canopy(), dryLeaf() and wetLeaf(),
instead of recalculating them sometimes.

Move the initialisation of `canopy%DvLitt` and `canopy%kthLitt` to the
top of `define_canopy()` since these variables are constant throughout
the run. We also want these variables to be defined in all cases to
avoid errors in function calls, but only used with `cable_user%litter`
turned on.

## Type of change

Please delete options that are not relevant.

- [X] Bug fix

## Testing

benchcab/me.org analysis:
https://modelevaluation.org/analyses/anywhere/BPLvifw3DyARokYdg/s6k22L3WajmiS9uGv/CJGXP5GQWhGf3nH28/all

Tested in benchcab the changes for DvLitt and kthLitt on their own.
benchcab showed bitwise identical results when moving the initialisation
compared to the previous implementation with the litter option on.

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
# CABLE

Add a subroutine to CABLE to ensure the consistency of ice points
between soil and vegetation, related to #338.
This branch will be used for the benchcab testing.

Fixes #280
Fixes #338

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

<!-- readthedocs-preview cable end -->
# CABLE

## Description

Following analysis in CABLE4 and discussion in #354 we should use total
SW down in the call to `spitter` and apply the resultant `Fbeam` equally
to the visible and near-infrared bands. The original intent (cable1.4)
of spitter is to evaluate the fraction of SW down that is diffuse in
nature - via the beam fraction `Fbeam`. This was evaluated once based on
the total shortwave down. From CABLE2 onwards spitter was called twice -
once for the visible and once for the near-infrared components. However
the function itself was not adjusted - the result is that beam fraction
is universally smaller than the original intent with unknown impacts on
the model trajectory.

This change set reverts the evaluation of `Fbeam` to operate on the
total SW down and then applies `Fbeam` equally to the two radiation
bands.

This has been implemented as a bug fix, not on a switch. This change may
require recalibration of the CABLE model (notably stomatal conductance
parameters, leaf nitrogen exponential and any other parameters that
impact GPP).

Fixes #355

## Type of change
- [x] Bug fix
- [ ] New or updated documentation

## Checklist

- [x] The new content is accessible and located in the appropriate
section.
- [ ] I have checked that links are valid and point to the intended
content.
- [ ] I have checked my code/text and corrected any misspellings


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

<!-- readthedocs-preview cable end -->
# CABLE ground water hydrology work of Mengyuan Mu

This pull request is part of integrating the ground water hydrology work
of Mengyuan Mu into CABLE, which will take place in a sequence of pull
requests. The actual integration project itself was completed back in
September 2023. The current work involves just a gradual merge into the
main branch.

Fixes #230

## Description

1. cable_gw_hydro.F90 is replaced by its new version (drop-in
replacement)
2. Dependencies are introduced in **src/offline/cable_parameters.F90**,
**src/offline/cable_define_types.F90**,
**src/science/soilsnow/cbl_thermal.F90**, and
**src/util/cable_common.F90** in order for the code to compile
successfully (confirmed)
3. *PLEASE* do not delete any of my in-code comments (tagged rk4417) for
now. Once the integration of the ground water hydrology work is complete
and its functionality is confirmed to reproduce the results obtained
already outside of the repo, I will clean-up all of my comments in a
single pull request. Deleting any of my comments now will just make it
much more difficult for me to track down and fix any issues that might
arise during this process.


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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

The creation of the call graphs (and others) by FORD in the API
documentation was broken since moving to readthedocs. This should solve
the issue

Fixes #9



## Type of change

Please delete options that are not relevant.

- [X] Bug fix

## Testing

We do not build the graphs in the pull request preview of the
documentation since it takes >3 min to build the doc with the graphs.
This was tested in this fork: https://github.com/ccarouge/CABLE_test

Result example in the documentation:
https://cable-test.readthedocs.io/en/latest/api/proc/bgcdriver.html

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

casaflux%frac_sapwood needs to be initialised to 1 for the correct
calculation of casaflux%crmplant(:,wood) in casa_rplant when
cable_user%CALL_climate is not true.
This had been done in casa_inout at #274 but this subroutine is only
used for offline simulations.
This change does the initialisation, in casa_init_pk, for ESM1.5/ESM1.6
cases.

Fixes #373

## Type of change

Please delete options that are not relevant.

- [X ] Bug fix


## Checklist

- [ ] The new content is accessible and located in the appropriate
section.
- [ ] I have checked that links are valid and point to the intended
content.
- [ ] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

As described in #275, this change reinstates code that had been removed
in 2022 with little justification. Reinstating the code provides
consistency with ACCESS-ESM1.5 and also with relevant code in
src_pop/core/biogeochem. Testing for a small sample of sites showed a
large impact if this change was made alone. The related issue (#279)
incorrectly calculates btran by summing over layers twice. This
calculation is only used if the removed code in #275 is reinstated. Site
tests with both changes showed only small differences.
Change only impacts runs with CASA-CNP active.
Also dealt with corrections to comment for subroutine avgsoil as noted
in #279 and subroutine casa_coeffsoil (#277).
Fixes #275, #277, #279.

## Type of change

Please delete options that are not relevant.

- [X ] Bug fix
- [ ] New or updated documentation

## Checklist

- [ ] The new content is accessible and located in the appropriate
section.
- [ ] I have checked that links are valid and point to the intended
content.
- [ ] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
AM3 application specific see issue #370

Fixes #370 

## Type of change

Hopefully will cover CM3 as well

- [? ] New or updated documentation

## Checklist

- [? ] The new content is accessible and located in the appropriate
section.
- [?] I have checked that links are valid and point to the intended
content.
- [ ] I have checked my code/text and corrected any misspellings



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

<!-- readthedocs-preview cable end -->
The build-ci action has now been fixed (see
ACCESS-NRI/build-ci#179).

Fixes #408

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

<!-- readthedocs-preview cable end -->
…odule, as gfortran provides a similar function that takes no arguments.
#375)

# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

Removed two ifdef ESM15 cases related to the use of xkpsorb. The ESM15
code option is inconsistent with
how xkpsorb is input in the CABLE3 code.

Fixes #283

## Type of change

- [X ] Bug fix


## Checklist

- [ ] The new content is accessible and located in the appropriate
section.
- [ ] I have checked that links are valid and point to the intended
content.
- [ ] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.



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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

This PR fixes compilation with gfortran. I also fixes several compiler
warnings (mostly about unused variables) in the MPI master/driver
modules.

## Type of change

Please delete options that are not relevant.

- [X] Bug fix

## Checklist

- [x] The new content is accessible and located in the appropriate
section.
- [x] I have checked that links are valid and point to the intended
content.
- [x] I have checked my code/text and corrected any misspellings


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

<!-- readthedocs-preview cable end -->
# CABLE

Thank you for submitting a pull request to the CABLE Project.

## Description

In casa_delsoil the evaluation of casaflux%Pleach had two versions, one
specific to ESM1.5 (selected via an ifdef) and the alternat for all
other cases. Following the code through, the ESM1.5 version had a
hard-wired value (1.e-4) for casaflux%fPleach. casaflux%fPleach is
filled from xfPleach which is read in from the pftlookup file. The
ESM1.5 specific code can be safely removed with no change to the output
if xfPleach is set to 1.e-4 in the pftlookup file. Note that there are
versions of the pftlookup with xfPleach = 5.e-4. I am not aware of
when/who set it to this value and whether it is considered to be
better/more realistic.

Fixes #278

## Type of change

Please delete options that are not relevant.

- [X] Code clean-up

## Checklist

- [X ] The new content is accessible and located in the appropriate
section.
- [ ] I have checked that links are valid and point to the intended
content.
- [X ] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


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

<!-- readthedocs-preview cable end -->
@bschroeter bschroeter merged commit 0911f96 into 219-implement-init_wetfac-as-per-access3-jules Oct 18, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants