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

Floating overflow in SoilBiogeochemNitrifDenitrifMod.F90 #65

Closed
billsacks opened this issue Dec 16, 2017 · 6 comments
Closed

Floating overflow in SoilBiogeochemNitrifDenitrifMod.F90 #65

billsacks opened this issue Dec 16, 2017 · 6 comments
Labels
tag: bug - critical big problems in important configurations type: bug something is working incorrectly
Milestone

Comments

@billsacks
Copy link
Member

Bill Sacks < sacks@ucar.edu > - 2017-07-06 13:16:08 -0600
Bugzilla Id: 2488
Bugzilla CC: andre@ucar.edu, cdkoven@lbl.gov, dlawren@ucar.edu, rfisher@ucar.edu,

On the compsetchanges branch (soon to be clm4_5_16_r249), the following tests are failing:

SMS_Ld5_D_P24x1.f10_f10_musgs.IHistClm50Bgc.hobart_nag.clm-decStart
ERP_D_P24x1.f10_f10_musgs.IHistClm50Bgc.hobart_nag.clm-decStart
ERP_D.f10_f10_musgs.IHistClm50Bgc.yellowstone_gnu.clm-decStart

The two hobart_nag tests point to line 443 of SoilBiogeochemNitrifDenitrifMod.F90. The last one dies in the same module; no line numbers are available, but my guess is that it's the same problem.

This death occurs in this block of code:

        if ( soil_co2_prod(c,j) > 0 ) then
           ratio_no3_co2(c,j) = smin_no3_massdens_vr(c,j) / soil_co2_prod(c,j)
        else
           ! fucntion saturates at large no3/co2 ratios, so set as some nominally large number
           ratio_no3_co2(c,j) = 100._r8
        endif

where line 443 is:

           ratio_no3_co2(c,j) = smin_no3_massdens_vr(c,j) / soil_co2_prod(c,j)

Probably the conditional needs to be changed so that if soil_co2_prod is close to 0, some alternative is used, but this needs some scientific input.

@billsacks billsacks added this to the clm5 milestone Dec 16, 2017
@billsacks billsacks added the tag: bug - critical big problems in important configurations label Dec 16, 2017
@billsacks
Copy link
Member Author

Erik Kluzek < erik@ucar.edu > - 2017-07-07 15:38:10 -0600

I looked into the case on yellowstone with gnu in the ddt debugger, and soil_co2_prod(c,j) is order 1.e-317, so extremely small, while smin_no3_massdens_vr(c,j) was order 1.e-3.

@billsacks
Copy link
Member Author

billsacks commented Dec 16, 2017

Erik Kluzek < erik@ucar.edu > - 2017-09-14 13:57:09 -0600

Here's the fix I have on a branch that I'll bring to the trunk...

Index: soilbiogeochem/SoilBiogeochemNitrifDenitrifMod.F90
===================================================================
--- soilbiogeochem/SoilBiogeochemNitrifDenitrifMod.F90	(revision 86613)
+++ soilbiogeochem/SoilBiogeochemNitrifDenitrifMod.F90	(working copy)
@@ -439,7 +439,7 @@
             ratio_k1(c,j) = max(1.7_r8, 38.4_r8 - 350._r8 * diffus(c,j))
 
             ! ratio function (figure 7c)
-            if ( soil_co2_prod(c,j) > 0 ) then
+            if ( soil_co2_prod(c,j) > 1.0e-9_r8 ) then
                ratio_no3_co2(c,j) = smin_no3_massdens_vr(c,j) / soil_co2_prod(c,j)
             else
                ! fucntion saturates at large no3/co2 ratios, so set as some nominally large number

@billsacks
Copy link
Member Author

At least as of CLM tag r272 (that's as far back as I looked: for r272 I checked @ekluzek 's case directory from his testing), this test ERP_D.f10_f10_musgs.IHistClm50Bgc.cheyenne_gnu.clm-decStart is no longer failing due to this issue, but is instead failing due to ESCOMP/MOSART#3

@billsacks billsacks added type: bug something is working incorrectly and removed type: bug - science labels Feb 8, 2018
@ekluzek
Copy link
Contributor

ekluzek commented Apr 9, 2018

This issue does have a potential to change answers. For example this test case

LVG_Ld5_D.f10_f10.I1850Clm50Bgc.cheyenne_intel.clm-no_vector_output

Showed a roundoff difference in the field: F_N2O_DENIT.

@billsacks
Copy link
Member Author

@ekluzek this was fixed by #333 and so should be closed, right?

@ekluzek
Copy link
Contributor

ekluzek commented Apr 10, 2018

Fixed in clm5.0.dev0004

@ekluzek ekluzek closed this as completed Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: bug - critical big problems in important configurations type: bug something is working incorrectly
Projects
None yet
Development

No branches or pull requests

2 participants