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

Missing sa_leaf code from PPE branch in CanopyFluxesMod.F90 #2777

Closed
olyson opened this issue Sep 20, 2024 · 4 comments · Fixed by #2788
Closed

Missing sa_leaf code from PPE branch in CanopyFluxesMod.F90 #2777

olyson opened this issue Sep 20, 2024 · 4 comments · Fixed by #2788
Assignees
Labels
bug something is working incorrectly non-bfb Changes answers (incl. adding tests) science Enhancement to or bug impacting science

Comments

@olyson
Copy link
Contributor

olyson commented Sep 20, 2024

Brief summary of bug

The PPE branch (latest tag being PPE.n16_ctsm5.1.dev030) had a change to sa_leaf in CanopyFluxesMod.F90 when biomass heat storage is on. This didn't make it into the latest CTSM tag.

General bug information

CTSM version you are using: ctsm5.2.028

Does this bug cause significantly incorrect results in the model's science? Maybe. The code mod was put in place to improve survivability, but our survivability in the current model in general is pretty good so it may not have that much effect. Although maybe it will improve our C4 grass which was at 58% survivability in a recent simulation.

Configurations affected: All with biomass heat storage on.

Details of bug

The relevant code block that was in the last PPE tag is:

            if(.not.(is_tree(patch%itype(p)) .or. is_shrub(patch%itype(p))) &
                 .or. dbh(p) < min_stem_diameter) then
               frac_rad_abs_by_stem(p) = 0.0
               sa_stem(p) = 0.0
!KO
               sa_leaf(p) = sa_leaf(p) + esai(p)
!KO
            endif

The line bracketed by !KO is not in the current code.
My notes from January 2021 detail a conversation with @dlawrenncar and @swensosc in which we were trying to improve survivability with biomass heat storage on. After various tests we came up with this modification: "For grasses and skinny trees/shrubs, add in esai to sa_leaf". I believe it was added for the situation where elai could be zero and thus sa_leaf would be zero (sa_leaf is set equal to 2*elai in code above this).

@olyson olyson added the bug something is working incorrectly label Sep 20, 2024
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Sep 20, 2024
@ekluzek ekluzek added this to the CESM3 Answer changing freeze milestone Sep 20, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Sep 20, 2024

@olyson thanks for pointing this out. I'm glad you did so we can bring it in. We thought we had everything, but good to get this last bit in as well.

It looks like this will have a small change to answers is that correct? The code change is simple, but we'll have to bring it in as part of an answer changing tag if so.

@olyson
Copy link
Contributor Author

olyson commented Sep 20, 2024

We should probably test this separately before bringing it in, but yes it should be part of an answer changing tag. There is one other PPE-related code mod in CNVegStructMod.F90 that we'll need to test as well. It may not have any effect, but I need to test it next week. My opinion is that these shouldn't stop the CTSM5.3 tag, they could be part of the bug fix tags that come later, if @wwieder agrees.

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 20, 2024

Yes, this will NOT be part of ctsm5.3.0 -- but hopefully one of the quick follow on tags. Thanks for the update here.

Yes, if there are other changes you could test with it as well that sounds ideal. If you want to start testing now you could use the ctsm5.3.0 branch tag that Sam created for you yesterday.

@ekluzek ekluzek added science Enhancement to or bug impacting science non-bfb Changes answers (incl. adding tests) labels Sep 20, 2024
@olyson
Copy link
Contributor Author

olyson commented Sep 24, 2024

I tested the sa_leaf change in 5 year I2000 cases:

/glade/work/oleson/ctsm5.3.n04_ctsm5.2.028/cime/scripts/ctsm53n04ctsm52028_f09_saleaf_2000
/glade/work/oleson/ctsm5.3.n04_ctsm5.2.028/cime/scripts/ctsm53n04ctsm52028_f09_2000

Diagnostics are here:

https://webext.cgd.ucar.edu/I2000/ctsm53n04ctsm52028_f09_saleaf/lnd/ctsm53n04ctsm52028_f09_saleaf_2000_2001_2004-ctsm53n04ctsm52028_f09_2000_2001_2004/setsIndex.html

The differences in JJA TLAI and GPP are pretty scattered and relatively small. Mostly decreases in Asia, eastern Siberia, and western U.S., with increases in Alaska, northern Canada, and northern Europe.
We originally vetted this change with respect to survivability in 1850 spinups and eventually historical simulations, but I'm not we necessarily need to repeat that.

@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Sep 26, 2024
samsrabin added a commit to samsrabin/CTSM that referenced this issue Oct 15, 2024
PPE change to sa_leaf in CanopyFluxesMod.F90

Add change to sa_leaf that was in PPE branch but did not get on master.

Larger than roundoff changes in answers are explained in this post
ESCOMP#2777 (comment)
and the diagnostics are here
https://webext.cgd.ucar.edu/I2000/ctsm53n04ctsm52028_f09_saleaf/lnd/ctsm53n04ctsm52028_f09_saleaf_2000_2001_2004-ctsm53n04ctsm52028_f09_2000_2001_2004/setsIndex.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly non-bfb Changes answers (incl. adding tests) science Enhancement to or bug impacting science
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants