Address reviewer comments. Add expanded test for transforms

scripts/suite_objects.py

@@ -159,10 +159,10 @@ def call_string(self, cldicts=None, is_func_call=False, subname=None, sub_lname_
                     # end if
                 # end if
                 # Modify Scheme call_list to handle local_name change for this var.
-                # Are there any varaible transforms for this scheme?
+                # Are there any variable transforms for this scheme?
                 # If so, change Var's local_name need to local dummy array containing
                 # transformed argument, var_trans_local.
-                if (sub_lname_list is not None) and len(sub_lname_list) > 0:
+                if sub_lname_list:
                     for (sname, var_trans_local) in sub_lname_list:
                         if (sname == stdname):
                             lname = var_trans_local
@@ -1115,7 +1115,8 @@ def __init__(self, scheme_xml, context, parent, run_env):
         self.__var_debug_checks = list()
         self.__forward_transforms = list()
         self.__reverse_transforms = list()
-        self.__sub_lname_list = list()
+        self.__forward_sub_lname_list = list()
+        self.__reverse_sub_lname_list = list()
         self._has_run_phase = True
         self.__optional_vars = list()
         super().__init__(name, context, parent, run_env, active_call_list=True)
@@ -1259,7 +1260,7 @@ def analyze(self, phase, group, scheme_library, suite_vars, level):
             # end if
 
             # Is this a conditionally allocated variable?
-            # If so, declare localpointer varaible. This is needed to
+            # If so, declare localpointer variable. This is needed to
             # pass inactive (not present) status through the caps.
             if var.get_prop_value('optional'):
                 newvar_ptr = var.clone(var.get_prop_value('local_name')+'_ptr')
@@ -1608,7 +1609,7 @@ def add_var_transform(self, var, compat_obj, vert_dim):
         for the transform and save for use during write stage"""
 
         # Add local variable (<var(local_name)>_local) needed for transformation.
-        # Do not let the Group manage this varaible. Handle local var
+        # Do not let the Group manage this variable. Handle local var
         # when writing Group.
         prop_dict = var.copy_prop_dict()
         prop_dict['local_name'] = var.get_prop_value('local_name')+'_local'
@@ -1622,7 +1623,7 @@ def add_var_transform(self, var, compat_obj, vert_dim):
                               self.run_env)
         found = self.__group.find_variable(source_var=local_trans_var, any_scope=False)
         if not found:
-            lmsg = "Adding new local Group variable, '{}', for variable transform"
+            lmsg = "Adding new local variable, '{}', for variable transform"
             self.run_env.logger.info(lmsg.format(local_trans_var.get_prop_value('local_name')))
             self.__group.transform_locals.append(local_trans_var)
         # end if
@@ -1676,8 +1677,8 @@ def add_var_transform(self, var, compat_obj, vert_dim):
                                               var.get_prop_value('local_name'),
                                               rindices, lindices, compat_obj,
                                               var.get_prop_value('standard_name')])
-            self.__sub_lname_list.append([var.get_prop_value('standard_name'),
-                                          local_trans_var.get_prop_value('local_name')])
+            self.__reverse_sub_lname_list.append([var.get_prop_value('standard_name'),
+                                                  local_trans_var.get_prop_value('local_name')])
         # end if
         # Register any forward (post-Scheme) transforms.
         if (var.get_prop_value('intent') != 'in'):
@@ -1689,11 +1690,13 @@ def add_var_transform(self, var, compat_obj, vert_dim):
             self.__forward_transforms.append([var.get_prop_value('local_name'),
                                               local_trans_var.get_prop_value('local_name'),
                                               lindices, rindices, compat_obj])
+            self.__forward_sub_lname_list.append([var.get_prop_value('standard_name'),
+                                                  local_trans_var.get_prop_value('local_name')])
         # end if
     def write_var_transform(self, var, dummy, rindices, lindices, compat_obj,
                             outfile, indent, forward):
         """Write variable transformation needed to call this Scheme in <outfile>.
-        <var> is the varaible that needs transformation before and after calling Scheme.
+        <var> is the variable that needs transformation before and after calling Scheme.
         <dummy> is the local variable needed for the transformation..
         <lindices> are the LHS indices of <dummy> for reverse transforms (before Scheme).
         <rindices> are the RHS indices of <var>   for reverse transforms (before Scheme).
@@ -1732,7 +1735,7 @@ def write(self, outfile, errcode, errmsg, indent):
         my_args = self.call_list.call_string(cldicts=cldicts,
                                              is_func_call=True,
                                              subname=self.subroutine_name,
-                                             sub_lname_list = self.__sub_lname_list)
+                                             sub_lname_list = self.__reverse_sub_lname_list)
         #
         outfile.write('', indent)
         outfile.write('if ({} == 0) then'.format(errcode), indent)
