Fix to groundwater parameter initialisations that can impact the soil conductivity

[ccarouge]

CABLE

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

Description

Fixes #490

Working on the groundwater implementation, I have found that the function total_soil_conductivity depends on some GW parameters that aren't correctly implemented. This is to fix this.

Type of change

Please delete options that are not relevant.

• Bug fix

Testing

Since this requires MPI to test the impact, I haven't used modelevaluation.org to analyse the results. I have run benchcab with the spatial configuration for:

1. main with cable_user%soil_thermal_fix = .TRUE
2. main with cable_user%soil_thermal_fix = .FALSE
3. this branch with cable_user%soil_thermal_fix = .TRUE
4. this branch with cable_user%soil_thermal_fix = .FALSE

Case 3 - Case 1 allow to gauge the total effect of the change. Case 2 - Case 1 give a measure of the change compared to the initial fix to the soil conductivity. Case 4 - Case 2 shows whether there are any impacts beyond the soil conductivity (without explicitly finding these other sources).

The comparison of case 4 and case 2 with cdo diffn shows there are no differences when cable_user%soil_thermal_fix is false.

The largest effect for this branch is found on the ground flux but the effect is limited (right plot) compared to the effect of soil_thermal_fix (left plot):

These are time-mean values of the differences between the simulations.

Please add a reviewer when ready for review.

---

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

[har917]

Approved: code looks okay, comments are appropriate.

Testing is appropriate to the issue at hand - only aspect not assessed is the magnitude of the impact when vectorised soil parameters are used (but out of scope for now since the base case and inputs aren't available.).

NB - we may need to assess/check that these variables are initialised properly within the coupled model framework since we are defaulting to use cable_users%soil_thermal_fix = .true.

[har917]

@ccarouge @JhanSrbinovsky Quick follow up on this PR as it relates to AM3 etc.

It would appear that most of the _vec variables and soil%watr are initialised from their single layer counterparts properly in AM3 - the exceptions being clay_vec, sand_vec, silt_vec and org_vec as well as the GW*_vec (CASA vars and GW vars). However the value of soil%watr is a bit questionable.

An element to pick up on later (between alpha and beta releases of AM3 and/or part of the merge of the GW science) is the meaning/role and consistency in value of %watr - offline seems to be setting this (if GW_MODEL = .false.) to 0.05 and/or 0.01 and/or 0.0, whereas CM2 and AM3 have it at 0.001. Basically I think there's some confusion as to whether %watr is a GW-only variable or more widely-used.

[JhanSrbinovsky]

I was of the impression that the *_vec variables are not being used? It looks like soil%watr is used IF soil_thermal_fixis TRUE. This is likely even why I switched it to false back when 4 timesteps was our longest run – from stempv You should look at cable_um_init_soil. It is initialized there as .05 and later clobbered as .001 – intentional? My comment where it is clobbered reflects what is actually needed?

[har917]

I was of the impression that the *_vec variables are not being used?

I hadn't fully realised this before - but Mark Dekker coded the soil_thermal_fix option for the soil thermal conductivity to use the *_vec variables (and %watr) all the time - regardless of whether GW_MODEL = .true./.false.. Since we want (need) to use that science all the time this means that those variables need to carried and filled (in all applications) - or we need to rewrite the code to accommodate both cases.

I don't know what %watr is meant to represent - nor know how/why its default value has changed through the development process. I just picked up its quirky initialisation when reviewing the ACCESS code in response to this PR request.

[ccarouge]

The whole implementation of the *_vec variables is quirky and should not have been done this way. There is no reason to carry 2 sets of soil parameter variables in the code (except laziness around updating existing code to add the extra dim). And I agree it would have been nice to know what watr is!