Calculations of local noon assume that longitude is 0 to 360 rather than -180 to 180

[ekluzek]

Brief summary of bug

SurfaceRadiationMod.F90, PhotosynthesisMod.F90 and UrbanFluxesMod.F90 all calculate the time for local noon, the way it's coded assumes that the grid has longitude from 0 to 360 and NOT -180 to 180.

General bug information

CTSM version you are using: ctsm1.0.dev009

but goes back to ctsm1.0.dev004

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

Because our standard grids are all on 0 to 360 and NOT -180 to 180.

Configurations affected: Any with grids on -180 to 180

Details of bug

Here's a snippet...

PhotosynthesisMod.F90:               local_secp1 = secs + nint((grc%londeg(g)/degpsec)/dtime)*dtime
PhotosynthesisMod.F90:               local_secp1 = mod(local_secp1,isecspday)
PhotosynthesisMod.F90:               if (local_secp1 >= (isecspday/2 - 3600) .and. local_secp1 <= (isecspday/2 + 3600)) then

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

Also see the cime issue: ESMCI/cime#2778

Here's a note from @swensosc about files that can be used...

"It requires a domain file (and surface data file) with negative longitudes. I've created some here:
/glade/scratch/swensosc/sfcdata/surfdata_0.9x1.25_78pfts_CMIP6_simyr1850_c170824.negative.longitude.nc
/glade/scratch/swensosc/sfcdata/domain.lnd.fv0.9x1.25_gx1v6.090309.negative.longitude.nc
An out of the box f09_g16 clm I case run for 1 month would show the affect."

Important output or errors that show the problem

It doesn't flag an error, it just silently does the wrong thing.

@olyson @barlage

[ekluzek]

I have a fix for this, that I'm testing. There are still fields that have large changes though. There's also a local time calculation needed for irrigation, that is concerning. So I turned off irrigation for the following test, and still see changes.

 RMS lon                              2.5367E+02   ! This of course is expected!!!
 RMS DISPVEGC                         1.2908E+01
 RMS DISPVEGN                         3.6411E-01
 RMS EFLX_LH_TOT                      4.0616E-02
 RMS EFLX_LH_TOT_R                    4.9569E-02
 RMS FGEV                             4.0616E-02
 RMS FGR                              4.7322E-01
 RMS FGR12                            3.7026E-02
 RMS FIRA                             1.4372E-01
 RMS FIRA_R                           1.7735E-01
 RMS FIRE                             1.4372E-01
 RMS FIRE_R                           1.7735E-01
 RMS FROOTN                           1.5357E-01
 RMS FSH                              2.9781E-01
 RMS FSH_G                            2.9781E-01
 RMS FSH_R                            3.6740E-01
 RMS FSH_TO_COUPLER                   2.9781E-01
 RMS H2OSNO                           2.0291E-01
 RMS HIA                              1.4429E-02
 RMS HIA_R                            1.7698E-02
 RMS HUMIDEX                          1.6274E-02
 RMS HUMIDEX_R                        1.9956E-02
 RMS ICE_CONTENT1                     2.0289E-01
 RMS LEAFN                            2.1131E-01
 RMS SMP                              4.0061E+01
 RMS SNOTXMASS                        5.0921E+01
 RMS SNOWICE                          2.0305E-01
 RMS STORVEGC                         1.9029E+01
 RMS STORVEGN                         5.5376E-01
 RMS SWBGT_R                          1.1632E-02
 RMS TG                               3.5421E-02
 RMS TOTCOLC                          2.0728E+01
 RMS TOTCOLN                          6.0558E-01
 RMS TOTECOSYSC                       2.0728E+01
 RMS TOTECOSYSN                       6.0558E-01
 RMS TOTPFTC                          2.0728E+01
 RMS TOTPFTN                          6.0558E-01
 RMS TOTVEGC                          2.0728E+01
 RMS TOTVEGN                          6.0558E-01
 RMS TSA                              1.4429E-02
 RMS TSKIN                            4.8694E-02
 RMS TWS                              2.0292E-01

[ekluzek]

Discussed this with @swensosc he is going to look into this more, and get back to me. I'm thinking at this point there's likely multiple issues. The behavior he say of the NLDAS grid clearly showed that cime/datm was mishandling Solar. So there might still be issues in cime/datm as well as others in ctsm. My fix will be going in #449.

[ekluzek]

OK, looking into this more. Note that COSZEN is only roundoff different between the two cases. COSZEN should be what's driving the distinction between night and day. I'm not seeing the connection that makes this screwed up. But, it does affect a large set of variables and hence makes simulations unusable, for a grid that includes the America's. So I think I should open a new issue that talks about the science issue, as the original issue here was just the local noon calculations, and for that I've got a fix to come in.

[ekluzek]

Irrigation changes answers with changing to a function call and with the change from modulo to mod, for certain values. So I'll do a step that's bit for bit and an answer changing one.

[billsacks]

@ekluzek I spent some time this evening looking into your new local time calculation (assuming the SSREsnowmip branch on ESCOMP is up-to-date). The problem is that the get_local_time function is incorrect: The division by dtime in this line breaks the function:

    get_local_time  = secs + nint((lon/degpsec)/real(dtime,r8))*dtime

Consider, for example, secs = 60, longitude = 1 degree. Then lon/degpsec = 240, and we should be adding 240 seconds to get the local time. But the division by dtime before taking nint results in: nint(240/1800) = 0, so there ends up being no longitude-based correction to get local time. I'm not sure who originally introduced this division by dtime, but it seems that it shouldn't be there.

I see that you introduced a unit test to try to compare the 'irrig' version of local time with the 'standard' version, for a wide range of longitudes and times. Unfortunately, you made a small error in the logic that made this a no-op: You start with secs = 0, which makes is_end_curr_year true, which means that this loop never executes:

       do while ( .not. is_end_curr_year() )
          @assertEqual( get_local_irrig_time( londeg ), get_local_time( londeg ) )
          call advance_timestep()
       end do

If you change this to start at secs = 1800 rather than 0, you'll see that that unit test flags the two functions as differing significantly.

When I changed get_local_time as follows:

--- a/src/utils/clm_time_manager.F90
+++ b/src/utils/clm_time_manager.F90
@@ -1509,7 +1509,7 @@ integer function get_local_time( londeg, offset )
     call  get_curr_date(yr, mon, day, secs, offset )
     lon = londeg
     if ( lon < 0.0_r8 ) lon = lon + 360.0_r8
-    get_local_time  = secs + nint((lon/degpsec)/real(dtime,r8))*dtime
+    get_local_time  = secs + nint(lon/degpsec)
     get_local_time  = mod(get_local_time,isecspday)
   end function get_local_time

the fixed unit test now says the two functions are equivalent. Actually, I only ran with longitude every 1 deg, not every 0.1 deg, because even that took 9 secs on my Mac, and I didn't have the patience to wait 90 sec :-).