@@ -1760,8 +1763,16 @@ def write(self, outfile, errcode, errmsg, indent):
         if len(self.__reverse_transforms) > 0:
             outfile.comment('Compute reverse (pre-scheme) transforms', indent+1)
         # end if
-        for (dummy, var, rindices, lindices, compat_obj, __) in self.__reverse_transforms:
-            tstmt = self.write_var_transform(var, dummy, rindices, lindices, compat_obj, outfile, indent+1, False)
+        for rcnt, (dummy, var, rindices, lindices, compat_obj, __) in enumerate(self.__reverse_transforms):
+            # Any transform(s) were added during the Group's analyze phase, but
+            # the local_name(s) of the <var> assoicated with the transform(s)
+            # may have since changed. Here we need to use the standard_name
+            # from <var> and replace its local_name with the local_name from the
+            # Group's call_list.
+            lname      = self.__reverse_sub_lname_list[rcnt][0]
+            lvar       = self.__group.call_list.find_variable(standard_name=lname)
+            lvar_lname = lvar.get_prop_value('local_name')
+            tstmt = self.write_var_transform(lvar_lname, dummy, rindices, lindices, compat_obj, outfile, indent+1, False)
         # end for
         outfile.write('',indent+1)
         #
@@ -1801,7 +1812,15 @@ def write(self, outfile, errcode, errmsg, indent):
         if len(self.__forward_transforms) > 0:
             outfile.comment('Compute forward (post-scheme) transforms', indent+1)
         # end if
-        for (var, dummy, lindices, rindices, compat_obj) in self.__forward_transforms:
+        for fcnt, (var, dummy, lindices, rindices, compat_obj) in enumerate(self.__forward_transforms):
+            # Any transform(s) were added during the Group's analyze phase, but
+            # the local_name(s) of the <var> assoicated with the transform(s)
+            # may have since changed. Here we need to use the standard_name
+            # from <var> and replace its local_name with the local_name from the
+            # Group's call_list.
+            lname      = self.__forward_sub_lname_list[fcnt][0]
+            lvar       = self.__group.call_list.find_variable(standard_name=lname)
+            lvar_lname = lvar.get_prop_value('local_name')
             tstmt = self.write_var_transform(var, dummy, rindices, lindices, compat_obj, outfile, indent+1, True)
         # end for
         outfile.write('', indent)

test/var_compatibility_test/effr_diag.F90

@@ -0,0 +1,42 @@
+!Test unit conversions for intent in, inout, out variables
+!
+
+module effr_diag
+
+   use ccpp_kinds, only: kind_phys
+
+   implicit none
+   private
+
+   public :: effr_diag_run
+
+contains
+
+   !> \section arg_table_effr_diag_run  Argument Table
+   !! \htmlinclude arg_table_effr_diag_run.html
+   !!
+   subroutine effr_diag_run( effrr_in,  errmsg, errflg)
+
+      real(kind_phys),    intent(in)    :: effrr_in(:,:)
+      character(len=512), intent(out)   :: errmsg
+      integer,            intent(out)   :: errflg
+      !----------------------------------------------------------------
+      real(kind_phys) :: effrr_min, effrr_max
+
+      errmsg = ''
+      errflg = 0
+
+      call cmp_effr_diag(effrr_in, effrr_min, effrr_max)
+
+   end subroutine effr_diag_run
+
+   subroutine cmp_effr_diag(effr, effr_min, effr_max)
+     real(kind_phys), intent(in)  :: effr(:,:)
+     real(kind_phys), intent(out) :: effr_min, effr_max
+
+     ! Do some diagnostic calcualtions...
+     effr_min = minval(effr)
+     effr_max = maxval(effr)
+
+   end subroutine cmp_effr_diag
+end module effr_diag

