Skip to content

Commit

Permalink
Merge tag 'ctsm1.0.dev057' into soil_texture_interp_bug
Browse files Browse the repository at this point in the history 
Fix frac_sno bugs

Two bug fixes related to frac_sno, and a third change involving
replacing a frac_sno calculation with a simpler equation that is
algebraically equivalent:

(1) Fix lake frac_sno always being 0
    (ESCOMP#783). This fixes the albedo
    calculation (and possibly others) over snow-covered lake surfaces.

(2) Fix threshold for explicit snow pack initiation to use frac_sno_eff,
    not frac_sno (ESCOMP#785). For
    standard runs (which have use_subgrid_fluxes = .true.), this just
    changes answers for urban columns. This also changes answers more
    widely for runs with use_subgrid_fluxes = .false.

(3) Rewrite Swenson & Lawrence 2012 frac_sno equation to be more
    straightforward and less sensitive to roundoff errors
    (ESCOMP#784)

Addressed conflicts in /doc
  • Loading branch information
Samuel Levis committed Aug 21, 2019
2 parents 2ee4dd7 + efca2bd commit be350df
Show file tree
Hide file tree
Showing 10 changed files with 1,237 additions and 726 deletions.
2 changes: 1 addition & 1 deletion cime_config/testdefs/testlist_clm.xml
View file
Expand Up @@ -1678,7 +1678,7 @@
just a compiler issue, we're allowing this level of
memory leak. See
https://github.com/ESCOMP/ctsm/issues/763 for details. -->
<option name="memleak_tolerance">0.2</option>
<option name="memleak_tolerance">0.3</option>
</options>
</machine>

Expand Down
357 changes: 343 additions & 14 deletions doc/ChangeLog
View file
@@ -1,7 +1,7 @@
===============================================================
Tag name: ctsm1.0.dev057
Tag name: ctsm1.0.dev058
Originator(s): slevis (Samuel Levis,SLevis Consulting LLC,303-665-1310)
Date: Mon Aug 19 17:24:02 MDT 2019
Date: Wed Aug 21 14:45:49 MDT 2019
One-line Summary: Soil texture interpolation bug-fix

Purpose of changes
Expand Down Expand Up @@ -60,18 +60,6 @@ Code reviewed by: @billsacks

CTSM testing:

[... Remove before making master tag. Available test levels:

a) regular (must be run before handing off a tag to SEs and must be run
before committing a tag)
b) build_namelist (if namelists and/or build_system changed))
c) tools (only if tools are modified and no CTSM source is modified)
d) short (for use during development and in rare cases where only a small
change with known behavior is added ... eg. a minor bug fix)
e) doc (no source testing required)

... ]

[PASS means all tests PASS and OK means tests PASS other than expected fails.]

