diff --git a/scripts/metavar.py b/scripts/metavar.py index cafdbf9f..b64e043f 100755 --- a/scripts/metavar.py +++ b/scripts/metavar.py @@ -1863,6 +1863,12 @@ def find_local_name(self, local_name, any_scope=False): # end if return pvar + def known_local_names(self): + """Return a list of the local names in use in this dictionary. + The returned names are lowercase because local name use is + case insensitive.""" + return list(self.__local_names.keys()) + def find_error_variables(self, any_scope=False, clone_as_out=False): """Find and return a consistent set of error variables in this dictionary. @@ -2098,11 +2104,14 @@ def var_call_string(self, var, loop_vars=None): """ return var.call_string(self, loop_vars=loop_vars) - def new_internal_variable_name(self, prefix=None, max_len=63): + def new_internal_variable_name(self, prefix=None, max_len=63, + reserved=None): """Find a new local variable name for this dictionary. The new name begins with _ or with (where is this VarDictionary's name) if is None. The new variable name is kept to a maximum length of . + If is passed, it is a collection of local names (e.g., + from a related dictionary) which the new name must also avoid. """ index = 0 if prefix is None: @@ -2110,7 +2119,12 @@ def new_internal_variable_name(self, prefix=None, max_len=63): else: var_prefix = '{}'.format(prefix) # end if - varlist = [x for x in self.__local_names.keys() if var_prefix in x] + # Local names are used case insensitively, compare in lowercase + varlist = [x for x in self.__local_names.keys() + if var_prefix.lower() in x] + if reserved: + varlist.extend([x.lower() for x in reserved]) + # end if newvar = None while newvar is None: if index == 0: @@ -2122,7 +2136,7 @@ def new_internal_variable_name(self, prefix=None, max_len=63): if len(newvar) > max_len: var_prefix = var_prefix[:-1] newvar = None - elif newvar in varlist: + elif newvar.lower() in varlist: newvar = None # end if # end while diff --git a/scripts/suite_objects.py b/scripts/suite_objects.py index 3c846ed5..fd5d9bf5 100755 --- a/scripts/suite_objects.py +++ b/scripts/suite_objects.py @@ -1632,6 +1632,38 @@ def manage_variable(self, newvar): raise CCPPError(emsg) # end if + def ensure_unique_local_names(self): + """Rename any group-local variable (this Group's dictionary) whose + local name is also the local name of one of this Group's dummy + arguments (call list) or of a suite module variable. + The group subroutine declares its dummy arguments and its local + variables in the same scope, so a shared name is a Fortran error + (e.g., a constituent dummy argument that keeps the local name of the + scheme which first requested it while another scheme uses the same + local name for a scheme-supplied interstitial). A group local would + also shadow a same-named suite module variable, silently redirecting + any reference to that suite variable in this group's subroutine. + Renaming a group local is safe because generated code looks + variables up by standard name at write time and group locals are + invisible outside the group subroutine.""" + reserved = self.call_list.known_local_names() + reserved.extend(self.parent.known_local_names()) + rset = set(reserved) + for var in list(self.variable_list()): + lname = var.get_prop_value('local_name') + if lname.lower() in rset: + stdname = var.get_prop_value('standard_name') + new_lname = self.new_internal_variable_name(prefix=lname, + reserved=reserved) + self.remove_variable(stdname) + # Add directly to this Group's dictionary (bypassing the + # SuiteObject dummy-argument adjustments already applied to + # the original variable) + VarDictionary.add_variable(self, var.clone(new_lname), + self.run_env, exists_ok=True) + # end if + # end for + def analyze(self, phase, suite_vars, scheme_library, ddt_library, check_suite_state, set_suite_state): """Analyze the Group's interface to prepare for writing""" @@ -1653,6 +1685,9 @@ def analyze(self, phase, suite_vars, scheme_library, ddt_library, # end for self._phase_check_stmts = check_suite_state self._set_state = set_suite_state + # All of this Group's dummy arguments and local variables are now + # known; resolve any local name collisions between them + self.ensure_unique_local_names() if (self.run_env.logger and self.run_env.logger.isEnabledFor(logging.DEBUG)): self.run_env.logger.debug("{}".format(self)) @@ -1736,8 +1771,17 @@ def write(self, outfile, host_arglist, indent, const_mod, for var in item.declarations(): lname = var.get_prop_value('local_name') sname = var.get_prop_value('standard_name') - if (lname in subpart_allocate_vars) or (lname in subpart_optional_vars) or (lname in subpart_scalar_vars): - if subpart_allocate_vars[lname][0].compatible(var, self.run_env): + if lname in subpart_allocate_vars: + prev_var = subpart_allocate_vars[lname][0] + elif lname in subpart_optional_vars: + prev_var = subpart_optional_vars[lname][0] + elif lname in subpart_scalar_vars: + prev_var = subpart_scalar_vars[lname][0] + else: + prev_var = None + # end if + if prev_var is not None: + if prev_var.compatible(var, self.run_env): pass # We already are going to declare this variable else: errmsg = "Duplicate Group variable, {}" diff --git a/test/advection_test/CMakeLists.txt b/test/advection_test/CMakeLists.txt index 4c20835b..f21cdcc6 100644 --- a/test/advection_test/CMakeLists.txt +++ b/test/advection_test/CMakeLists.txt @@ -1,5 +1,5 @@ # Create list of SCHEME_FILES, HOST_FILES, and SUITE_FILES -set(SCHEME_FILES "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices") +set(SCHEME_FILES "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices" "cld_shadow") set(SCHEME_FILES_ERROR "cld_liq" "cld_ice" "dlc_liq") set(HOST_FILES "test_host_data" "test_host_mod") set(SUITE_FILES "cld_suite.xml") diff --git a/test/advection_test/README.md b/test/advection_test/README.md index 6dd53e9f..01743dba 100644 --- a/test/advection_test/README.md +++ b/test/advection_test/README.md @@ -8,6 +8,9 @@ Contains tests to exercise the capabilities of the constituents object, includin - Accessing and modifying a constituent tendency variable - Passing around the constituent tendency array - Dimensions are case-insensitive +- Reuse of a constituent or host variable local name by a scheme + interstitial with a different standard name (cld_shadow); the group cap + must rename its local variables to avoid duplicate declarations ## Building/Running diff --git a/test/advection_test/advection_test_reports.py b/test/advection_test/advection_test_reports.py index 4fbe8e68..16bafca8 100644 --- a/test/advection_test/advection_test_reports.py +++ b/test/advection_test/advection_test_reports.py @@ -35,7 +35,8 @@ _CCPP_FILES = _UTILITY_FILES + _HOST_FILES + _SUITE_FILES _DEPENDENCIES = [""] _PROCESS_LIST = [""] -_MODULE_LIST = ["cld_ice", "cld_liq", "const_indices", "apply_constituent_tendencies"] +_MODULE_LIST = ["cld_ice", "cld_liq", "cld_shadow", "const_indices", + "apply_constituent_tendencies"] _SUITE_LIST = ["cld_suite"] _REQUIRED_VARS_CLD = ["ccpp_error_code", "ccpp_error_message", "horizontal_loop_begin", "horizontal_loop_end", diff --git a/test/advection_test/cld_shadow.F90 b/test/advection_test/cld_shadow.F90 new file mode 100644 index 00000000..c1ac438a --- /dev/null +++ b/test/advection_test/cld_shadow.F90 @@ -0,0 +1,41 @@ +! Test parameterization whose local names collide with other identifiers +! in the group cap: is also the local name of the cloud +! ice constituent (see cld_ice) and is also the local name of the +! host model horizontal dimension. Both name a scheme-supplied +! interstitial here, so capgen must rename the group-cap locals. +! + +module cld_shadow + + use ccpp_kinds, only: kind_phys + + implicit none + private + + public :: cld_shadow_run + +contains + + !> \section arg_table_cld_shadow_run Argument Table + !! \htmlinclude arg_table_cld_shadow_run.html + !! + subroutine cld_shadow_run(ncol, timestep, cld_ice_array, ncols, & + errmsg, errflg) + + integer, intent(in) :: ncol + real(kind_phys), intent(in) :: timestep + real(kind_phys), intent(out) :: cld_ice_array(:,:) + real(kind_phys), intent(out) :: ncols(:) + character(len=512), intent(out) :: errmsg + integer, intent(out) :: errflg + !---------------------------------------------------------------- + + errmsg = '' + errflg = 0 + + cld_ice_array(:ncol,:) = timestep + ncols(:ncol) = real(ncol, kind_phys) + + end subroutine cld_shadow_run + +end module cld_shadow diff --git a/test/advection_test/cld_shadow.meta b/test/advection_test/cld_shadow.meta new file mode 100644 index 00000000..2df6d28f --- /dev/null +++ b/test/advection_test/cld_shadow.meta @@ -0,0 +1,46 @@ +[ccpp-table-properties] + name = cld_shadow + type = scheme +[ccpp-arg-table] + name = cld_shadow_run + type = scheme +[ ncol ] + standard_name = horizontal_loop_extent + type = integer + units = count + dimensions = () + intent = in +[ timestep ] + standard_name = time_step_for_physics + long_name = time step + units = s + dimensions = () + type = real | kind = kind_phys + intent = in +[ cld_ice_array ] + standard_name = cld_shadow_scratch_array + units = s + type = real | kind = kind_phys + dimensions = (horizontal_loop_extent, vertical_layer_dimension) + intent = out +[ ncols ] + standard_name = cld_shadow_column_scratch + units = count + type = real | kind = kind_phys + dimensions = (horizontal_loop_extent) + intent = out +[ 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/advection_test/cld_suite.xml b/test/advection_test/cld_suite.xml index fac613e8..d5760416 100644 --- a/test/advection_test/cld_suite.xml +++ b/test/advection_test/cld_suite.xml @@ -7,5 +7,6 @@ apply_constituent_tendencies cld_ice apply_constituent_tendencies + cld_shadow