Consolidate Modifications to CASA-CNP

[penguian]

keyword_CASA-CNP keyword_carbon_cycle_maygit owner:jxs599@nci.org.au resolution_wontfix type_model improvement | by srb001@csiro.au

---

Overarching Ticket to consolidate changes to CASA-CNP from YIng-Ping's branch that were not included in recent trunk updates. These changes will need to be specifically documented as consolidation progresses.

Repo link

TRAC link

An approach was adopted to isolate the "real" differences to the trunk for consideration. Differences in white-space, case, formatting, and even alternative code changes have (in general) been ignored.

There were however cases, expecially those involving equations over multiple lines, where some of these differences, and formatting which I altered as well, crept into the merged product.

The steps taken(with links) were:

1. Subdirectories were made in biogeochem and populated with existing files.

2. These files, particularly the large modules were separated into subroutines, and renamed as individual files.
   here

Like files containing the same subroutine were manually merged from the other side.

It is intended that these files and subroutines will be reintegrated into a modular structure, however I anticipate these modules will be smaller, more of them, and I think the directory structure should be considered as well. The biogeophys/ and UM directories are similarly modified in the UM10.6 application and also in anticipation of merging with the UKMO repository.

The merged differences are marked by Instances of "Ticket#146" or "Ticket146" or "if(Ticket146)" or "if (.NOT. Ticket146)". In my earlier work, as well as making formatting changes, I also included references to every point where YP's branch was different from the trunk. However, this got quite messy and I think it is simpler to proceed as we usually do and indicate only the changes proposed to the trunk.

The biggest difference is probably that to the IO. Although there is overlap here, the subroutines have different names, they might be called from different places, live in different modules and/or USE different modules.

Per directory, following the merge, the proposed trunk contains these differences:

cable/

cnp/

inout/

vars/

---

Issue migrated from trac:146 at 2023-11-27 11:20:37 +1100

[penguian]

@jxs599@nci.org.au changed type from defect to model improvement

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@jxs599@nci.org.au edited the issue description

[penguian]

@bep599@nci.org.au commented

---

A lot of the differences are formatting changes and old codes that were commented out. These should have been removed before presenting here for a decision. It is also unhelpful when the names of individual files are different from the subroutine names. There are also instances where part of the trunk code has been merged into Yingping's version while wanting advice if they should change. The worse is when real differences are not reflected on this comparison page, which made me go through more pages in more detail to make sure.

1) casa_cable.F90

a) bgcdriver:

As I said in the CABLE committee nearly a year ago, the last big update was not properly done. If you put them in like building blocks (CASA-CNP first, then POP/LUC after), you will not end up with this mess especially in the IO.

e.g. CALL biogeochem(ktau,dels,idoy,LALLOC,veg,soil,casabiome,casapool,casaflux, & casamet,casabal,phen,POP,climate, xnplimit, ...) - this happens a few times in various places.

Yingping's version is the basic block, LALLOC, POP and climate are the add-ons. After you put in the add-ons, you now look back at the basic block and ask which statement to choose. Surely here, the trunk version is your choice. However, instead of the condition if(Ticket146) then why is there no switch like if(using_POP) then in this place? Isn't that the rule? Recommendation: use trunk version because you have already added more things and add the POP switch if achievable.

For the initializations (casaflux%cgpp, etc.), they were originally in trunk a684ae0eb78318e043f9e67b58e1656655e62a73. They were removed without a ticket and a reason. Now, do you want to put it back in? Recommendation: use YP's version which is just reinstating the original situation, no need of a switch.

Saving data in the new variables casamet%spin is not new; the comment line says it was there since Nov 2012 (although only in YP and BP's branches). The trunk version of CASA-CNP was purported to be from YP's branch in mid 2016 and so it was thrown into the trunk without fuss. Who has removed that and why? No ticket. Recommendation: Of course, it should be added back without a switch.

b) spincasacnp:

This comparison for this subroutine is very misleading as a lot of differences are absent while others have already been merged to show just formatting differences. Actually, the two versions are not comparable. The code there in the trunk is one generation older. YP has since pulled out most of those (the call to analyticpool and its related variables avg_*) into a new subroutine spinanalytic (SVN dated at 30/06/2016). Recommendation: The merge of this subroutine is better done again from scratch – putting in YP’s changes before doing the POP add-on. Finally, please explain why subroutine spincasacnp was ripped out of casa_cable.F90 when read_casa_dump and write_casa_dump are still there.

Following are a few more points worth noting.

