Rewrite ERP with SystemTestsCompareTwo

[billsacks]

Yet another attempt to rewrite the ERP test using the
SystemTestsCompareTwo infrastructure. If approved, this will supersede
#1830 and #1845.

This version is similar to the version of @jedwards4b in #1845. However,
rather than building our own mini-archiving functionality into
SystemTestsCompareTwo, I have refactored case_st_archive to extract a
function that can be called from SystemTestsCompareTwo. In this way,
there is very little test-specific code: we pretty much can just reuse
the existing logic for archiving restart files. This should reduce the
chance for errors and the maintenance cost compared with #1845.

In doing the refactoring for this, I stumbled upon and fixed two issues
with the short-term archiver: #1851 and #1853. Fixing #1853 uncovered
#1854, which is not fixed here.

Testing done: In addition to the standard scripts_regression_tests, I
have done the following testing, using
https://svn-ccsm-models.cgd.ucar.edu/clm2/trunk_tags/clm4_5_16_r253:

1. ERP_P24x2_Ld5.f10_f10.I1850Clm50Bgc.cheyenne_intel.clm-default

• made sure it passes
• made sure the two cases use the correct ntasks and nthreads
• made sure it still passes if I rerun it

2. ERP_P24x2_Ln240.f10_f10.I1850Clm50Bgc.cheyenne_intel.clm-default

• needs ./xmlchange GLC_NCPL=48,OCN_NCPL=48,ROF_NCPL=48

3. ERP_P24x2_Ld5.f10_f10.I1850Clm50Bgc.cheyenne_intel.clm-default with
   SourceMods to make the restart fail; confirmed it really does fail:

--- ../../../components/clm/src/biogeophys/WaterStateType.F902017-08-31 08:01:21.094907095 -0600
+++ SourceMods/src.clm/WaterStateType.F902017-08-31 08:54:32.910961000 -0600
@@ -862,7 +862,7 @@
     use clm_varcon       , only : denice, denh2o, pondmx, watmin, spval, nameg
     use landunit_varcon  , only : istcrop, istdlak, istsoil
     use column_varcon    , only : icol_roof, icol_sunwall, icol_shadewall
-    use clm_time_manager , only : is_first_step
+    use clm_time_manager , only : is_first_step, is_restart
     use clm_varctl       , only : bound_h2osoi
     use ncdio_pio        , only : file_desc_t, ncd_io, ncd_double
     use restUtilMod
@@ -875,7 +875,7 @@
     real(r8)         , intent(in)    :: watsat_col (bounds%begc:, 1:)  ! volumetric soil water at saturation (porosity)
     !
     ! !LOCAL VARIABLES:
-    integer  :: c,l,j,nlevs
+    integer  :: p,c,l,j,nlevs
     logical  :: readvar
     real(r8) :: maxwatsat    ! maximum porosity
     real(r8) :: excess       ! excess volumetric soil water
@@ -920,6 +920,14 @@
          long_name='canopy water', units='kg/m2', &
          interpinic_flag='interp', readvar=readvar, data=this%h2ocan_patch)

+    if (flag == 'read' .and. is_restart()) then
+       do p = bounds%begp, bounds%endp
+          if (this%h2ocan_patch(p) < 1._r8) then
+             this%h2ocan_patch(p) = this%h2ocan_patch(p) * (1._r8 + 1.e-14_r8)
+          end if
+       end do
+    end if
+
     call restartvar(ncid=ncid, flag=flag, varname='SNOCAN', xtype=ncd_double,  &
          dim1name='pft', &
          long_name='canopy snow water', units='kg/m2', &

--- ../../../components/clm/src/biogeophys/BalanceCheckMod.F902017-08-31 08:01:21.149685054 -0600
+++ SourceMods/src.clm/BalanceCheckMod.F902017-08-31 08:55:49.498168158 -0600
@@ -333,7 +333,6 @@
                   qflx_surf(indexc)+qflx_h2osfc_surf(indexc)+qflx_drain(indexc))

              write(iulog,*)'clm model is stopping'
-             call endrun(decomp_index=indexc, clmlevel=namec, msg=errmsg(sourcefile, __LINE__))

           else if (abs(errh2o(indexc)) > 1.e-5_r8 .and. (nstep > 2) ) then

@@ -361,7 +360,6 @@
              write(iulog,*)'qflx_snwcp_discarded_liq = ',qflx_snwcp_discarded_liq(indexc)*dtime
              write(iulog,*)'qflx_rootsoi_col(1:nlevsoil)  = ',qflx_rootsoi_col(indexc,:)*dtime
              write(iulog,*)'clm model is stopping'
-             call endrun(decomp_index=indexc, clmlevel=namec, msg=errmsg(sourcefile, __LINE__))
           end if
        end if

