Merge pull request #1959 from rgknox/clmfates-cbalance

migrate FATES to use the normal BGC call sequence

.git-blame-ignore-revs

@@ -12,3 +12,4 @@ b771971e3299c4fa56534b93421f7a2b9c7282fd
 8bc4688e52ea23ef688e283698f70a44388373eb
 # Ran SystemTests and python/ctsm through black python formatter
 5364ad66eaceb55dde2d3d598fe4ce37ac83a93c
+8056ae649c1b37f5e10aaaac79005d6e3a8b2380

.github/workflows/black.yml

@@ -16,6 +16,22 @@ jobs:
              # Use options and version identical to the conda environment
              # Using pyproject.toml makes sure this testing is consistent with our python directory testing
              options: "--check --config python/pyproject.toml"
-             src:     "./python"
+             src: "./python"
              # Version should be coordinated with the ctsm_pylib conda environment under the python directory
              version: "22.3.0"
+          # Actions identical to above for each directory and source file we need to check (arrays aren't allowed for src: field)
+        - uses: psf/black@stable
+          with:
+             options: "--check --config python/pyproject.toml"
+             src: "./cime_config/SystemTests"
+             version: "22.3.0"
+        - uses: psf/black@stable
+          with:
+             options: "--check --config python/pyproject.toml"
+             src: "./cime_config/buildlib"
+             version: "22.3.0"
+        - uses: psf/black@stable
+          with:
+             options: "--check --config python/pyproject.toml"
+             src: "./cime_config/buildnml"
+             version: "22.3.0"

Externals_CLM.cfg

@@ -2,7 +2,7 @@
 local_path = src/fates
 protocol = git
 repo_url = https://github.com/NGEET/fates
-tag = sci.1.67.1_api.26.0.0
+tag = sci.1.67.1_api.27.0.0
 required = True
 
 [externals_description]

bld/CLMBuildNamelist.pm

@@ -91,11 +91,13 @@ OPTIONS
                                     This toggles off the namelist variable: use_cn
                                 bgc   = Carbon Nitrogen with methane, nitrification, vertical soil C,
                                         CENTURY or MIMICS decomposition
-                                    This toggles on the namelist variables:
+				        This toggles on the namelist variables:
                                           use_cn, use_lch4, use_nitrif_denitrif
-                                fates = FATES/Ecosystem Demography with below ground BGC
-                                    This toggles on the namelist variables:
-                                          use_fates
+				fates = FATES/Ecosystem Demography with below ground BGC
+				        CENTURY or MIMICS decomposition
+                                        This toggles on the namelist variables:
+				        use_fates. use_lch4 and use_nitrif_denitrif are optional
+				 
                               (Only for CLM4.5/CLM5.0)
      -[no-]chk_res            Also check [do NOT check] to make sure the resolution and
                               land-mask is valid.
@@ -763,26 +765,12 @@ sub setup_cmdl_fates_mode {
           }
        }
 