“TYPE (casa_met)  :: casaspin” is not used in the trunk and not in YP’s version. Where does it come from? Which ticket? Recommendation: Remove from trunk.

The trunk version actually has the comment '! more variables to store the spinup pool size over the last 10 loops.' but the variables are still in dimension(5,...). Of course, YP's version has dimension(10,...). But who is going to decide on that number, the committee? Recommendation: No big deal; committee decide; but need comment and code to be consistent.

By the way, the variables casamet%spin mentioned in 1a are being used here in the trunk, proving that the trunk version of CASA-CNP derived from YP’s branch, but was mishandled.

Why are the “Call totcnppools” commented out?

c) ncdf_dump vs write_casa_dump:

YP’s version is named ncdf_dump, which has now become write_casa_dump in the trunk. The new name is more fitting, and a facelift to the code as well. Most of the ‘extra’ USE statements in YP’s version are there in the trunk, just at different position; cable_io_vars_module is not needed after Lars found another suitable variable to use; and cable_ncdf_module is superseded by the cable_diag_module. Just need to find where those casamet%Tairk, etc. variables are filled. And also need to test if it works without declaring the mplant dimension. Recommendation: Use the trunk version, but need testing.

d) analyticpool:

Same comment and recommendation as in 2f for casa_coeffsoil.

Two new lines in the trunk for casabal%clitterlast and casabal%csoillast. They are not there in Yingping's version (at least for the past two years) because they have been dealt with in casa_inout for initialization and then updated in casa_cnp. They are not necessary here. Recommendation: Remove these two lines from the trunk version. If the POP code needs them, put a POP switch around them.

The comment “! why is this commented here but used in UM” and the two commented lines after can be removed from the trunk and from the UM codes. ACCESS does not run this subroutine.

e) read_casa_dump:

Recommendation: Use the trunk version, but need testing.

f) totcnppools:

Choose 5 or 10 for output dimension? Recommendation: Use the same number as chosen in 1b.

The comparison here are mismatched – LHS blocks are one or two blocks below the RHS ones. As mentioned in 1b, YP’s version is newer. The initialization of those variables has to go out of this subroutine and get into spincasacnp because this subroutine would be called twice and you don’t want the variables to be re-initialized at the second call. Within the DO-loops, everything is exactly the same except for a new condition to make sure bmarea is non-zero. Recommendation: Use YP’s version, careful not mixing up this with the merged version.

g) casa_feedback:

The value of npleafx is now read in from the parameter file and stored in the variable casabiome%ratioNPplantmin(veg%iveg(:),leaf) so that it will not be 14.2 for all PFTs and the scaling factor xnslope is not needed anymore. Recommendation: Use YP’s version and merge the ‘Walker2014’ part only (which may need its own npleafx initialization).

2) casa_cnp.F90

a) casa_xkN:

Putting xkNlimiting within the 0 to 1 range. That is like a bug fix or code enhancement. Recommendation: Use YP's version without switch.

b) casa_rplant:

New variable delcrmleaf and its initialization. I was curious why the new variable has not been used. Going through the original codes on trunk and YP’s branch, I found that there are 21 lines of code in YP’s version that is not in the comparison page. They are the codes for fixing negative NPP. Again, that is like a bug fix or code enhancement. Recommendation: Use YP's version without additional switch – don’t forget the extra 21 lines. I said ‘additional’ switch because there is already a CALL_climate switch.

By the way, there are 80 lines of code for T-acclimation from ticket #110 which looks unwieldy. The purpose of these seems to be calculating 2 variables resp_coeff_root and resp_coeff_sapwood for each PFT. The equations look the same in the 4 repetitions, except for one parameter. Recommendation: It can easily be simplified with parName(veg%iveg(npt)) in place of the parameter value in the equations.

c) casa_cnpcycle:

Pool size modifications after casamet%glai has been trimmed within range by the MIN/MAX statement are there to correct the mass balance. It is a bug fix. The MIN statement in the condition LALLOC.ne.3 does not exist in YP's version; please don't mix up the codes. Recommendation: Use YP's version without switch; get rid of the part with LALLOC.ne.3 in the trunk and add a LALLOC.eq.3 switch (or POP) for the POP situation so that it can be switched off.

