From b98c853b99b28f40da0a7d718a388f72b2804eb2 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Tue, 22 Oct 2024 21:04:47 +0000 Subject: [PATCH 1/8] Changes to how auto conversions are handled in Group Cap. --- scripts/suite_objects.py | 114 ++++++++++++++++++++++++++++++--------- scripts/var_props.py | 5 +- 2 files changed, 92 insertions(+), 27 deletions(-) diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 1056b8dd..addb2b6c 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -107,13 +107,14 @@ def add_variable(self, newvar, run_env, exists_ok=False, gen_unique=False, super().add_variable(newvar, run_env, exists_ok=exists_ok, gen_unique=gen_unique, adjust_intent=adjust_intent) - def call_string(self, cldicts=None, is_func_call=False, subname=None): + def call_string(self, cldicts=None, is_func_call=False, subname=None, sub_lname_list=None): """Return a dummy argument string for this call list. may be a list of VarDictionary objects to search for local_names (default is to use self). should be set to True to construct a call statement. If is False, construct a subroutine dummy argument list. + may be a list of local_name substitutions. """ arg_str = "" arg_sep = "" @@ -157,6 +158,17 @@ def call_string(self, cldicts=None, is_func_call=False, subname=None): lname = dummy # end if # end if + # Modify Scheme call_list to handle local_name change for this var. + # Are there any varaible 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: + for (sname, var_trans_local) in sub_lname_list: + if (sname == stdname): + lname = var_trans_local + # end if + # end for + # end if if is_func_call: if cldicts is not None: use_dicts = cldicts @@ -877,9 +889,6 @@ def match_variable(self, var, run_env): new_dict_dims = dict_dims match = True # end if - # Create compatability object, containing any necessary forward/reverse - # transforms from and - compat_obj = var.compatible(dict_var, run_env) # If variable is defined as "inactive" by the host, ensure that # this variable is declared as "optional" by the scheme. If # not satisfied, return error. @@ -912,6 +921,15 @@ def match_variable(self, var, run_env): # end if # end if # end if + # We have a match! + # Are the Scheme's and Host's compatible? + # If so, create compatability object, containing any necessary + # forward/reverse transforms to/from and . + if dict_var is not None: + dict_var = self.parent.find_variable(source_var=var, any_scope=True) + print("SWALES dict_varA",dict_var,dict_var.get_prop_value('units'),var.get_prop_value('units')) + compat_obj = var.compatible(dict_var, run_env) + # end if return found_var, dict_var, var_vdim, new_vdims, missing_vert, compat_obj def in_process_split(self): @@ -1098,6 +1116,7 @@ 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._has_run_phase = True self.__optional_vars = list() super().__init__(name, context, parent, run_env, active_call_list=True) @@ -1589,9 +1608,25 @@ def add_var_transform(self, var, compat_obj, vert_dim): from to perform the transformation. Determine the indices needed for the transform and save for use during write stage""" - # Add dummy variable (_local) needed for transformation. - dummy = var.clone(var.get_prop_value('local_name')+'_local') - self.__group.manage_variable(dummy) + # Add local variable (_local) needed for transformation. + # Do not let the Group manage this varaible. Handle local var + # when writing Group. + prop_dict = var.copy_prop_dict() + prop_dict['local_name'] = var.get_prop_value('local_name')+'_local' + # This is a local variable. + if 'intent' in prop_dict: + del prop_dict['intent'] + # end if + local_trans_var = Var(prop_dict, + ParseSource(_API_SOURCE_NAME, + _API_LOCAL_VAR_NAME, var.context), + 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" + 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 # Create indices (default) for transform. lindices = [':']*var.get_rank() @@ -1630,22 +1665,32 @@ def add_var_transform(self, var, compat_obj, vert_dim): #hdim = find_horizontal_dimension(var.get_dimensions()) #if compat_obj.has_dim_transforms: - # - # Register any reverse (pre-Scheme) transforms. - # + # Register any reverse (pre-Scheme) transforms. Also, save local_name used in + # transform (used in write stage). if (var.get_prop_value('intent') != 'out'): - self.__reverse_transforms.append([dummy.get_prop_value('local_name'), + lmsg = "Automatic unit conversion from '{}' to '{}' for '{}' before entering '{}'" + self.run_env.logger.info(lmsg.format(compat_obj.v2_units, + compat_obj.v1_units, + compat_obj.v2_stdname, + compat_obj.v1_stdname)) + self.__reverse_transforms.append([local_trans_var.get_prop_value('local_name'), var.get_prop_value('local_name'), - rindices, lindices, compat_obj]) - - # + 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')]) + # end if # Register any forward (post-Scheme) transforms. - # if (var.get_prop_value('intent') != 'in'): + lmsg = "Automatic unit conversion from '{}' to '{}' for '{}' after returning '{}'" + self.run_env.logger.info(lmsg.format(compat_obj.v1_units, + compat_obj.v2_units, + compat_obj.v1_stdname, + compat_obj.v2_stdname)) self.__forward_transforms.append([var.get_prop_value('local_name'), - dummy.get_prop_value('local_name'), + local_trans_var.get_prop_value('local_name'), lindices, rindices, compat_obj]) - + # 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 . @@ -1687,7 +1732,8 @@ def write(self, outfile, errcode, errmsg, indent): cldicts.extend(self.__group.suite_dicts()) my_args = self.call_list.call_string(cldicts=cldicts, is_func_call=True, - subname=self.subroutine_name) + subname=self.subroutine_name, + sub_lname_list = self.__sub_lname_list) # outfile.write('', indent) outfile.write('if ({} == 0) then'.format(errcode), indent) @@ -1715,7 +1761,7 @@ 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: + 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) # end for outfile.write('',indent+1) @@ -2103,6 +2149,7 @@ def __init__(self, group_xml, transition, parent, context, run_env): self._phase_check_stmts = list() self._set_state = None self._ddt_library = None + self.transform_locals = list() def phase_match(self, scheme_name): """If scheme_name matches the group phase, return the group and @@ -2299,6 +2346,7 @@ def write(self, outfile, host_arglist, indent, const_mod, optional_var_set = set() pointer_var_set = list() inactive_var_set = set() + local_var_set = set() for item in [self]:# + self.parts: for var in item.declarations(): lname = var.get_prop_value('local_name') @@ -2369,6 +2417,19 @@ def write(self, outfile, host_arglist, indent, const_mod, # end if pointer_var_set.append([name,kind,dimstr,vtype]) # end for + # Any arguments used in variable transforms before or after the + # Scheme call? If so, declare local copy for reuse in the Group cap. + for ivar in self.transform_locals: + lname = ivar.get_prop_value('local_name') + opt_var = ivar.get_prop_value('optional') + dims = ivar.get_dimensions() + if (dims is not None) and dims: + subpart_allocate_vars[lname] = (ivar, item, opt_var) + allocatable_var_set.add(lname) + else: + subpart_scalar_vars[lname] = (ivar, item, opt_var) + # end if + # end for # end for # First, write out the subroutine header @@ -2477,6 +2538,14 @@ def write(self, outfile, host_arglist, indent, const_mod, self._phase_check_stmts.write(outfile, indent, {'errcode' : errcode, 'errmsg' : errmsg, 'funcname' : self.name}) + # Write any loop match calculations + outfile.write("! Set horizontal loop extent",indent+1) + for vmatch in self._loop_var_matches: + action = vmatch.write_action(self, dict2=self.call_list) + if action: + outfile.write(action, indent+1) + # end if + # end for # Allocate local arrays outfile.write('\n! Allocate local arrays', indent+1) alloc_stmt = "allocate({}({}))" @@ -2508,13 +2577,6 @@ def write(self, outfile, host_arglist, indent, const_mod, # end if dims (do not allocate scalars) # end for # end if - # Write any loop match calculations - for vmatch in self._loop_var_matches: - action = vmatch.write_action(self, dict2=self.call_list) - if action: - outfile.write(action, indent+1) - # end if - # end for # Write the scheme and subcycle calls for item in self.parts: item.write(outfile, errcode, errmsg, indent + 1) diff --git a/scripts/var_props.py b/scripts/var_props.py index 9a2fd7d0..d73ed241 100755 --- a/scripts/var_props.py +++ b/scripts/var_props.py @@ -874,6 +874,10 @@ def __init__(self, var1_stdname, var1_type, var1_kind, var1_units, self.__v2_context = v2_context self.__v1_kind = var1_kind self.__v2_kind = var2_kind + self.v1_units = var1_units + self.v2_units = var2_units + self.v1_stdname = var1_stdname + self.v2_stdname = var2_stdname # Default (null) transform information self.__dim_transforms = None self.__kind_transforms = None @@ -938,7 +942,6 @@ def __init__(self, var1_stdname, var1_type, var1_kind, var1_units, if self.__compat: # Check units argument if var1_units != var2_units: - self.__equiv = False # Try to find a set of unit conversions self.__unit_transforms = self._get_unit_convstrs(var1_units, var2_units) From 2eee649b2f15ea043b6710e6665144a74ccf7b64 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Tue, 22 Oct 2024 21:06:06 +0000 Subject: [PATCH 2/8] Omission from previous commit --- scripts/suite_objects.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index addb2b6c..7db44311 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -927,7 +927,6 @@ def match_variable(self, var, run_env): # forward/reverse transforms to/from and . if dict_var is not None: dict_var = self.parent.find_variable(source_var=var, any_scope=True) - print("SWALES dict_varA",dict_var,dict_var.get_prop_value('units'),var.get_prop_value('units')) compat_obj = var.compatible(dict_var, run_env) # end if return found_var, dict_var, var_vdim, new_vdims, missing_vert, compat_obj From 4d24e9ff489de9434d264ccb916410b90ad9dae1 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Fri, 25 Oct 2024 17:57:09 +0000 Subject: [PATCH 3/8] 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 From 93403c2e81a9b98ced65bb491d382041db86b2f2 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Mon, 28 Oct 2024 17:08:53 +0000 Subject: [PATCH 4/8] Some housekeeping. --- scripts/suite_objects.py | 24 +++++++++--------------- test/unit_tests/test_var_transforms.py | 4 ++-- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 639ee7d5..3e2ad6a3 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -163,7 +163,7 @@ def call_string(self, cldicts=None, is_func_call=False, subname=None, sub_lname_ # If so, change Var's local_name need to local dummy array containing # transformed argument, var_trans_local. if sub_lname_list: - for (sname, var_trans_local) in sub_lname_list: + for (var_trans_local, var_lname, sname, rindices, lindices, compat_obj, __) in sub_lname_list: if (sname == stdname): lname = var_trans_local # end if @@ -1115,8 +1115,6 @@ def __init__(self, scheme_xml, context, parent, run_env): self.__var_debug_checks = list() self.__forward_transforms = list() self.__reverse_transforms = 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) @@ -1675,10 +1673,9 @@ def add_var_transform(self, var, compat_obj, vert_dim): compat_obj.v1_stdname)) self.__reverse_transforms.append([local_trans_var.get_prop_value('local_name'), var.get_prop_value('local_name'), + var.get_prop_value('standard_name'), rindices, lindices, compat_obj, var.get_prop_value('standard_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'): @@ -1688,10 +1685,9 @@ def add_var_transform(self, var, compat_obj, vert_dim): compat_obj.v1_stdname, compat_obj.v2_stdname)) self.__forward_transforms.append([var.get_prop_value('local_name'), + var.get_prop_value('standard_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): @@ -1735,7 +1731,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.__reverse_sub_lname_list) + sub_lname_list = self.__reverse_transforms) # outfile.write('', indent) outfile.write('if ({} == 0) then'.format(errcode), indent) @@ -1763,14 +1759,13 @@ 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 rcnt, (dummy, var, rindices, lindices, compat_obj, __) in enumerate(self.__reverse_transforms): + for rcnt, (dummy, var_lname, var_sname, 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 = self.__group.call_list.find_variable(standard_name=var_sname) 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 @@ -1812,16 +1807,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 fcnt, (var, dummy, lindices, rindices, compat_obj) in enumerate(self.__forward_transforms): + for fcnt, (var_lname, var_sname, 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 = self.__group.call_list.find_variable(standard_name=var_sname) lvar_lname = lvar.get_prop_value('local_name') - tstmt = self.write_var_transform(var, dummy, rindices, lindices, compat_obj, outfile, indent+1, True) + tstmt = self.write_var_transform(lvar_lname, dummy, rindices, lindices, compat_obj, outfile, indent+1, True) # end for outfile.write('', indent) outfile.write('end if', indent) diff --git a/test/unit_tests/test_var_transforms.py b/test/unit_tests/test_var_transforms.py index 94a6c4a0..e595bc6f 100755 --- a/test/unit_tests/test_var_transforms.py +++ b/test/unit_tests/test_var_transforms.py @@ -136,7 +136,7 @@ def test_valid_unit_change(self): compat = real_scalar1.compatible(real_scalar2, self.__run_env) self.assertIsInstance(compat, VarCompatObj, msg=self.__inst_emsg.format(type(compat))) - self.assertFalse(compat) + self.assertTrue(compat) self.assertTrue(compat.compat) self.assertEqual(compat.incompat_reason, '') self.assertFalse(compat.has_kind_transforms) @@ -150,7 +150,7 @@ def test_valid_unit_change(self): compat = real_scalar1.compatible(real_scalar2, self.__run_env) self.assertIsInstance(compat, VarCompatObj, msg=self.__inst_emsg.format(type(compat))) - self.assertFalse(compat) + self.assertTrue(compat) self.assertTrue(compat.compat) self.assertEqual(compat.incompat_reason, '') self.assertFalse(compat.has_kind_transforms) From 8e443bed8dcb6e41ebc5c862f9f9602aa8d5aeb9 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Wed, 30 Oct 2024 14:47:23 +0000 Subject: [PATCH 5/8] Address reviewers comments --- scripts/suite_objects.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 3e2ad6a3..674b7144 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -163,7 +163,7 @@ def call_string(self, cldicts=None, is_func_call=False, subname=None, sub_lname_ # If so, change Var's local_name need to local dummy array containing # transformed argument, var_trans_local. if sub_lname_list: - for (var_trans_local, var_lname, sname, rindices, lindices, compat_obj, __) in sub_lname_list: + for (var_trans_local, var_lname, sname, rindices, lindices, compat_obj) in sub_lname_list: if (sname == stdname): lname = var_trans_local # end if @@ -1674,8 +1674,7 @@ def add_var_transform(self, var, compat_obj, vert_dim): self.__reverse_transforms.append([local_trans_var.get_prop_value('local_name'), var.get_prop_value('local_name'), var.get_prop_value('standard_name'), - rindices, lindices, compat_obj, - var.get_prop_value('standard_name')]) + rindices, lindices, compat_obj]) # end if # Register any forward (post-Scheme) transforms. if (var.get_prop_value('intent') != 'in'): @@ -1759,7 +1758,7 @@ 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 rcnt, (dummy, var_lname, var_sname, rindices, lindices, compat_obj, __) in enumerate(self.__reverse_transforms): + for rcnt, (dummy, var_lname, var_sname, 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 @@ -2358,7 +2357,6 @@ def write(self, outfile, host_arglist, indent, const_mod, optional_var_set = set() pointer_var_set = list() inactive_var_set = set() - local_var_set = set() for item in [self]:# + self.parts: for var in item.declarations(): lname = var.get_prop_value('local_name') From e51db48b92061afc1f9df8d2c5962d2a064d6ef4 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Wed, 30 Oct 2024 15:04:35 +0000 Subject: [PATCH 6/8] Address compat vs. equiv --- scripts/metavar.py | 2 +- scripts/var_props.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/metavar.py b/scripts/metavar.py index e84cd2b7..691fc729 100755 --- a/scripts/metavar.py +++ b/scripts/metavar.py @@ -1632,7 +1632,7 @@ def add_variable(self, newvar, run_env, exists_ok=False, gen_unique=False, # end if if cvar is not None: compat = cvar.compatible(newvar, run_env) - if compat: + if compat.compat: # Check for intent mismatch vintent = cvar.get_prop_value('intent') dintent = newvar.get_prop_value('intent') diff --git a/scripts/var_props.py b/scripts/var_props.py index 21477b4a..660fc10f 100755 --- a/scripts/var_props.py +++ b/scripts/var_props.py @@ -950,6 +950,7 @@ def __init__(self, var1_stdname, var1_type, var1_kind, var1_units, # Check units argument if var1_units != var2_units: # Try to find a set of unit conversions + self.__equiv = False self.__unit_transforms = self._get_unit_convstrs(var1_units, var2_units) # end if From 4f5a7f1478d1f1e805055e84fd8a3cb0ad4d247d Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Wed, 30 Oct 2024 15:09:06 +0000 Subject: [PATCH 7/8] Propagate equiv/compat distinction to unit tests --- test/unit_tests/test_var_transforms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit_tests/test_var_transforms.py b/test/unit_tests/test_var_transforms.py index e595bc6f..d6c9c171 100755 --- a/test/unit_tests/test_var_transforms.py +++ b/test/unit_tests/test_var_transforms.py @@ -136,7 +136,7 @@ def test_valid_unit_change(self): compat = real_scalar1.compatible(real_scalar2, self.__run_env) self.assertIsInstance(compat, VarCompatObj, msg=self.__inst_emsg.format(type(compat))) - self.assertTrue(compat) + self.assertFalse(compat.equiv) self.assertTrue(compat.compat) self.assertEqual(compat.incompat_reason, '') self.assertFalse(compat.has_kind_transforms) @@ -150,7 +150,7 @@ def test_valid_unit_change(self): compat = real_scalar1.compatible(real_scalar2, self.__run_env) self.assertIsInstance(compat, VarCompatObj, msg=self.__inst_emsg.format(type(compat))) - self.assertTrue(compat) + self.assertFalse(compat.equiv) self.assertTrue(compat.compat) self.assertEqual(compat.incompat_reason, '') self.assertFalse(compat.has_kind_transforms) From 65b6cec0078f53d597a7ff3703c75ee5d80bd5f5 Mon Sep 17 00:00:00 2001 From: Dustin Swales Date: Mon, 4 Nov 2024 11:24:29 -0700 Subject: [PATCH 8/8] Update scripts/suite_objects.py Co-authored-by: Dom Heinzeller --- scripts/suite_objects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 674b7144..3bb4b0ce 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -923,7 +923,7 @@ def match_variable(self, var, run_env): # end if # We have a match! # Are the Scheme's and Host's compatible? - # If so, create compatability object, containing any necessary + # If so, create compatibility object, containing any necessary # forward/reverse transforms to/from and . if dict_var is not None: dict_var = self.parent.find_variable(source_var=var, any_scope=True)