build-namelist tests:
Expand Down Expand Up @@ -126,6 +114,347 @@ Detailed list of changes
Pull Requests that document the changes (include PR ids):
(https://github.com/ESCOMP/ctsm/pull/788)

===============================================================
===============================================================
Tag name: ctsm1.0.dev057
Originator(s): sacks (Bill Sacks)
Date: Tue Aug 20 13:17:39 MDT 2019
One-line Summary: Fix frac_sno bugs

Purpose of changes
------------------

Two bug fixes related to frac_sno, and a third change involving
replacing a frac_sno calculation with a simpler equation that is
algebraically equivalent:

(1) Fix lake frac_sno always being 0
(https://github.com/ESCOMP/ctsm/issues/783). This fixes the albedo
calculation (and possibly others) over snow-covered lake surfaces.

(2) Fix threshold for explicit snow pack initiation to use frac_sno_eff,
not frac_sno (https://github.com/ESCOMP/ctsm/issues/785). For
standard runs (which have use_subgrid_fluxes = .true.), this just
changes answers for urban columns. This also changes answers more
widely for runs with use_subgrid_fluxes = .false.

(3) Rewrite Swenson & Lawrence 2012 frac_sno equation to be more
straightforward and less sensitive to roundoff errors
(https://github.com/ESCOMP/ctsm/issues/784)

Bugs fixed or introduced
------------------------

Issues fixed (include CTSM Issue #):
- Resolves ESCOMP/ctsm#783 (frac_sno is always 0 for lake points)
- Resolves ESCOMP/ctsm#785 (Threshold for explicit snow pack initiation
should use frac_sno_eff, not frac_sno)
- Resolves ESCOMP/ctsm#784 (Suggested algebraic rework of frac_sno
calculation)

Significant changes to scientifically-supported configurations
--------------------------------------------------------------

Does this tag change answers significantly for any of the following physics configurations?
(Details of any changes will be given in the "Answer changes" section below.)

[Put an [X] in the box for any configuration with significant answer changes.]

[X] clm5_0

[X] ctsm5_0-nwp

[X] clm4_5

Notes of particular relevance for users
---------------------------------------

Caveats for users (e.g., need to interpolate initial conditions): none

Changes to CTSM's user interface (e.g., new/renamed XML or namelist variables): none

Changes made to namelist defaults (e.g., changed parameter values): none

Changes to the datasets (e.g., parameter, surface or initial files): none

Substantial timing or memory changes: none

Notes of particular relevance for developers: (including Code reviews and testing)
---------------------------------------------
NOTE: Be sure to review the steps in README.CHECKLIST.master_tags as well as the coding style in the Developers Guide

Caveats for developers (e.g., code that is duplicated that requires double maintenance): none

Changes to tests or testing: none

Code reviewed by: self


CTSM testing:

[PASS means all tests PASS and OK means tests PASS other than expected fails.]

build-namelist tests:

cheyenne - not run

tools-tests (test/tools):

cheyenne - not run

PTCLM testing (tools/shared/PTCLM/test):

cheyenne - not run

python testing (see instructions in python/README.md; document testing done):

(any machine) - not run

regular tests (aux_clm):

cheyenne ---- ok
hobart ------ ok

ok means tests pass, answers change as expected

Ran most testing on 95dad328 (before 8f1263a7, which removes a
temporary endrun error check). Just reran
SMS_Ld5_D_P48x1.f10_f10_musgs.IHistClm50Bgc.hobart_nag.clm-decStart
on the final version.

There was a memleak for this test:

FAIL SMS_D_Ld10.f10_f10_musgs.I2000Clm50BgcCropGs.hobart_nag.clm-tracer_consistency MEMLEAK memleak detected, memory went from 1250.830000 to 1518.500000 in 8 days

I have had memleak issues for this test before
(https://github.com/ESCOMP/ctsm/issues/763), which I have chalked up
to a compiler-specific issue. I have simply increased the tolerance
for memleaks for this test moving forward.

If the tag used for baseline comparisons was NOT the previous tag, note that here:


Answer changes
--------------

Changes answers relative to baseline: YES

If a tag changes answers relative to baseline comparison the
following should be filled in (otherwise remove this section):

Summarize any changes to answers, i.e.,
- what code configurations: all
- what platforms/compilers: all
- nature of change (roundoff; larger than roundoff/same climate; new climate):
Larger than roundoff; expected to be same climate in general,
because the only impacts are on urban and lake points, which
represent a small fraction of most grid cells. However, this needs
further investigation.

See above (under "Purpose of changes") for a description of the
different answer changes in this tag.

If bitwise differences were observed, how did you show they were no worse
than roundoff?

I verified that change (3) is no greater than roundoff by
temporarily putting in an endrun if the new frac_sno differs from
the old by more than 1e-13; this was not triggered for any test in
the aux_clm test suite.

If this tag changes climate describe the run(s) done to evaluate the new
climate (put details of the simulations in the experiment database)
- casename: N/A (Keith Oleson will run this)

URL for LMWG diagnostics output used to validate new climate: N/A (Keith Oleson will run this)


Detailed list of changes
------------------------

List any externals directories updated (cime, rtm, mosart, cism, fates, etc.): none

Pull Requests that document the changes (include PR ids): none

===============================================================
===============================================================
Tag name: ctsm1.0.dev056
Originator(s): sacks (Bill Sacks)
Date: Fri Aug 16 11:44:43 MDT 2019
One-line Summary: Start adding water tracers to LakeHydrology, and related refactoring

Purpose of changes
------------------

Start adding water tracers to LakeHydrology, beginning with some initial
things done for snow.

This also includes some significant refactoring to allow LakeHydrology
to reuse some of the same snow code used for non-lake columns.

Bugs fixed or introduced
------------------------

Issues fixed (include CTSM Issue #):
- Partially addresses ESCOMP/ctsm#775 (Implement water tracers for
initial snow-related code in LakeHydrology)

Known bugs found since the previous tag (include github issue ID):
The following will be fixed in an upcoming tag:
- ESCOMP/ctsm#783 (frac_sno is always 0 for lake points)
- ESCOMP/ctsm#784 (Suggested algebraic rework of frac_sno calculation)
- ESCOMP/ctsm#785 (Threshold for explicit snow pack initiation should
use frac_sno_eff, not frac_sno)

Significant changes to scientifically-supported configurations
--------------------------------------------------------------

Does this tag change answers significantly for any of the following physics configurations?
(Details of any changes will be given in the "Answer changes" section below.)

[Put an [X] in the box for any configuration with significant answer changes.]

[ ] clm5_0

[ ] ctsm5_0-nwp

[ ] clm4_5

Notes of particular relevance for users
---------------------------------------

Caveats for users (e.g., need to interpolate initial conditions): none

Changes to CTSM's user interface (e.g., new/renamed XML or namelist variables): none

Changes made to namelist defaults (e.g., changed parameter values): none

Changes to the datasets (e.g., parameter, surface or initial files): none

Substantial timing or memory changes: none

Notes of particular relevance for developers: (including Code reviews and testing)
---------------------------------------------
NOTE: Be sure to review the steps in README.CHECKLIST.master_tags as well as the coding style in the Developers Guide

Caveats for developers (e.g., code that is duplicated that requires double maintenance): none

Changes to tests or testing: none

Code reviewed by: self


CTSM testing:

[PASS means all tests PASS and OK means tests PASS other than expected fails.]

build-namelist tests:

cheyenne - not run

tools-tests (test/tools):

cheyenne - not run

PTCLM testing (tools/shared/PTCLM/test):

cheyenne - not run

python testing (see instructions in python/README.md; document testing done):

(any machine) - not run

regular tests (aux_clm):

cheyenne ---- ok
hobart ------ ok

ok means tests pass, answers change as expected

Note: Did most testing on 054dc95b (before the small change in
7e4e52a9); just ran
LWISO_Ld10.f10_f10_musgs.I2000Clm50BgcCropGs.cheyenne_gnu.clm-coldStart
on the final commit

If the tag used for baseline comparisons was NOT the previous tag, note that here:


Answer changes
--------------

Changes answers relative to baseline: YES

If a tag changes answers relative to baseline comparison the
following should be filled in (otherwise remove this section):

Summarize any changes to answers, i.e.,
- what code configurations: all
- what platforms/compilers: all
- nature of change (roundoff; larger than roundoff/same climate; new climate): roundoff

There are two answer-changes here:

(1) Roundoff-level changes due to changing some order of operations
for the updating of snow_depth and dz for lakes: The previous
code effectively used (qflx_snow_grnd(c)/bifall(c))*dtime,
whereas the new code uses (qflx_snow_grnd(c)*dtime)/bifall(c);
similarly, the dz update differs.

(2) A change in FSNO_EFF for lakes: previously, this was set to 0
when there was no snow. Now this is set consistently with other
landunits, for which it is 1 even if there is no snow. Note that
this only affects the FSNO_EFF diagnostic field, and nothing
else.

If bitwise differences were observed, how did you show they were no worse
than roundoff?

I confirmed that the order-of-operations change is the only
answer-changing part of this tag by comparing against a one-off
from master with the following diffs; this comparison was
bit-for-bit except for changes in the FSNO_EFF diagnostic field:

diff --git a/src/biogeophys/LakeHydrologyMod.F90 b/src/biogeophys/LakeHydrologyMod.F90
index 91ddfc79..9711dad7 100644
--- a/src/biogeophys/LakeHydrologyMod.F90
+++ b/src/biogeophys/LakeHydrologyMod.F90
@@ -126,6 +126,8 @@ subroutine LakeHydrology(bounds, &
real(r8) :: heatsum(bounds%begc:bounds%endc) ! used in case above [J/m^2]
real(r8) :: snowmass ! liquid+ice snow mass in a layer [kg/m2]
real(r8) :: snowcap_scl_fct ! temporary factor used to correct for snow capping
+ real(r8) :: temp_snow_depth
+ real(r8) :: newsnow
real(r8), parameter :: snow_bd = 250._r8 ! assumed snow bulk density (for lakes w/out resolved snow layers) [kg/m^3]
! Should only be used for frost below.
!-----------------------------------------------------------------------
@@ -246,8 +248,10 @@ subroutine LakeHydrology(bounds, &
! U.S.Department of Agriculture Forest Service, Project F,
! Progress Rep. 1, Alta Avalanche Study Center:Snow Layer Densification.

- dz_snowf = qflx_snow_grnd(c)/bifall(c)
- snow_depth(c) = snow_depth(c) + dz_snowf*dtime
+ temp_snow_depth = snow_depth(c)
+ newsnow = qflx_snow_grnd(c)*dtime
+ snow_depth(c) = snow_depth(c) + newsnow/bifall(c)
+ dz_snowf = (snow_depth(c) - temp_snow_depth)/dtime
if (snl(c) == 0) then
h2osno_no_layers(c) = h2osno_no_layers(c) + qflx_snow_grnd(c)*dtime ! snow water equivalent (mm)
else

If this tag changes climate describe the run(s) done to evaluate the new
climate (put details of the simulations in the experiment database)
- casename: N/A

URL for LMWG diagnostics output used to validate new climate: N/A


Detailed list of changes
------------------------

List any externals directories updated (cime, rtm, mosart, cism, fates, etc.): none

Pull Requests that document the changes (include PR ids): none

===============================================================
===============================================================
Tag name: ctsm1.0.dev055
Expand Down
4 changes: 3 additions & 1 deletion doc/ChangeSum
View file
@@ -1,6 +1,8 @@
Tag Who Date Summary
============================================================================================================================
ctsm1.0.dev057 slevis 08/19/2019 Soil texture interpolation bug-fix
ctsm1.0.dev058 slevis 08/21/2019 Soil texture interpolation bug-fix
ctsm1.0.dev057 sacks 08/20/2019 Fix frac_sno bugs
ctsm1.0.dev056 sacks 08/16/2019 Start adding water tracers to LakeHydrology, and related refactoring
ctsm1.0.dev055 sacks 08/06/2019 Modularize snow cover fraction method
ctsm1.0.dev054 sacks 08/02/2019 Fix interpolation of surfdat soil layers so we can use interpolation for 10SL case
ctsm1.0.dev053 slevis 08/01/2019 Soil layer definition clean-up and user-defined option
Expand Down

0 comments on commit be350df

Please sign in to comment.