The bit of Ticket108 to put lower bound to Nsoilmin was merged into Yingping’s version in the comparison so that I nearly missed it. Recommendation: Use the trunk version, but YP should have the last word. There are 6 instances of writing out diagnostics in this subroutine using WRITE(57,*) accompanied by calling casa_poolzero and then resetting the pool size. The output variables differ arbitrarily and YP's version put an extra condition on to remove writing out from barren patches. Firstly, the resetting of pool sizes (using max(0,pool)) should be commented out so that we get the right mass balance even though the pool size go below zero (hopefully constrained somewhere to prevent it happening). Secondly, casa_poolzero is just the format of the output warning; together with the write statement, they should only be active during code development. In production stage, some people may choose to have the WARNING statements. The barren condition is a moot point. Recommendation: Comment out the resetting of pool sizes in all 6 occasions in the trunk version; the committee can decide if a switch (not ticket146, but something like negPool that goes into the namelist file) should be used for writing out the diagnostics here.

d) casa_delplant:

The conditional statement "IF (casamet%lnonwood(npt)==0) THEN" and its ELSE component does not exist in YP's version. Again, please do not mix up the versions for comparison. If you have already decided and implemented the merge, then there is no point in asking for advice; if not, leave them be. Above this, there is a big chunk of additional codes from ticket #108 to avoid negative pools. This conditional statement is not included in that ticket. However, this implementation in the trunk is an insurance in case the variable has not been initialized elsewhere. Recommendation: Use trunk version.

The calculation of casaflux%FluxNtolitter(npt,str) in YP's version has already been replaced by the trunk version but formatted differently. Took me a while to know that my advice is not needed. By the way, where are the simulation results for ticket108 so that I can have a look? Yingping should check this one carefully if he agree to it or if his implementation has already dealt with this problem.

The comment "! now accounted for in delsoil" is actually good to have there so that others can understand why that line of code is not needed there anymore. Recommendation: Use YP's version.

e) avgsoil:

Just can't understand why there need to be a condition "if( .NOT. Ticket146)" there. When you implement ticket #121, it was adding a MAX function in part of the equation for casamet%btran. How come the old calculation is not commented out? Recommendation: Use the trunk version, but comment out the old code for casamet%btran.

f) casa_coeffsoil:

The parameter values are now given names cuexxx which means Carbon-use-efficiency of the pool xxx. The trunk version using different values for the same pool was just for testing purpose, with the final version now in YP's branch (not the merged version shown in the comparison). Recommendation: Scrap the merged version which is incorrectly using cuestr in the cwd pool. Use YP's original version without switch. Please note that there are 10 more lines of commented out codes above it (in YP’s version). These are possible equations and ranges for the parameters. They could be useful for future developers to play with.

g) casa_allocation:

Near miss again. The initialization of fracClabile was commented out in the trunk - which ticket? However, as the use of this variable in phen%phase=3 section has been commented out, and it will again be initialized in casa_xnp later on, this move is actually correct. Recommendation: Use trunk version.

If you love switches, shouldn’t the new CASE(3) section (from ticket #61) be on a POP switch?

There is a big chunk of codes in green to determine fracCalloc. This is not new, it is actually the chunk in the trunk within the condition “IF (LALLOC.ne.(3)) THEN”. As mentioned above in 2c, this is an incorrect switch. This part should be run normally. The next part in the ELSE portion should have a POP switch. There are a couple of problems in this chunk in the trunk. i) The trunk code at ca46a6831f471a2f09f9f2ab47ff536e207ae418 shows that after dealing with prognostic LAI reaching glaimax, there was a section for reaching glaimin. This was removed from normal situation but continue to exist in the ELSE section for the POP case. ii) YP has modified the condition here to act some time before reaching glaimax and glaimin. iii) The penultimate bit to set the ratio differ in the use of Crmplant or the lacking of it; this is a non-issue as both will reach the same result. Recommendation: Use a POP switch (make it obvious and not hide in the additional LALLOC case) throughout the code for the POP additions; Use YP’s code for this section while the POP code is within a switch after that.

h) casa_Nrequire:

Again, the trunk has been replacing the YP’s version in the comparison, but the contradictory comment seems to suggest using YP’s new variable xscale to simplify the equation. Recommendation: Use YP’s version without switch.

3) casa_inout.F90

Too many changes and will resort to just running the code to see if the input/output work as expected. New ticket #167 shows the existing trunk code has not been thoroughly tested before implementation.

[penguian]

@jxs599@nci.org.au changed status from new to closed

[penguian]

@jxs599@nci.org.au set resolution to wontfix

[penguian]

@jxs599@nci.org.au changed milestone from 3. Implementation to 1. Closed

[penguian]

@jxs599@nci.org.au commented

---

YP has adopted trunk@HEAD version that may include these now anyway

[penguian]

@ccc561@nci.org.au changed keywords from CASA-CNP, carbon cycle to CASA-CNP, carbon cycle maygit