-      # The following variables may be set by the user and are compatible with use_fates
-      # no need to set defaults, covered in a different routine
-      my @list  = (  "use_lch4" );
-      foreach my $var ( @list ) {
-	  if ( defined($nl->get_value($var))  ) {
-	      $nl_flags->{$var} = $nl->get_value($var);
-	      $val = $nl_flags->{$var};
-	      my $group = $definition->get_group_name($var);
-	      $nl->set_variable_value($group, $var, $val);
-	      if (  ! $definition->is_valid_value( $var, $val ) ) {
-		  my @valid_values   = $definition->get_valid_values( $var );
-		  $log->fatal_error("$var has a value ($val) that is NOT valid. Valid values are: @valid_values");
-	      }
-	  }
-      }
     } else {
        # dis-allow fates specific namelist items with non-fates runs
        my @list  = (  "fates_spitfire_mode", "use_fates_planthydro", "use_fates_ed_st3", "use_fates_ed_prescribed_phys",
-                      "use_fates_cohort_age_tracking",
-                      "use_fates_inventory_init","use_fates_fixed_biogeog","use_fates_nocomp","use_fates_sp","fates_inventory_ctrl_filename","use_fates_logging","fates_parteh_mode","use_fates_tree_damage" );
+                      "use_fates_cohort_age_tracking","use_fates_inventory_init","use_fates_fixed_biogeog",
+		      "use_fates_nocomp","use_fates_sp","fates_inventory_ctrl_filename","use_fates_logging",
+		      "fates_parteh_mode","use_fates_tree_damage" );
        # dis-allow fates specific namelist items with non-fates runs
        foreach my $var ( @list ) {
           if ( defined($nl->get_value($var)) ) {
@@ -2990,24 +2978,30 @@ sub setup_logic_supplemental_nitrogen {
   my ($opts, $nl_flags, $definition, $defaults, $nl) = @_;
 
   if ( $nl_flags->{'bgc_mode'} ne "sp" && $nl_flags->{'bgc_mode'} ne "fates" && &value_is_true($nl_flags->{'use_crop'}) ) {
-    add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl,
-                'suplnitro', 'use_cn'=>$nl_flags->{'use_cn'}, 'use_crop'=>$nl_flags->{'use_crop'});
-  }
+      # If this is non-fates, non-sp and crop is active
+      add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl,
+		  'suplnitro', 'use_cn'=>$nl_flags->{'use_cn'}, 'use_crop'=>$nl_flags->{'use_crop'});
 
+  } elsif ( $nl_flags->{'bgc_mode'} eq "fates" && not &value_is_true( $nl_flags->{'use_fates_sp'})  ) {
+      # Or... if its fates but not fates-sp
+      add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl,
+		  'suplnitro', 'use_fates'=>$nl_flags->{'use_fates'});
+  }
+
   #
   # Error checking for suplnitro
   #
   my $suplnitro = $nl->get_value('suplnitro');
   if ( defined($suplnitro) ) {
     if ( $nl_flags->{'bgc_mode'} eq "sp" ) {
-      $log->fatal_error("supplemental Nitrogen (suplnitro) is set, but neither CN nor CNDV is active!");
+      $log->fatal_error("supplemental Nitrogen (suplnitro) is set, but neither CN nor CNDV nor FATES is active!");
     }
     if ( ! &value_is_true($nl_flags->{'use_crop'}) && $suplnitro =~ /PROG_CROP_ONLY/i ) {
       $log->fatal_error("supplemental Nitrogen is set to run over prognostic crops, but prognostic crop is NOT active!");
     }
 
     if ( $suplnitro =~ /ALL/i ) {
-      if ( $nl_flags->{'bgc_spinup'} eq "on" ) {
+      if ( $nl_flags->{'bgc_spinup'} eq "on" && $nl_flags->{'bgc_mode'} ne "fates" ) {
         $log->warning("There is no need to use a bgc_spinup mode when supplemental Nitrogen is on for all PFT's, as these modes spinup Nitrogen" );
       }
     }
@@ -3324,6 +3318,12 @@ sub setup_logic_luna {
                  'use_cn'=>$nl_flags->{'use_cn'} );
   }
   $nl_flags->{'use_luna'} = $nl->get_value('use_luna');
+
+  # LUNA can NOT be on with FATES
+  if ( &value_is_true( $nl_flags->{'use_luna'} ) && &value_is_true( $nl_flags->{'use_fates'} )) {
+     $log->fatal_error("Cannot turn use_luna to true when bgc=fates" );
+  }
+
   my $vcmax_opt= $nl->get_value('vcmax_opt');
   # lnc_opt only applies if luna is on or for vcmax_opt=3/4
   if ( &value_is_true( $nl_flags->{'use_luna'} ) || $vcmax_opt == 3 || $vcmax_opt == 4 ) {
@@ -3490,18 +3490,18 @@ sub setup_logic_nitrogen_deposition {
   my ($opts, $nl_flags, $definition, $defaults, $nl) = @_;
 
   #
-  # Nitrogen deposition for bgc=CN
+  # Nitrogen deposition for bgc=CN or fates
   #
-  if ( $nl_flags->{'bgc_mode'} =~/bgc/ ) {
+  if ( ($nl_flags->{'bgc_mode'} =~/bgc/) ) {   # or  ($nl_flags->{'bgc_mode'} =~/fates/) ) {
     add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'ndepmapalgo', 'phys'=>$nl_flags->{'phys'},
                 'use_cn'=>$nl_flags->{'use_cn'}, 'hgrid'=>$nl_flags->{'res'},
                 'clm_accelerated_spinup'=>$nl_flags->{'clm_accelerated_spinup'} );
     add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'ndep_taxmode', 'phys'=>$nl_flags->{'phys'},
-                'use_cn'=>$nl_flags->{'use_cn'},
-                'lnd_tuning_mode'=>$nl_flags->{'lnd_tuning_mode'} );
+		'use_cn'=>$nl_flags->{'use_cn'}, 
+		'lnd_tuning_mode'=>$nl_flags->{'lnd_tuning_mode'} );
     add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'ndep_varlist', 'phys'=>$nl_flags->{'phys'},
-                'use_cn'=>$nl_flags->{'use_cn'},
-                'lnd_tuning_mode'=>$nl_flags->{'lnd_tuning_mode'} );
+		'use_cn'=>$nl_flags->{'use_cn'}, 
+		'lnd_tuning_mode'=>$nl_flags->{'lnd_tuning_mode'} );
     add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'stream_year_first_ndep', 'phys'=>$nl_flags->{'phys'},
                 'use_cn'=>$nl_flags->{'use_cn'}, 'sim_year'=>$nl_flags->{'sim_year'},
                 'sim_year_range'=>$nl_flags->{'sim_year_range'});
