From 4d24e9ff489de9434d264ccb916410b90ad9dae1 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Fri, 25 Oct 2024 17:57:09 +0000 Subject: [PATCH] Address reviewer comments. Add expanded test for transforms --- scripts/suite_objects.py | 45 +++++++++++++------ test/var_compatibility_test/effr_diag.F90 | 42 +++++++++++++++++ test/var_compatibility_test/effr_diag.meta | 31 +++++++++++++ test/var_compatibility_test/effr_post.F90 | 34 ++++++++++++++ test/var_compatibility_test/effr_post.meta | 30 +++++++++++++ test/var_compatibility_test/effr_pre.F90 | 34 ++++++++++++++ test/var_compatibility_test/effr_pre.meta | 30 +++++++++++++ test/var_compatibility_test/run_test | 4 +- test/var_compatibility_test/test_host.F90 | 3 +- test/var_compatibility_test/test_reports.py | 3 +- .../var_compatibility_files.txt | 3 ++ .../var_compatibility_suite.xml | 3 ++ 12 files changed, 245 insertions(+), 17 deletions(-) create mode 100644 test/var_compatibility_test/effr_diag.F90 create mode 100644 test/var_compatibility_test/effr_diag.meta create mode 100644 test/var_compatibility_test/effr_post.F90 create mode 100644 test/var_compatibility_test/effr_post.meta create mode 100644 test/var_compatibility_test/effr_pre.F90 create mode 100644 test/var_compatibility_test/effr_pre.meta diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 7db44311..639ee7d5 100755 --- a/scripts/suite_objects.py +++ b/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 (_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 . - is the varaible that needs transformation before and after calling Scheme. + is the variable that needs transformation before and after calling Scheme. is the local variable needed for the transformation.. are the LHS indices of for reverse transforms (before Scheme). are the RHS indices of 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 assoicated with the transform(s) + # may have since changed. Here we need to use the standard_name + # from 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 assoicated with the transform(s) + # may have since changed. Here we need to use the standard_name + # from 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) diff --git a/test/var_compatibility_test/effr_diag.F90 b/test/var_compatibility_test/effr_diag.F90 new file mode 100644 index 00000000..38b87c1a --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/effr_diag.meta b/test/var_compatibility_test/effr_diag.meta new file mode 100644 index 00000000..ebb765f2 --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/effr_post.F90 b/test/var_compatibility_test/effr_post.F90 new file mode 100644 index 00000000..ca04c247 --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/effr_post.meta b/test/var_compatibility_test/effr_post.meta new file mode 100644 index 00000000..d65be238 --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/effr_pre.F90 b/test/var_compatibility_test/effr_pre.F90 new file mode 100644 index 00000000..ba6ea2b9 --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/effr_pre.meta b/test/var_compatibility_test/effr_pre.meta new file mode 100644 index 00000000..d6f67ec3 --- /dev/null +++ b/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 diff --git a/test/var_compatibility_test/run_test b/test/var_compatibility_test/run_test index a5edac37..5a1d6b5c 100755 --- a/test/var_compatibility_test/run_test +++ b/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 diff --git a/test/var_compatibility_test/test_host.F90 b/test/var_compatibility_test/test_host.F90 index 3a805a8e..3bb50da4 100644 --- a/test/var_compatibility_test/test_host.F90 +++ b/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 ' /) diff --git a/test/var_compatibility_test/test_reports.py b/test/var_compatibility_test/test_reports.py index f7c37897..6f10fc6d 100755 --- a/test/var_compatibility_test/test_reports.py +++ b/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 diff --git a/test/var_compatibility_test/var_compatibility_files.txt b/test/var_compatibility_test/var_compatibility_files.txt index 5f7db5b2..6d83c980 100644 --- a/test/var_compatibility_test/var_compatibility_files.txt +++ b/test/var_compatibility_test/var_compatibility_files.txt @@ -1 +1,4 @@ effr_calc.meta +effr_diag.meta +effr_pre.meta +effr_post.meta diff --git a/test/var_compatibility_test/var_compatibility_suite.xml b/test/var_compatibility_test/var_compatibility_suite.xml index 4b70fe48..5956a8bd 100644 --- a/test/var_compatibility_test/var_compatibility_suite.xml +++ b/test/var_compatibility_test/var_compatibility_suite.xml @@ -2,6 +2,9 @@ + effr_pre effr_calc + effr_post + effr_diag