min & max don't work correctly for some history fields

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2013-12-17 11:26:23 -0700 Bugzilla Id: 1884 Bugzilla CC: muszala@ucar.edu, rfisher@ucar.edu,

The 'min' and 'max' functions don't work correctly for some history fields. Specifically, this seems to apply to fields that take on spval at some points in time, but not others. This definitely applies to the multi-layer snow fields that I'm adding; I'm not sure if it applies to any others.

This problem seems to be in clm4.0 and clm4.5, although I have only witnessed it in CLM4.5.

The fix is as shown at the bottom of the bug report; I have tested this for my new snow fields (exercising the clm4.5 code, with 2-d fields averaged to the grid cell level). Note that the diffs below just fix the problem in one place. This problem occurs in at least 3 other places in the clm4.5 code, and a similar number of places in the clm4.0 code (and maybe more; I didn't look carefully - search for nacs in the code). The main reason I'm not just making the fix right now is that I don't have time to test this fix in all the places where it occurs. This could actually be a good excuse to consolidate some of the duplicated code in histFileMod - i.e., consolidate the 4 (or more) places where the average / min / max logic is duplicated (with small variations).

Note that I am planning to add min/max versions of the snow history fields to some of the CLM tests. If I do this, then I would expect this bug fix to change answers for these min/max snow history fields (in particular, I expect to see a FILLDIFF in cprnc).

Here are the diffs that fix the problem in one place:

Index: main/histFileMod.F90
===================================================================
--- main/histFileMod.F90    (revision 56050)
+++ main/histFileMod.F90    (working copy)
@@ -1391,10 +1391,10 @@
                 if (field_gcell(k,j) /= spval) then
                    if (nacs(k,j) == 0) hbuf(k,j) = -1.e50_r8
                    hbuf(k,j) = max( hbuf(k,j), field_gcell(k,j) )
+                   nacs(k,j) = 1
                 else
-                   hbuf(k,j) = spval
+                   if (nacs(k,j) == 0) hbuf(k,j) = spval
                 endif
-                nacs(k,j) = 1
              end do
           end do
        case ('M') ! Minimum over time
@@ -1403,10 +1403,10 @@
                 if (field_gcell(k,j) /= spval) then
                    if (nacs(k,j) == 0) hbuf(k,j) = +1.e50_r8
                    hbuf(k,j) = min( hbuf(k,j), field_gcell(k,j) )
+                   nacs(k,j) = 1
                 else
-                   hbuf(k,j) = spval
+                   if (nacs(k,j) == 0) hbuf(k,j) = spval
                 endif
-                nacs(k,j) = 1
              end do
           end do
        case default

[billsacks]

Let's discuss how to proceed here: We could

1. close this as wontfix

2. fix it according to the suggestions and put similar changes in other parts of the code without necessarily testing all places (quick, but risky)

3. fix it according to the suggestions and put similar changes in other parts of the code, with sufficient testing (safe, but longer)

4. refactor the code to eliminate duplication, fix it, and do testing (less testing required than 3) (longest implementation time, but less testing burden than (3))

[billsacks]

From discussion: let's do (2) but do just limited testing to ensure that answers don't change for min & max fields that don't go between spval and real values.