Merge branch 'initial_tracer_updates'

Water tracer updates for initial things in driver loop

Update water tracers for initial stuff done in driver loop. This
includes atm2lnd forcings (non-downscaled and downscaled), balance check
initialization, and dyn subgrid updates.

Broadly speaking, the changes here are:

(1) Reworked WaterType to make it easier / more robust for other code to
    loop over tracers or bulk+tracers

(2) The most interesting changes are probably the code to update the
    atm2lnd water tracers (in Wateratm2lndType.F90 and
    WaterTracerUtils.F90)

(3) In various other places, do some infrastructurey stuff (initializing
    water balance, doing dyn subgrid stuff) for tracers as well as bulk

(4) Supporting unit tests and unit test infrastructure

Resolves #487
Resolves #488
Resolves #489

cime_config/testdefs/ExpectedTestFails.xml

@@ -7,6 +7,7 @@
     <entry issue="#442"            >FAIL SMS.f10_f10_musgs.I2000Clm50BgcCrop.hobart_pgi.clm-crop RUN</entry>
     <entry issue="#442"            >FAIL SMS_D.f10_f10_musgs.I2000Clm50BgcCrop.hobart_pgi.clm-crop RUN</entry>
     <entry issue="#550"            >FAIL ERS_D_Ln9_P480x3.f19_g16.I2000Clm50SpGs.cheyenne_intel.clm-waccmx_offline COMPARE_base_rest</entry>
+    <entry issue="ESMCI/cime#2917" >FAIL PFS_Ld20.f09_g17.I2000Clm50BgcCrop.cheyenne_intel GENERATE</entry>
   </category>
   <category name="fates">
     <entry issue="NGEET/fates#315">FAIL ERS_Ld60.f45_f45_mg37.I2000Clm45Fates.hobart_nag.clm-Fates COMPARE_base_rest</entry>

cime_config/testdefs/testlist_clm.xml

@@ -1980,6 +1980,16 @@
       </machine>
     </machines>
   </test>
+  <test name="PFS_Ld20" grid="f09_g17" compset="I2000Clm50BgcCrop">
+    <machines>
+      <machine name="cheyenne" compiler="intel" category="aux_clm">
+        <options>
+          <option name="wallclock">0:30</option>
+          <option name="comment">Can use this test to determine if there are significant throughput changes, at least for this common and important configuration. Note that this deliberately doesn't have any testmods in order to (1) avoid doing history output (because the timing of output can be very variable, and mixing output timing with other aspects of model time can be confusing), and (2) generally keep the test replicating a production configuration as closely as possible (so, for example, we do NOT set BFBFLAG=TRUE for this test).</option>
+        </options>
+      </machine>
+    </machines>
+  </test>
   <test name="FUNITCTSM_P1x1" grid="f10_f10_musgs" compset="I2000Clm50SpGs">
     <machines>
       <machine name="cheyenne" compiler="intel" category="aux_clm">

doc/.ChangeLog_template

@@ -46,7 +46,7 @@ Changes made to namelist defaults (e.g., changed parameter values):
 
 Changes to the datasets (e.g., parameter, surface or initial files):
 
-Substantial timing or memory changes:
+Substantial timing or memory changes: [For timing changes, can check PFS test(s) in the test suite]
 
 Notes of particular relevance for developers: (including Code reviews and testing)
 ---------------------------------------------

doc/ChangeLog

@@ -1,4 +1,147 @@
 ===============================================================
