ESCOMP / ESCOMP/CTSM

List of _hst variables duplicates same-name variables to circumvent a problem when writing to history

Open
#2,188 0 comments 0 reactions 0 assignees View on GitHub
code health
Dominant language
Fortran
Stars
352
Forks
361
Avg merge
2d 21h
Merged PRs (30d)
7

Description

Start with this [snicar PR conversation](https://github.com/ESCOMP/CTSM/pull/1861/files#r1286448570).

However, the problem originates before #1861. In particular, these two variables are >=6 years old:
```
real(r8), pointer :: albsnd_hst_col(:,:) ! col snow albedo, direct , for history files (col,bnd) [frc]
real(r8), pointer :: albsni_hst_col(:,:) ! col snow albedo, diffuse, for history files (col,bnd) [frc]
```
and they differ from albsnd, albsni as follows (copied directly from SurfaceAlbedoMod.F90):
```
! For diagnostics, set snow albedo to spval over non-snow non-urban points
! so that it is not averaged in history buffer (OPTIONAL)
! TODO - this is set to 0 not spval - seems wrong since it will be averaged in

do ib = 1, nband
do fc = 1,num_nourbanc
c = filter_nourbanc(fc)
if ((coszen_col(c) > 0._r8) .and. (h2osno_total(c) > 0._r8)) then
albsnd_hst(c,ib) = albsnd(c,ib)
albsni_hst(c,ib) = albsni(c,ib)
else
albsnd_hst(c,ib) = 0._r8
albsni_hst(c,ib) = 0._r8
endif
enddo
enddo
```

In #1861 @cenlinhe introduced `_hst2` for these two variables and `_hst` for a long list of other albedo variables because the existing ones, as @cenlinhe explains, "are assigned to zero when there is no sunlight, which leads to an incorrect daily and monthly mean [...]. For these new albedo variables, I set their values to NaN when there is no sunlight so that the CLM daily/monthly mean calculations automatically exclude the time points without sunlight. Of course, we could modify the existing default albedo variables to correct this but when I implemented the code, I tried to be cautious to avoid directly changing the existing default diagnostic variables."

(I think that, where @cenlinhe says NaN, he means spval.)

I'm not calling this a bug, yet, because it may have been intentional. I understand that Cenlin's choice is correct for his purposes, but are the default variables correct for clm's purposes, or is this actually a bug?

@ekluzek suggested that I include the following participants:
@cenlinhe @ckoven @wwieder @dlawrenncar and Mark Flanner (If he has a github handle).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.