test/var_compatibility_test/effr_diag.meta

@@ -0,0 +1,31 @@
+[ccpp-table-properties]
+  name = effr_diag
+  type = scheme
+  dependencies =
+[ccpp-arg-table]
+  name = effr_diag_run
+  type = scheme
+[effrr_in]
+  standard_name = effective_radius_of_stratiform_cloud_rain_particle
+  long_name = effective radius of cloud rain particle in micrometer
+  units = um
+  dimensions = (horizontal_loop_extent,vertical_layer_dimension)
+  type = real
+  kind = kind_phys
+  intent = in
+  top_at_one = True
+[ errmsg ]
+  standard_name = ccpp_error_message
+  long_name = Error message for error handling in CCPP
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  long_name = Error flag for error handling in CCPP
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out

test/var_compatibility_test/effr_post.F90

@@ -0,0 +1,34 @@
+!Test unit conversions for intent in, inout, out variables
+!
+
+module effr_post
+
+   use ccpp_kinds, only: kind_phys
+
+   implicit none
+   private
+
+   public :: effr_post_run
+
+contains
+
+   !> \section arg_table_effr_post_run  Argument Table
+   !! \htmlinclude arg_table_effr_post_run.html
+   !!
+   subroutine effr_post_run( effrr_inout,  errmsg, errflg)
+
+      real(kind_phys),    intent(inout) :: effrr_inout(:,:)
+      character(len=512), intent(out)   :: errmsg
+      integer,            intent(out)   :: errflg
+      !----------------------------------------------------------------
+      real(kind_phys) :: effrr_min, effrr_max
+
+      errmsg = ''
+      errflg = 0
+
+      ! Do some post-processing on effrr...
+      effrr_inout(:,:) = effrr_inout(:,:)*1._kind_phys
+
+   end subroutine effr_post_run
+
+ end module effr_post

test/var_compatibility_test/effr_post.meta

@@ -0,0 +1,30 @@
+[ccpp-table-properties]
+  name = effr_post
+  type = scheme
+  dependencies =
+[ccpp-arg-table]
+  name = effr_post_run
+  type = scheme
+[effrr_inout]
+  standard_name = effective_radius_of_stratiform_cloud_rain_particle
+  long_name = effective radius of cloud rain particle in micrometer
+  units = m
+  dimensions = (horizontal_loop_extent,vertical_layer_dimension)
+  type = real
+  kind = kind_phys
+  intent = inout
+[ errmsg ]
+  standard_name = ccpp_error_message
+  long_name = Error message for error handling in CCPP
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  long_name = Error flag for error handling in CCPP
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out

test/var_compatibility_test/effr_pre.F90

@@ -0,0 +1,34 @@
+!Test unit conversions for intent in, inout, out variables
+!
+
+module effr_pre
+
+   use ccpp_kinds, only: kind_phys
+
+   implicit none
+   private
+
+   public :: effr_pre_run
+
+contains
+
+   !> \section arg_table_effr_pre_run  Argument Table
+   !! \htmlinclude arg_table_effr_pre_run.html
+   !!
+   subroutine effr_pre_run( effrr_inout,  errmsg, errflg)
+
+      real(kind_phys),    intent(inout) :: effrr_inout(:,:)
+      character(len=512), intent(out)   :: errmsg
+      integer,            intent(out)   :: errflg
+      !----------------------------------------------------------------
+      real(kind_phys) :: effrr_min, effrr_max
+
+      errmsg = ''
+      errflg = 0
+
+      ! Do some pre-processing on effrr...
+      effrr_inout(:,:) = effrr_inout(:,:)*1._kind_phys
+
+   end subroutine effr_pre_run
+
+ end module effr_pre

test/var_compatibility_test/effr_pre.meta

@@ -0,0 +1,30 @@
+[ccpp-table-properties]
+  name = effr_pre
+  type = scheme
+  dependencies =
+[ccpp-arg-table]
+  name = effr_pre_run
+  type = scheme
+[effrr_inout]
+  standard_name = effective_radius_of_stratiform_cloud_rain_particle
+  long_name = effective radius of cloud rain particle in micrometer
+  units = m
+  dimensions = (horizontal_loop_extent,vertical_layer_dimension)
+  type = real
+  kind = kind_phys
+  intent = inout
+[ errmsg ]
+  standard_name = ccpp_error_message
+  long_name = Error message for error handling in CCPP
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  long_name = Error flag for error handling in CCPP
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out

