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

Fix the FUN bug, where an if statement was backwards... #727

Merged
merged 6 commits into from
Jun 5, 2019

Conversation

ekluzek
Copy link
Collaborator

@ekluzek ekluzek commented May 22, 2019

Apply the difference from the olyson/delta_CN_FUNbug off of release-clm5.0.15 to ctsm/master.

Description of changes

Technical notes: Page 167

The bug in the FUN model source code is that the functions corresponding
to these two conditions relevant to deltaCN are placed in a wrong order. So
under the deltaCN > 0 condition, it actually uses the function for deltaCN <
0.

1238 if(delta_CN .gt.0.and. frac_ideal_C_use.lt.1.0)then
1239 frac_ideal_C_use = frac_ideal_C_use + (1.0_r8-frac_ideal_C_use)*min(1.0_r8, delta_CN/fun_cn_flex_c(ivt(p)));
1240 end if
1241 ! If we have too much N (e.g. from free N retranslocation) then make frac_ideal_c_use even lower.
1242 ! For a CN delta of fun_cn_flex_c, then we reduce C expendiure to the minimum of 0.5.
1243 ! This seems a little intense?
1244 if(delta_CN.lt.0.0)then
1245 frac_ideal_C_use = frac_ideal_C_use + 0.5_r8*(1.0_r8*delta_CN/fun_cn_flex_c(ivt(p)))
1246 endif

Clm5.0/src/biogeochem/CNFUNMod.F90

Original code

1238 if(delta_CN.lt.0.0)then
1239 frac_ideal_C_use = frac_ideal_C_use + (1.0_r8-frac_ideal_C_use)*min(1.0_r8, delta_CN/fun_cn_flex_c(ivt(p)));
1240 end if
1241 ! If we have too much N (e.g. from free N retranslocation) then make frac_ideal_c_use even lower.
1242 ! For a CN delta of fun_cn_flex_c, then we reduce C expendiure to the minimum of 0.5.
1243 ! This seems a little intense?
1244 if(delta_CN .gt.0.and. frac_ideal_C_use.lt.1.0)then
1245 frac_ideal_C_use = frac_ideal_C_use

Specific notes

Contributors other than yourself, if any: @olyson, @rosiealice, Yanyan Cheng, Maoiyi Huang

CTSM Issues Fixed (include github issue #): #704
Fixes #704 -- FUN bug

Are answers expected to change (and if so in what way)? yes (when FUN is on, so Clm50Bgc)

Any User Interface Changes (namelist or namelist defaults changes)? None

Testing performed, if any: regular testing being done

We've completed 40 year global 1850 runs (after AD and post-AD spinup) for a control and with the bug fix (on release-clm5.0.15). Diagnostics are here:

http://webext.cgd.ucar.edu/I1850/clm50_release-clm5.0.15_delta_CN_FUNbug_2deg_GSWP3V1_1850/lnd/clm50_release-clm5.0.15_delta_CN_FUNbug_2deg_GSWP3V1_1850.21_40-clm50_release-clm5.0.15_2deg_GSWP3V1_1850.21_40/setsIndex.html

We've also completed a historical run. Diagnostics are here:

http://webext.cgd.ucar.edu/I20TR/clm50_release-clm5.0.15_delta_CN_FUNbug_2deg_GSWP3V1_hist/lnd/clm50_release-clm5.0.15_delta_CN_FUNbug_2deg_GSWP3V1_hist.1995_2014-clm50_release-clm5.0.15_2deg_GSWP3V1_hist.1995_2014/setsIndex.html

Our assessment is that the impact is relatively small, but would affect climate; however it doesn’t appear to strongly affect transient C response.

@ekluzek ekluzek added type: bug - impacts science priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete labels May 22, 2019
@ekluzek ekluzek self-assigned this May 22, 2019
@olyson
Copy link
Contributor

olyson commented May 22, 2019

Looks good to me.

@billsacks billsacks added tag: bug - impacts science bug something is working incorrectly and removed type: bug - impacts science labels May 24, 2019
@ekluzek
Copy link
Collaborator Author

ekluzek commented Jun 4, 2019

OK, tests have passed on cheyenne as expected. A test that previously failed inexplicably started working (for #550), so I'm rerunning that one. I have some failures on hobart to sort through.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Jun 4, 2019

Rerunning the test -- it now fails properly as expected.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Jun 5, 2019

OK, I fixed the testing on hobart. Now everything is as expected.

@ekluzek ekluzek merged commit 8e5c61b into ESCOMP:master Jun 5, 2019
@ekluzek ekluzek deleted the CN_FUNbug branch June 5, 2019 16:22
@samsrabin samsrabin added the science Enhancement to or bug impacting science label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations science Enhancement to or bug impacting science
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FUN code logic to reduce or increase carbon allocation used for uptake is reversed
4 participants