4. In this case, confirmed that the directory listings of both the
   archive directory and the run directory (post-archiving) match master:

./create_newcase --case check_clm_archiving_0830a --compset I1850Clm45Bgc --res f10_f10_musgs --user-mods-dir /glade/scratch/sacks/cesm_code/clm4_5_16_r253_newCime2/components/clm/cime_config/testdefs/testmods_dirs/clm/default

./xmlchange HIST_OPTION=ndays,HIST_N=1,REST_OPTION=ndays,REST_N=1

5. Same as (4) but with:

./xmlchange DOUT_S_SAVE_INTERIM_RESTART_FILES=TRUE

In (5), differences from master are the presence of rpointer.unset files
in the interim restart directories. This is due to the pre-existing
issue #1854 (which was masked in master due to #1853)

Test suite: scripts_regression_tests.py on cheyenne
Also tests listed above
Test baseline: n/a
Test namelist changes: none
Test status: bit for bit

Fixes #1851
Fixes #1853
Addresses #1647 (rewrites ERP, not ERS)

User interface changes?: none

Update gh-pages html (Y/N)?: N

Code review:

[billsacks]

@jedwards4b @jgfouca @mvertens @bertinia - I have added you all as reviewers, though I don't think all 4 of you need to review it. However, I'd love at least one (and preferably two) careful sets of eyes on my changes to case_st_archive, because I know there are some subtleties there that I might have missed.

[billsacks]

Changes in this file are unrelated fixes taken from @jedwards4b 's #1845

[billsacks]

Changes in this file match @jedwards4b 's #1845

[billsacks]

Changes in this file basically match @jedwards4b 's #1845 , except for this line

[billsacks]

This allows us to link rather than copy the restarts in erp and similar tests. This saves some time and disk space, but I'm not tied to this if others feel it adds too much complexity.

[billsacks]

This major refactor could use a careful review. Note that I have reversed the loop order, so that the date loop is now the outer loop. This was needed so that a test could easily call the restart archiving routine for a single date. Doing this refactor also incidentally fixed #1851 and #1853

[billsacks]

Most of the refactor supports sharing code between the real short-term archiving process and the erp test (and potentially future tests). This function is the one piece of new code that is NOT part of the standard short-term archiving process.

[billsacks]

I'm done making line comments; this is ready for review.

[jedwards4b]

After confering with @billsacks I will pushed my changes to this PR.

[jedwards4b]

The archive object already exists in a case and you don't need to instantiate a new one
archive = case.get_env("archive")

[jedwards4b]

I think that this code is backward, instead of looping over all of the entries in env_archive and looking for a match in compname, one should just call self.get_optional_node('comp_archive_spec', attributes={"compname":compname}) for each compname.

[jgfouca]

Minor nit: "if copy_only is True" is a bit redundant, just do "if copy_only"

[jgfouca]

I would replace this function implementation with:

return shutil.copyfile if copy_only else shutil.move

[jgfouca]

Looks good, just a couple nits.

[jedwards4b]

@jgfouca nit action taken

[bertinia]

@billsacks as we discussed, it would be useful to run an ERR multi-instance, fully coupled
test to make sure all the changes to the STA get tested across all components.

[jedwards4b]

@bertinia we have added to cime_config several fully active IRT test for this issue, including a multi-instance case. It will currently fail because the pop model does not properly generate some of the multiinstance files - this is being worked on by pop developers.

[billsacks]

For the record, I'm happy with @jedwards4b 's changes here - thank you for those!

@bertinia Based on the feedback from @jedwards4b I'll kick off ERR and IRT tests with single-instance, B compset; and ERR_N3 and IRT_N3 with F compsets. I'll let you know once those have passed.

[billsacks]

@bertinia Based on the feedback from @jedwards4b I'll kick off ERR and IRT tests with single-instance, B compset; and ERR_N3 and IRT_N3 with F compsets. I'll let you know once those have passed.

Actually, I'd prefer not to go down the rabbit hole of the ERR_N3 and IRT_N3 right now, since I don't see any such passing tests in our test suite.

[jedwards4b]

IRT_N3_Ln9.f19_f19_mg17.F2000_DEV.cheyenne_intel.cam-outfrq9s PASS
ERR_N3_Ln9.f19_f19_mg17.FHIST_DEV.cheyenne_intel.cam-outfrq9s PASS

[billsacks]

Thanks for running those tests, @jedwards4b

These two also pass, from cesm2_0_alpha07c with latest cime:

ERR.f19_g17.B1850.cheyenne_intel.allactive-defaultio
IRT.f19_g17.B1850.cheyenne_intel.allactive-defaultio