+Tag name:  ctsm1.0.dev018
+Originator(s):  sacks (Bill Sacks)
+Date: Thu Nov 29 16:03:50 MST 2018
+One-line Summary: Water tracer updates for initial things in driver loop
+
+Purpose of changes
+------------------
+
+Update water tracers for initial stuff done in driver loop. This includes
+atm2lnd forcings (non-downscaled and downscaled), balance check initialization,
+and dyn subgrid updates.
+
+Broadly speaking, the changes here are:
+
+(1) Reworked WaterType to make it easier / more robust for other code to loop
+    over tracers or bulk+tracers
+
+(2) The most interesting changes are probably the code to update the atm2lnd
+    water tracers (in Wateratm2lndType.F90 and WaterTracerUtils.F90)
+
+(3) In various other places, do some infrastructurey stuff (initializing water
+    balance, doing dyn subgrid stuff) for tracers as well as bulk
+
+(4) Supporting unit tests and unit test infrastructure
+
+
+Bugs fixed or introduced
+------------------------
+
+Issues fixed (include CTSM Issue #):
+Resolves ESCOMP/ctsm#487
+Resolves ESCOMP/ctsm#488
+Resolves ESCOMP/ctsm#489
+
+
+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
+
+[ ] clm4_5
+
+[ ] clm4_0
+
+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 ../CTSMMasterChecklist as well as the coding style in the Developers Guide
+
+Caveats for developers (e.g., code that is duplicated that requires double maintenance):
+- We have chosen not to set all water tracers as soon as possible, but instead
+  to do these tracer settings later in the driver loop. This keeps the driver
+  loop cleaner, but means that you cannot arbitrarily sprinkle calls to
+  TracerConsistencyCheck throughout the driver. Specifically for this tag: the
+  non-downscaled, gridcell-level atm2lnd water tracers are not updated until
+  after the call to downscale_forcings, so tracer consistency checks before that
+  point would fail.
+
+Changes to tests or testing:
+- Added a PFS test
+
+Code reviewed by: Portions of the design (and possibly code) have been reviewed
+by Mat Rothstein, David Noone and Mariana Vertenstein
+
+
+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
+
+  regular tests (aux_clm):
+
+    cheyenne ---- ok
+    hobart ------ ok
+
+    ok means tests pass, some answers change as expected
+
+CTSM tag used for the baseline comparisons: ctsm1.0.dev017
+
+
+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: many
+    - what platforms/compilers: all
+    - nature of change (roundoff; larger than roundoff/same climate; new climate): 
+      roundoff-level changes in sensible heat flux from precip conversion due to
+      refactoring this calculation; everything else bit-for-bit
+
+   If bitwise differences were observed, how did you show they were no worse
+   than roundoff? via summarize_cprnc_diffs to see differences in the 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
+
+   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):
+https://github.com/ESCOMP/ctsm/pull/572
+
+===============================================================
+===============================================================
 Tag name:  ctsm1.0.dev017
 Originator(s):  slevis (Samuel Levis, Slevis Consulting LLC,303-665-1310)
 Date: Wed Nov 28 14:27:50 MST 2018

doc/ChangeSum

@@ -1,5 +1,6 @@
 Tag                   Who      Date  Summary
 ============================================================================================================================
+  ctsm1.0.dev018    sacks 11/29/2018 Water tracer updates for initial things in driver loop
   ctsm1.0.dev017   slevis 11/28/2018 Merge the collapse2gencrop branch
   ctsm1.0.dev016    sacks 11/01/2018 Update cime, fix FATES DEBUG token, add script to easily run system tests
   ctsm1.0.dev015    sacks 10/28/2018 CMIP6 compset modifiers, usermods for typical output, and other output enhancements

doc/design/water_tracers.rst

@@ -113,6 +113,48 @@ of understanding the suite of water flux variables, and reducing the number of i
 that need to get passed all around (e.g., we won't need to pass irrigation_inst to as many
 places).
 
+========================
+ Loops over all tracers
+========================
+
+Initially, I was hoping that we could keep loops over tracers in WaterType, for the
+following reasons:
+
+1. To keep this complexity out of other modules
+
+2. To make it easier to change the details of how the bulk and tracer instances are
+   stored, if we ever need to: By keeping as many of the loops as possible in WaterType,
+   we reduce the number of places that would need to be changed
+
+However, it was starting to get too awkward to require all loops over tracers to happen in
+WaterType (or some other centralized location): I had originally imagined that we wouldn't
+need too many loops over tracers, but it turns out that we need loops over tracers in a
+lot of places. Requiring all of these loops over tracers to be in WaterType both (a)
+bloats that module, and (b) adds extra indirection (which makes it harder to understand
+the code, because you're bouncing back and forth between more modules, and has possible
+performance implications as we break routines into tiny pieces for this purpose).
+
+So we allow loops over tracers (or bulk plus tracers) anywhere in the code. See comments
+at the top of WaterType.F90 for example code showing how to write these loops.
+
+Note that the bulk instances (``waterfluxbulk_inst``, etc.) can be obtained in two ways:
+
+1. Using ``water_inst%water*bulk_inst``
+
+2. As one of the indices in ``water_inst%bulk_and_tracers(:)%water*_inst``
+
+Method (2) is just meant to be used when you are doing the same operation on bulk water
+and all water tracers. Reasons why it is better, or necessary, to use method (1) when you
+are really just working with bulk water are:
+
+- This makes it more explicit and clear that you are working with bulk water
+
+- When passing an argument to a subroutine whose dummy argument is of type
+  ``water*bulk_type``, method (2) only works if you surround the call with a ``select
+  type`` statement, which is awkward, to say the least. (Subroutines that expect bulk
+  water should generally declare their dummy arguments to be of type ``water*bulk_type``,
+  as discussed in `Object-oriented design for water tracer types`_.)
+
 ==============================================
  Infrastructure for looping through variables
 ==============================================