test/var_compatibility_test/run_test

@@ -127,7 +127,7 @@ ccpp_files="${utility_files}"
 ccpp_files="${ccpp_files},${build_dir}/ccpp/test_host_ccpp_cap.F90"
 ccpp_files="${ccpp_files},${build_dir}/ccpp/ccpp_var_compatibility_suite_cap.F90"
 #process_list=""
-module_list="effr_calc"
+module_list="effr_calc,effr_diag,effr_post,effr_pre"
 #dependencies=""
 suite_list="var_compatibility_suite"
 required_vars_var_compatibility="ccpp_error_code,ccpp_error_message"
@@ -162,10 +162,10 @@ output_vars_var_compatibility="ccpp_error_code,ccpp_error_message"
 output_vars_var_compatibility="${output_vars_var_compatibility},cloud_ice_number_concentration"
 output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_ice_particle"
 output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_liquid_water_particle"
+output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_rain_particle"
 output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_snow_particle"
 output_vars_var_compatibility="${output_vars_var_compatibility},scalar_variable_for_testing"
 
-
 ##
 ## Run a database report and check the return string
 ## $1 is the report program file

test/var_compatibility_test/test_host.F90

@@ -361,11 +361,12 @@ program test
         'flag_indicating_cloud_microphysics_has_graupel            ',         &
         'flag_indicating_cloud_microphysics_has_ice                '/)
 
-   character(len=cm), target :: test_outvars1(7) = (/                         &
+   character(len=cm), target :: test_outvars1(8) = (/                         &
         'ccpp_error_code                                           ',         &
         'ccpp_error_message                                        ',         &
         'effective_radius_of_stratiform_cloud_ice_particle         ',         &
         'effective_radius_of_stratiform_cloud_liquid_water_particle',         &
+        'effective_radius_of_stratiform_cloud_rain_particle        ',         &
         'effective_radius_of_stratiform_cloud_snow_particle        ',         &
         'cloud_ice_number_concentration                            ',         &
         'scalar_variable_for_testing                               ' /)

test/var_compatibility_test/test_reports.py

@@ -65,7 +65,7 @@ def usage(errmsg=None):
 _CCPP_FILES = _UTILITY_FILES + \
               [os.path.join(_BUILD_DIR, "ccpp", "test_host_ccpp_cap.F90"),
                os.path.join(_BUILD_DIR, "ccpp", "ccpp_var_compatibility_suite_cap.F90")]
-_MODULE_LIST = ["effr_calc"]
+_MODULE_LIST = ["effr_calc", "effr_diag", "effr_post", "effr_pre"]
 _SUITE_LIST = ["var_compatibility_suite"]
 _INPUT_VARS_VAR_ACTION = ["horizontal_loop_begin", "horizontal_loop_end", "horizontal_dimension", "vertical_layer_dimension",
                           "effective_radius_of_stratiform_cloud_liquid_water_particle",
@@ -81,6 +81,7 @@ def usage(errmsg=None):
                            "effective_radius_of_stratiform_cloud_liquid_water_particle",
                            "effective_radius_of_stratiform_cloud_snow_particle",
                            "cloud_ice_number_concentration",
+                           "effective_radius_of_stratiform_cloud_rain_particle",
                            "scalar_variable_for_testing"]
 _REQUIRED_VARS_VAR_ACTION = _INPUT_VARS_VAR_ACTION + _OUTPUT_VARS_VAR_ACTION
 

test/var_compatibility_test/var_compatibility_files.txt

@@ -1 +1,4 @@
 effr_calc.meta
+effr_diag.meta
+effr_pre.meta
+effr_post.meta

test/var_compatibility_test/var_compatibility_suite.xml

@@ -2,6 +2,9 @@
 
 <suite name="var_compatibility_suite" version="1.0">
   <group name="radiation">
+    <scheme>effr_pre</scheme>
     <scheme>effr_calc</scheme>
+    <scheme>effr_post</scheme> 
+    <scheme>effr_diag</scheme>
   </group>
 </suite>