CNFire code: btran2 should not be skipped when it's greater than 1

[billsacks]

Brief summary of bug

Ever since the first version of the CNFire code, there has been a conditional on btran2 so that it is not included in averages if it is greater than 1. But I am 99% sure this is a bug: btran2 can be greater than 1 due to rounding error, and it should not be excluded in this case.

General bug information

CTSM version you are using: master

Does this bug cause significantly incorrect results in the model's science? Yes

Configurations affected: All CN / BGC

Details of bug

Important details of your setup / configuration so we can reproduce the bug

I demonstrated this is an issue by introducing the following diffs of off the btran2incnfire_movecall branch (the branch in #1155):

diff --git a/src/biogeochem/CNFireBaseMod.F90 b/src/biogeochem/CNFireBaseMod.F90
index ddbc8f815..51aa9c20e 100644
--- a/src/biogeochem/CNFireBaseMod.F90
+++ b/src/biogeochem/CNFireBaseMod.F90
@@ -250,6 +250,14 @@ subroutine CNFire_calc_fire_root_wetness( this, bounds, num_exposedvegp, filter_
                     (smpso(patch%itype(p)) - smpsc(patch%itype(p))), 1._r8))
          end do
       end do
+
+      do f = 1, num_exposedvegp
+         p = filter_exposedvegp(f)
+         if (btran2(p) > 1._r8) then
+            write(iulog,'(a, i0, f23.17)') 'WJS: btran2 high: ', p, btran2(p)
+         end if
+      end do
+
     end associate 

   end subroutine CNFire_calc_fire_root_wetness

I then did a 20-day run with --compset IHistClm50BgcCrop --res f19_g17. The write statement was triggered 539460 times: an average of 562 patches per time step. In every one of these times, btran2 was exactly 1.00000000000000022.

[billsacks]

I haven't tried this with the new 2021 fire code. At a glance, I don't expect this problem to appear as often there, but I still feel it's a bug for that version to exclude btran2 values that are > 1.

[billsacks]

@dlawrenncar raised a very good point that this check for btran2 <= 1 excludes bare soil patches, and that was probably its original intent. I plan to add an explicit check for whether a patch is a bare soil patch rather than relying on this behavior.

[billsacks]

@dlawrenncar @ekluzek - I am finding that, for the new (2021) fire code, btran2 can sometimes exceed 1 by more than roundoff. From about 3 days at f10 resolution, it seems like this new definition of btran2 still does not get substantially more than 1, but I saw values as high as 1.01. (In contrast, the older definition of btran2 remained no greater than (1 + 1e-12) over multiple years.)

I still feel that the correct thing to do is to limit btran2 to be no greater than 1 – rather than the current code, which allows btran2 > 1 but then ignores all values > 1 when taking the column average. I will move ahead with this fix unless someone chimes in with different feelings.

[billsacks]

@dlawrenncar @ekluzek - I am finding that, for the new (2021) fire code, btran2 can sometimes exceed 1 by more than roundoff. From about 3 days at f10 resolution, it seems like this new definition of btran2 still does not get substantially more than 1, but I saw values as high as 1.01. (In contrast, the older definition of btran2 remained no greater than (1 + 1e-12) over multiple years.)

I still feel that the correct thing to do is to limit btran2 to be no greater than 1 – rather than the current code, which allows btran2 > 1 but then ignores all values > 1 when taking the column average. I will move ahead with this fix unless someone chimes in with different feelings.

Update on this: In a 5-year test at f10 resolution (ERS_Ly5_P144x1.f10_f10_musgs.IHistClm51BgcCropGs.cheyenne_intel.clm-cropMonthOutput), even though there were greater-than-roundoff-level differences in btran2 due to restricting it to being <= 1, this doesn't end up changing any other fire fields. From looking more carefully at the 2021 fire code, this makes sense: as long as btran2 is anywhere near 1, it seems like this version of the fire code predicts 0 fires.

I still feel that the correct thing to do is to limit btran2 to be no greater than 1, but it appears that – at least in nearly all cases – this will only change answers for Clm50 compsets.