Bug in `datm_datamode_clmncep_mod` preventing using diffuse & direct SW streams

[adrifoster]

A bug is currently in datm_datamode_clmncep_mod: datm_datamode_clmncep_advance on line 329. The array assignment for Faxa_swdn(:) = strm_Faxa_swdn(:) occurs whether strm_Faxa_swdn has been associated or not (and it's not if you try to use direct & diffuse rather than total)

    ! Direct copies of stream fields
    Sa_tbot(:) = strm_Sa_tbot(:)
    Faxa_swdn(:) = strm_Faxa_swdn(:)
    if (associated(strm_Sa_topo)) then
       Sa_topo(:) = strm_Sa_topo(:)
    else
       Sa_topo(:) = SHR_CONST_SPVAL
    end if

a fix could be to add an associates check (we already check that SOMETHING related to _swdn must be associated earlier)

    if (associated(strm_Faxa_swdn)) then
      Faxa_swdn(:) = strm_Faxa_swdn(:)
    end if

but I'm not sure if this is what we want to do. @mvertens @billsacks @ekluzek @swensosc

[kjetilaas]

Thanks for bringing this up, @adrifoster! I ran into the same problem yesterday. I ended up just providing a dummy input for Faxa_swdn, and did a test to confirm it didn't get used, but good to get this corrected properly.

[billsacks]

Thanks for opening this issue, @adrifoster . I agree that this looks wrong.

I think the safest thing to do would be to set Faxa_swdn in both branches of the conditional under the !--- shortwave radiation --- comment:

• Under if (associated(strm_Faxa_swdndf) .and. associated(strm_Faxa_swdndr)) then, set Faxa_swdn(n) to strm_Faxa_swdndr(n) + strm_Faxa_swdndf(n)
• Under elseif (associated(strm_Faxa_swdn)) then, set Faxa_swdn(n) to strm_Faxa_swdn(n).

How does this sound to others?

It looks like Faxa_swdn isn't used for atm-lnd coupling in CESM, so another alternative could be to just remove this field, but it looks like Faxa_swdn is used by UFS, according to this:

https://github.com/ESCOMP/CMEPS/blob/d4886180781b818d9d998e54ba995194a3430694/mediator/esmFldsExchange_ufs_mod.F90#L834-L852

so it seems safest to keep it... and then seems most robust to make sure it's always set.

Thoughts?

[ekluzek]

@billsacks yes I think Faxa_swdn should always be set. That will allow it to be used in case it's needed in the future. You have to provide either swdn in either the form of total or in the form of divided into diffuse/direct so you might as well always set it.

However, I think that line on 329 should just be removed and it added later near line 441 where there already is a check for what swdn options are associated.

So I suggest a diff like this:

index 181eac3..0e12a27 100644
--- a/datm/datm_datamode_clmncep_mod.F90
+++ b/datm/datm_datamode_clmncep_mod.F90
@@ -326,7 +326,6 @@ subroutine datm_datamode_clmncep_advance(mainproc, logunit, rc)
 
     ! Direct copies of stream fields
     Sa_tbot(:) = strm_Sa_tbot(:)
-    Faxa_swdn(:) = strm_Faxa_swdn(:)
     if (associated(strm_Sa_topo)) then
        Sa_topo(:) = strm_Sa_topo(:)
     else
@@ -443,7 +442,9 @@ subroutine datm_datamode_clmncep_advance(mainproc, logunit, rc)
           Faxa_swvdr(n) = strm_Faxa_swdndr(n) * 0.50_r8
           Faxa_swndf(n) = strm_Faxa_swdndf(n) * 0.50_r8
           Faxa_swvdf(n) = strm_Faxa_swdndf(n) * 0.50_r8
+          Faxa_swdn(n) = strm_Faxa_swdndr(n) + strm_Faxa_swdndf(n)
        elseif (associated(strm_Faxa_swdn)) then
+          Faxa_swdn(n) = strm_Faxa_swdn(n)
           ! relationship between incoming NIR or VIS radiation and ratio of
           ! direct to diffuse radiation calculated based on one year's worth of
           ! hourly CAM output from CAM version cam3_5_55

If that sounds acceptable I might as well create a PR with that change.

Another part that we should have is a test that makes sure you can provide SWDN as either total or divided into diffuse/direct. One way to do that would be to bring in TRENDY-2026 forcing as an option?

[ekluzek]

One, of the reasons that this bug comes up, is that we have a lot of flexibility in the list of variables that can be provided in DATM. But, there isn't necessarily testing that each of the different options works.

For example

If topo isn't provided you get a constant
Same for height
You provide PBOT or get a constant
You get humidity, by either providing specific, relative, or dewpoint temperature
You provide LWDN or get it calculated
You get solar by either providing total or direct/diffuse
Then bias and anomaly forcing has similar flexibility as well

It seems like having some tests that make sure you can provide humidity each of the three ways, and soilar the two ways would be good to have. It also seems like missing PBOT is going to be a bad mistake in most cases -- so maybe should be thought about more?

To test the options there could be three test mods that use an existing forcing option to make sure they all work.

We've been relying on random chance that we cover the different options through the different types of forcing we use -- but that doesn't guarantee coverage.

[billsacks]

@ekluzek - yes, your suggested change in #425 (comment) is exactly what I was trying to suggest, but less clearly - thanks for giving the diff with that change to specify it more clearly. (Though, for symmetry, I would probably put the Faxa_swdn(n) = strm_Faxa_swdndr(n) + strm_Faxa_swdndf(n) line at the top of that conditional block rather than the bottom.)