@@ -4278,6 +4278,13 @@ sub setup_logic_fates {
  	  add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, $var, 'use_fates'=>$nl_flags->{'use_fates'},
                       'use_fates_sp'=>$nl_flags->{'use_fates_sp'} );
         }
+        my $suplnitro = $nl->get_value('suplnitro');
+        my $parteh_mode = $nl->get_value('fates_parteh_mode');
+        if ( ($parteh_mode == 1) &&  ($suplnitro !~ /ALL/) && not &value_is_true( $nl_flags->{'use_fates_sp'}) ) {
+          $log->fatal_error("supplemental Nitrogen (suplnitro) is NOT set to ALL, FATES is on, " . 
+                            "but and FATES-SP is not active, but fates_parteh_mode is 1, so Nitrogen is not active" . 
+                            "Change suplnitro back to ALL");
+        }
         #
         # For FATES SP mode make sure no-competetiion, and fixed-biogeography are also set
         # And also check for other settings that can't be trigged on as well

bld/namelist_files/namelist_defaults_ctsm.xml

@@ -146,7 +146,7 @@ attributes from the config_cache.xml file (with keys converted to upper-case).
 
 <!-- Supplmental Nitrogen mode -->
 <suplnitro use_cn=".true." >NONE</suplnitro>
-<suplnitro use_fates=".true." >NONE</suplnitro>
+<suplnitro use_fates=".true." >ALL</suplnitro>
 
 <!-- Albedo for glaciers -->
 <albice phys="clm5_1" >0.50,0.30</albice>
@@ -510,6 +510,7 @@ attributes from the config_cache.xml file (with keys converted to upper-case).
 <use_luna phys="clm5_0"  >.true.</use_luna>
 <use_luna phys="clm5_0"  use_fates=".true." >.false.</use_luna>
 <use_luna phys="clm4_5"  >.false.</use_luna>
+<use_luna phys="clm4_5"  use_fates=".true." >.false.</use_luna>
 
 <!-- Flexible CN options -->
 <MM_Nuptake_opt             use_flexibleCN=".true." >.true.</MM_Nuptake_opt>

bld/unit_testers/build-namelist_test.pl

