Cable updates from esm16 merge1and2 #626
Conversation
does use coupled/shared/LAI_canopy_height_cbl.F90 although it is either already updated or should be
show this up. I think it came from an ammmendment to offline years ago. the diffefrence coming in is from casa_offline*** from ESM15 a decade ago AND the CABLE:main version contains this CASE anyway.
There was a problem hiding this comment.
revert this
Whyborn
left a comment
Whyborn
left a comment
There was a problem hiding this comment.
I've left a bunch of requests, almost all are just formatting. A couple about giving variables more descriptive names
src/science/casa-cnp/casa_cnp.F90
Outdated
|
|
||
| phen%phase = 2 | ||
|
|
||
There was a problem hiding this comment.
Ive aligned the multiple lines of the WHERE condition but then returned to the LOOP indentation level. Do you really want to change this?
There was a problem hiding this comment.
Yea, lines with nothing in them should be empty of everything including whitespace I think. Mostly due to many editors doing this by default, and causing annoying diffs in later commits
There was a problem hiding this comment.
so you mean the empty lines - like this?
|
@JhanSrbinovsky which changes do you expect to make a science difference? Are these changes already part of this PR or are you yet to include them? |
|
@Whyborn they are already here, and noted so, in "files changed" - but Im thinking I'll just do more than one PR. Even if the consequential changes are held up we'll be much closer. |
|
I can't see how either would lead to changes. For the first change you noted, adding of the |
|
5 site bench_cab test fails. I'm hoping it will show no change. There does appear to be a bunch of data there, but then all of a sudden it thinks it is somewhere else and says it can't find the config.yaml Any ideas @Whyborn ?
|
|
I might pass this on to @SeanBryan51 or @abhaasgoyal as the benchcab experts, but it looks like you're missing a config file? |
|
@JhanSrbinovsky I think hh5 is defunct now - you have to update to use xp65. There was a notification a while ago on the hive about this |
I vaguely remember this - ok - I think I've sorted it now |
|
I spoke too soon. I've updated to using xp65, but same error. I dot think it is looking for my config.yaml, although it already successfully read this to successfully build the branches I specified. @SeanBryan51 or @abhaasgoyal |
Hi @JhanSrbinovsky, sorry to hear you are having issues! It would be helpful for us if you created an issue describing the commands you ran and the errors produced from benchcab |
|
Thanks Sean, Here is some additional info. The straight outta the box config.yaml doesn't help, same error. But maybe problem is that it isn't even from this repo anymore. This is from a year ago or more. gadi:bench_example> gitbr
I cloned from git@github.com:CABLE-LSM/benchcab.git and trying again. |
Whyborn
left a comment
Whyborn
left a comment
There was a problem hiding this comment.
Still have a couple of comments, on stuff I remarked upon before
| LAI_pft_temp(i,n) = MAX( 0.99*CLAI_thresh, LAI_pft(i,n) ) | ||
| HGT_pft_temp(i,n) = MAX( 0.1, HGT_pft(i,n) ) | ||
| ! Apply correction to vegetated types only | ||
| IF ( n <= aust_xeric ) THEN |
There was a problem hiding this comment.
@JhanSrbinovsky I don't think this is resolved- if you think it's not worth making this change, that's fine, but I get the impression from your comment there's meant to be a change here (changing aust_xeric to something which conveys this is the last vegetated tile)?
@Whyborn @JhanSrbinovsky Realistically this aust_xeric should be an npft - i.e. the number of vegetated PFTs.
There was a problem hiding this comment.
Where have they been moved to? I'm guessing this climate type has to do with climate-based phenology, do the benchcab runs test this (@ccarouge)?
|
Looks good to me. Did you upload the benchcab results to MEorg? Did the bug fixes lead to any noticeable difference in results? |
|
I thought that line I posted was a summary of all 20 jobs where: so because they are identical I didn't post to ME.org - however I have never had great success with doing this. Last time I tried we ere on the verge of being to upload directly from GADI. Hs that happened? @Whyborn |
|
The changes to |
|
happy news. I've looked at the several remaining differences we've created on this side that MIGHT have to go back to t he ESM1.6 and it doesn't look like there are any. Mostly they are comments, the offending #def which is taken care of in the lib. The biggest difference is substituting npft for aust_xeric. The conclusion being that we can swap in the library for the official control. In a meting this morning we discussed removing xnslope etc in a casa routine and updating the lookup table. This can be our first update to the lib perhaps. Maybe build a lib, we'll build an ESM1.6 executable and test this against an Oct-X run. Then we'll tentatively make the switch? |
CABLE
Thank you for submitting a pull request to the CABLE Project.
Description
It might be beneficial to have these all in one place. Given a few trivial changes that really should go into the ESM side, and retained here (e.g referencing cable_serial replacing cable_driver) - comparing this branch to the ESM1.6 branch there are ZERO differences.
merging branches representative of #622 #623 #624
Fixes #619 #628
Type of change
Please delete options that are not relevant.
Checklist
Testing
bench_cab config.yaml
benchmark_cable_qsub.sh.o152947997
Please add a reviewer when ready for review.
📚 Documentation preview 📚: https://cable--626.org.readthedocs.build/en/626/