Capgen in SCM: Fix to allow for scheme subcycling. (#633)

Overview This PR contains changes to fix scheme subcycling in Capgen and extends the var_compatability_test to exercise subcycling. UPDATE: Added bugfix for suite-part list ordering (see change to ccpp_suite.py). Description Create local group variable for subcycle indexing. Fix bug (`self.loop` -> `self._loop`) in the Subcycle write phase, and in ccpp_datafile. User interface changes?: No Fixes: #632 Fixes: #634 Testing: Added to var_compatibility_test to exercise feature. --------- Co-authored-by: Dom Heinzeller <[email protected]> Co-authored-by: Steve Goldhaber <[email protected]>
NCAR · Feb 25, 2025 · dcb5ed5 · dcb5ed5
1 parent 6c494b2
commit dcb5ed5
Show file tree

Hide file tree

Showing 11 changed files with 62 additions and 47 deletions.
diff --git a/scripts/ccpp_datafile.py b/scripts/ccpp_datafile.py
@@ -1096,7 +1096,7 @@ def _add_suite_object(parent, suite_object):
         obj_elem.set("dimension_name", suite_object.dimension_name)
     # end if
     if isinstance(suite_object, Subcycle):
-        obj_elem.set("loop", suite_object.loop)
+        obj_elem.set("loop", suite_object._loop)
     # end if
     for obj_part in suite_object.parts:
         _add_suite_object(obj_elem, obj_part)

diff --git a/scripts/ccpp_suite.py b/scripts/ccpp_suite.py
@@ -773,7 +773,7 @@ def write_var_set_loop(ofile, varlist_name, var_list, indent,
         if add_allocate:
             ofile.write(f"allocate({varlist_name}({len(var_list)}))", indent)
         # end if
-        for ind, var in enumerate(sorted(var_list)):
+        for ind, var in enumerate(var_list):
             if start_var:
                 ind_str = f"{start_var} + {ind + start_index}"
             else:

diff --git a/scripts/metavar.py b/scripts/metavar.py
@@ -1677,7 +1677,7 @@ def add_variable(self, newvar, run_env, exists_ok=False, gen_unique=False,
                                        context=newvar.context)
             # end if
         # end if
-        # Check if local_name exists in Group. If applicable, Create new 
+        # Check if local_name exists in Group. If applicable, Create new
         # variable with unique name. There are two instances when new names are
         # created:
         # - Same <local_name> used in different DDTs.
@@ -1709,10 +1709,8 @@ def add_variable(self, newvar, run_env, exists_ok=False, gen_unique=False,
                 # same local_name
                 lname = new_lname
             elif not exists_ok:
-                errstr = 'Invalid local_name: {} already registered{}'
-                cstr = context_string(lvar.source.context, with_comma=True)
-                raise ParseSyntaxError(errstr.format(lname, cstr),
-                                       context=newvar.source.context)
+                errstr = f"Invalid local_name: {lname} already registered"
+                raise ParseSyntaxError(errstr, context=newvar.source.context)
             # end if (no else, things are okay)
         # end if (no else, things are okay)
         # Check if this variable has a parent (i.e., it is an array reference)

diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py
@@ -142,7 +142,7 @@ def call_string(self, cldicts=None, is_func_call=False, subname=None, sub_lname_
                         raise CCPPError(errmsg.format(stdname, clnames))
                     # end if
                     lname = dvar.get_prop_value('local_name')
-                    # Optional variables in the caps are associated with 
+                    # Optional variables in the caps are associated with
                     # local pointers of <lname>_ptr
                     if dvar.get_prop_value('optional'):
                         lname = dummy+'_ptr'
@@ -1161,7 +1161,7 @@ def update_group_call_list_variable(self, var):
                     gvar = None
                 # end if
             if gvar is None:
-                my_group.add_call_list_variable(var)
+                my_group.add_call_list_variable(var, gen_unique=True)
             # end if
         # end if
 
@@ -1219,7 +1219,7 @@ def analyze(self, phase, group, scheme_library, suite_vars, level):
                 # end if
                 # We have a match, make sure var is in call list
                 if new_dims == vdims:
-                    self.add_call_list_variable(var, exists_ok=True)
+                    self.add_call_list_variable(var, exists_ok=True, gen_unique=True)
                     self.update_group_call_list_variable(var)
                 else:
                     subst_dict = {'dimensions':new_dims}
@@ -1461,7 +1461,7 @@ def write_var_debug_check(self, var, internal_var, cldicts, outfile, errcode, er
         local_name = dvar.get_prop_value('local_name')
 
         # If the variable is allocatable and the intent for the scheme is 'out',
-        # then we can't test anything because the scheme is going to allocate 
+        # then we can't test anything because the scheme is going to allocate
         # the variable. We don't have this information earlier in
         # add_var_debug_check, therefore need to back out here,
         # using the information from the scheme variable (call list).
@@ -1794,11 +1794,11 @@ def write(self, outfile, errcode, errmsg, indent):
         #
         if self.__optional_vars:
             outfile.write('! Associate conditional variables', indent+1)
-        # end if 
+        # end if
         for (dict_var, var, var_ptr, has_transform) in self.__optional_vars:
             tstmt = self.associate_optional_var(dict_var, var, var_ptr, has_transform, cldicts, indent+1, outfile)
         # end for
-        # 
+        #
         # Write the scheme call.
         #
         if self._has_run_phase:
@@ -1813,7 +1813,7 @@ def write(self, outfile, errcode, errmsg, indent):
         #
         first_ptr_declaration=True
         for (dict_var, var, var_ptr, has_transform) in self.__optional_vars:
-            if first_ptr_declaration: 
+            if first_ptr_declaration:
                 outfile.write('! Copy any local pointers to dummy/local variables', indent+1)
                 first_ptr_declaration=False
             # end if
@@ -1977,23 +1977,29 @@ class Subcycle(SuiteObject):
     """Class to represent a subcycled group of schemes or scheme collections"""
 
     def __init__(self, sub_xml, context, parent, run_env):
-        name = sub_xml.get('name', None) # Iteration count
-        loop_extent = sub_xml.get('loop', "1") # Number of iterations
+        self._loop_extent = sub_xml.get('loop', "1") # Number of iterations
+        self._loop = None
         # See if our loop variable is an interger or a variable
         try:
-            loop_int = int(loop_extent) # pylint: disable=unused-variable
-            self._loop = loop_extent
+            _ = int(self._loop_extent)
+            self._loop = self._loop_extent
             self._loop_var_int = True
+            name = f"loop{self._loop}"
+            super().__init__(name, context, parent, run_env, active_call_list=False)
         except ValueError:
             self._loop_var_int = False
-            lvar = parent.find_variable(standard_name=self.loop, any_scope=True)
+            lvar = parent.find_variable(standard_name=self._loop_extent, any_scope=True)
             if lvar is None:
-                emsg = "Subcycle, {}, specifies {} iterations but {} not found"
-                raise CCPPError(emsg.format(name, self.loop, self.loop))
+                emsg = "Subcycle, {}, specifies {} iterations, variable not found"
+                raise CCPPError(emsg.format(name, self._loop_extent))
+            else:
+                self._loop_var_int = False
+                self._loop = lvar.get_prop_value('local_name')
             # end if
+            name = f"loop_{self._loop_extent}"[0:63]
+            super().__init__(name, context, parent, run_env, active_call_list=True)
             parent.add_call_list_variable(lvar)
         # end try
-        super().__init__(name, context, parent, run_env)
         for item in sub_xml:
             new_item = new_suite_object(item, context, self, run_env)
             self.add_part(new_item)
@@ -2004,12 +2010,11 @@ def analyze(self, phase, group, scheme_library, suite_vars, level):
         if self.name is None:
             self.name = "subcycle_index{}".format(level)
         # end if
-        # Create a variable for the loop index
-        self.add_variable(Var({'local_name':self.name,
-                               'standard_name':'loop_variable',
-                               'type':'integer', 'units':'count',
-                               'dimensions':'()'}, _API_SOURCE, self.run_env),
-                          self.run_env)
+        # Create a Group variable for the subcycle index.
+        newvar = Var({'local_name':self.name, 'standard_name':self.name,
+                      'type':'integer', 'units':'count', 'dimensions':'()'},
+                     _API_LOCAL, self.run_env)
+        group.manage_variable(newvar)
         # Handle all the suite objects inside of this subcycle
         scheme_mods = set()
         for item in self.parts:
@@ -2023,7 +2028,7 @@ def analyze(self, phase, group, scheme_library, suite_vars, level):
 
     def write(self, outfile, errcode, errmsg, indent):
         """Write code for the subcycle loop, including contents, to <outfile>"""
-        outfile.write('do {} = 1, {}'.format(self.name, self.loop), indent)
+        outfile.write('do {} = 1, {}'.format(self.name, self._loop), indent)
         # Note that 'scheme' may be a sybcycle or other construct
         for item in self.parts:
             item.write(outfile, errcode, errmsg, indent+1)
@@ -2033,13 +2038,7 @@ def write(self, outfile, errcode, errmsg, indent):
     @property
     def loop(self):
         """Return the loop value or variable local_name"""
-        lvar = self.find_variable(standard_name=self.loop, any_scope=True)
-        if lvar is None:
-            emsg = "Subcycle, {}, specifies {} iterations but {} not found"
-            raise CCPPError(emsg.format(self.name, self.loop, self.loop))
-        # end if
-        lname = lvar.get_prop_value('local_name')
-        return lname
+        return self._loop
 
 ###############################################################################
 
@@ -2408,8 +2407,8 @@ def write(self, outfile, host_arglist, indent, const_mod,
                     # end if
                 # end if
             # end for
-            # All optional dummy variables within group need to have 
-            # an associated pointer array declared. 
+            # All optional dummy variables within group need to have
+            # an associated pointer array declared.
             for cvar in self.call_list.variable_list():
                 opt_var = cvar.get_prop_value('optional')
                 if opt_var:

diff --git a/test/var_compatibility_test/effr_calc.F90 b/test/var_compatibility_test/effr_calc.F90
@@ -31,7 +31,7 @@ subroutine effr_calc_init(scheme_order, errmsg, errflg)
      endif
 
    end subroutine effr_calc_init
-  
+
    !> \section arg_table_effr_calc_run  Argument Table
    !! \htmlinclude arg_table_effr_calc_run.html
    !!
@@ -72,7 +72,7 @@ subroutine effr_calc_run(ncol, nlev, effrr_in, effrg_in, ncg_in, nci_out, &
       if (present(nci_out)) nci_out_local = nci_out
       effrl_inout = min(max(effrl_inout,re_qc_min),re_qc_max)
       if (present(effri_out)) effri_out   = re_qi_avg
-      effrs_inout = effrs_inout + 10.0 ! in micrometer
+      effrs_inout = effrs_inout + (10.0 / 6.0) ! in micrometer
       scalar_var = 2.0 ! in km
 
    end subroutine effr_calc_run

diff --git a/test/var_compatibility_test/run_test b/test/var_compatibility_test/run_test
@@ -143,6 +143,7 @@ required_vars_var_compatibility="${required_vars_var_compatibility},flag_indicat
 required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_dimension"
 required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_loop_begin"
 required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_loop_end"
+required_vars_var_compatibility="${required_vars_var_compatibility},num_subcycles_for_effr"
 required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing"
 required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing_a"
 required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing_b"
@@ -160,6 +161,7 @@ input_vars_var_compatibility="${input_vars_var_compatibility},flag_indicating_cl
 input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_dimension"
 input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_loop_begin"
 input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_loop_end"
+input_vars_var_compatibility="${input_vars_var_compatibility},num_subcycles_for_effr"
 input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing"
 input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing_a"
 input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing_b"

diff --git a/test/var_compatibility_test/test_host.F90 b/test/var_compatibility_test/test_host.F90
@@ -351,7 +351,7 @@ program test
 
    character(len=cs), target :: test_parts1(1) = (/ 'radiation       ' /)
 
-   character(len=cm), target :: test_invars1(12) = (/                         &
+   character(len=cm), target :: test_invars1(13) = (/                         &
         'effective_radius_of_stratiform_cloud_rain_particle        ',         &
         'effective_radius_of_stratiform_cloud_liquid_water_particle',         &
         'effective_radius_of_stratiform_cloud_snow_particle        ',         &
@@ -362,6 +362,7 @@ program test
         'scalar_variable_for_testing_b                             ',         &
         'scalar_variable_for_testing_c                             ',         &
         'scheme_order_in_suite                                     ',         &
+        'num_subcycles_for_effr                                    ',         &
         'flag_indicating_cloud_microphysics_has_graupel            ',         &
         'flag_indicating_cloud_microphysics_has_ice                '/)
 
@@ -375,9 +376,9 @@ program test
         'cloud_ice_number_concentration                            ',         &
         'scalar_variable_for_testing                               ',         &
         'scheme_order_in_suite                                     '/)
-
 
-   character(len=cm), target :: test_reqvars1(16) = (/                        &
+
+   character(len=cm), target :: test_reqvars1(17) = (/                        &
         'ccpp_error_code                                           ',         &
         'ccpp_error_message                                        ',         &
         'effective_radius_of_stratiform_cloud_rain_particle        ',         &
@@ -392,6 +393,7 @@ program test
         'scalar_variable_for_testing_b                             ',         &
         'scalar_variable_for_testing_c                             ',         &
         'scheme_order_in_suite                                     ', 	      &
+        'num_subcycles_for_effr                                    ',         &
         'flag_indicating_cloud_microphysics_has_graupel            ',         &
         'flag_indicating_cloud_microphysics_has_ice                '/)
 

diff --git a/test/var_compatibility_test/test_host_data.F90 b/test/var_compatibility_test/test_host_data.F90
@@ -17,6 +17,7 @@ module test_host_data
      real(kind_phys) :: scalar_varB
      integer :: scalar_varC
      integer :: scheme_order
+     integer :: num_subcycles
 
   end type physics_state
 
@@ -69,6 +70,8 @@ subroutine allocate_physics_state(cols, levels, state, has_graupel, has_ice)
 
     ! Initialize scheme counter.
     state%scheme_order = 1
+    ! Initialize subcycle counter.
+    state%num_subcycles = 3
 
   end subroutine allocate_physics_state
 

diff --git a/test/var_compatibility_test/test_host_data.meta b/test/var_compatibility_test/test_host_data.meta
@@ -85,3 +85,9 @@
   units = None
   dimensions = ()
   type = integer
+[num_subcycles]
+  standard_name = num_subcycles_for_effr
+  long_name = Number of times to subcycle the effr calculation
+  units = None
+  dimensions = ()
+  type = integer
diff --git a/test/var_compatibility_test/test_reports.py b/test/var_compatibility_test/test_reports.py
@@ -79,7 +79,8 @@ def usage(errmsg=None):
                           "scalar_variable_for_testing_c",
                           "scheme_order_in_suite",
                           "flag_indicating_cloud_microphysics_has_graupel",
-                          "flag_indicating_cloud_microphysics_has_ice"]
+                          "flag_indicating_cloud_microphysics_has_ice",
+                          "num_subcycles_for_effr"]
 _OUTPUT_VARS_VAR_ACTION = ["ccpp_error_code", "ccpp_error_message",
                            "effective_radius_of_stratiform_cloud_ice_particle",
                            "effective_radius_of_stratiform_cloud_liquid_water_particle",

diff --git a/test/var_compatibility_test/var_compatibility_suite.xml b/test/var_compatibility_test/var_compatibility_suite.xml
@@ -2,9 +2,13 @@
 
 <suite name="var_compatibility_suite" version="1.0">
   <group name="radiation">
-    <scheme>effr_pre</scheme>
-    <scheme>effr_calc</scheme>
-    <scheme>effr_post</scheme> 
+    <subcycle loop="num_subcycles_for_effr">
+      <scheme>effr_pre</scheme>
+      <subcycle loop="2">
+        <scheme>effr_calc</scheme>
+      </subcycle>
+      <scheme>effr_post</scheme>
+    </subcycle>
     <scheme>effr_diag</scheme>
   </group>
 </suite>