@@ -164,6 +164,7 @@ sub cat_and_create_namelistinfile {
 # Figure out number of tests that will run
 #
 my $ntests = 1992;
+
 if ( defined($opts{'compare'}) ) {
    $ntests += 1353;
 }
@@ -1015,6 +1016,21 @@ sub cat_and_create_namelistinfile {
                                      GLC_TWO_WAY_COUPLING=>"FALSE",
                                      phys=>"clm5_1",
                                    },
+     "useFATESWluna"             =>{ options=>"--bgc fates --envxml_dir . --no-megan",
+                                     namelst=>"use_luna=TRUE",
+                                     GLC_TWO_WAY_COUPLING=>"FALSE",
+                                     phys=>"clm5_1",
+                                   },
+     "useFATESWfun"              =>{ options=>"--bgc fates --envxml_dir . --no-megan",
+                                     namelst=>"use_fun=TRUE",
+                                     GLC_TWO_WAY_COUPLING=>"FALSE",
+                                     phys=>"clm5_1",
+                                   },
+     "useFATESWOsuplnitro"       =>{ options=>"--bgc fates --envxml_dir . --no-megan",
+                                     namelst=>"suplnitro='NONE'",
+                                     GLC_TWO_WAY_COUPLING=>"FALSE",
+                                     phys=>"clm5_1",
+                                   },
      "FireNoneButBGCfireon"    =>{ options=>"-bgc bgc -envxml_dir . -light_res none",
                                      namelst=>"fire_method='li2021gswpfrc'",
                                      GLC_TWO_WAY_COUPLING=>"FALSE",

cime_config/SystemTests/rxcropmaturity.py

@@ -353,7 +353,7 @@ def _run_generate_gdds(self, case_gddgen):
                 f"--sdates-file {sdates_file}",
                 f"--hdates-file {hdates_file}",
                 f"--output-dir generate_gdds_out",
-                f"--skip-crops miscanthus,irrigated_miscanthus"
+                f"--skip-crops miscanthus,irrigated_miscanthus",
             ]
         )
         stu.run_python_script(

doc/ChangeLog

@@ -1,4 +1,85 @@
 ===============================================================
+Tag name: ctsm5.1.dev134
+Originator(s): rgknox (Ryan Knox,LBNL EESA), erik (Erik Kluzek,UCAR/TSS,303-497-1326)
+Date: Wed Aug 16 17:20:27 MDT 2023
+One-line Summary: Migration of FATES to share normal soil BGC call sequence and functionality
+
+Purpose and description of changes
+----------------------------------
+
+This set of changes enables the normal soil biogeochemistry that is used for CN, to be used for FATES as well. FATES had been using a simplified subset of soil biogeochemistry in its own module. This change required coordination of litter flux and methane boundary conditions from FATES to CLM.  CNVEG datastructures were given trivial allocation (of size one on index zero) to prevent inappropriate use of CNVEG datastructures while FATES is active. Note that now the carbon balance checking for the soil is now active when FATES is active. Various accomodations have also been put in place to enable nitrogen cycling between the two models.
+
+
+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_1
+
+[ ] clm5_0
+
+[ ] ctsm5_0-nwp
+
+[ ] clm4_5
+
+
+Bugs fixed or introduced
+------------------------
+Surprisingly, nvhpc tests are now working, but it may just be coincidental. All existing aux_clm tests are passing. Tests without FATES are b4b, with roundoff differences in just TOTCOLC and TOTCOLN.
+
+CTSM issues fixed (include CTSM Issue #):
+   We think #1879 -- "AD spinup issues for FATES", is fixed but haven't proved it
+   #2112 -- black check on SystemTest file
+
+Notes of particular relevance for users
+---------------------------------------
+A CLM-FATES simulation will turn on nitrogen supplementation, this enables sufficient immobilization and decomposition. Until FATES and CLM can handle fully coupled nitrogen exchange, which would include root uptake of the mineralized aqueous forms (NH4 and NO3), N limitations in the soil are meaningless when FATES is on.
+
+Caveats for users (e.g., need to interpolate initial conditions):
+    FATES MUST have suplnitro='ALL' now (was NONE). When fates_parteh_mode>=1 other settings are allowed.
+    More checking for use_luna and suplnitro is added for FATES in the build-namelist
+
+Changes made to namelist defaults (e.g., changed parameter values): FATES runs now supplement N
+   suplnitro set to ALL for FATES
+   use_luna set to .false. for FATES and clm4_5 physics
+
+Notes of particular relevance for developers:
+---------------------------------------------
+
+Caveats for developers (e.g., code that is duplicated that requires double maintenance):
+    We should update defaults for suplnitro, when Nitrogen nutrients are allowed in FATES
+    The black checdk github action has to duplicate actions for each source file or directory
+    We should move to using the Makefile in the python directory when we figure it out
+
+Testing summary:
+----------------
+
+aux_clm test run on cheyenne and izumi. See:
+
+izumi: OK  /scratch/cluster/rgknox/tests_0814-095624iz
+cheyenne: OK  /glade/scratch/rgknox/tests_0814-134713ch
+
+
+Answer changes
+--------------
+
+Changes answers relative to baseline: Two diganostic fields (TOTCOLC and TOTCOLN)
+
+Baseline changes will be reported for many tests, all tests were combed to identify RMS diffs, all non-FATES tests had at most, roundoff level (<e-12) diffs in TOTCOLC and TOTCOLN.  FATES tests have diffs due to the subtle but unavoidable differences in how the models are coupled.
+
+Other details
+-------------
+
+Pull Requests that document the changes (include PR ids):
+https://github.com/ESCOMP/CTSM/pull/1959
+
+
+===============================================================
+===============================================================
 Tag name: ctsm5.1.dev133
 Originator(s): adrifoster (Adrianna Foster), glemieux (Gregory Lemieux, LBL/NGEET)
 Date: Wed Aug  9 22:44:46 MDT 2023

doc/ChangeSum

@@ -1,5 +1,6 @@
 Tag                   Who      Date  Summary
 ============================================================================================================================
+    ctsm5.1.dev134   rgknox 08/16/2023 Migration of FATES to share normal soil BGC call sequence and functionality
     ctsm5.1.dev133 glemieux 08/09/2023 FATES API update to facilitate fates refactor
     ctsm5.1.dev132   slevis 08/04/2023 Add parameterization to allow excess ice in soil and subsidence
     ctsm5.1.dev131 samrabin 07/27/2023 Enable prescribed crop calendars