Merge branch 'develop' of https://github.com/NCAR/ccpp-framework into…

… bugfix/allow_mod_filenames

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)

scripts/ccpp_prebuild.py

@@ -13,7 +13,7 @@
 import sys
 
 # CCPP framework imports
-from common import encode_container, decode_container, decode_container_as_dict, execute
+from common import encode_container, decode_container, decode_container_as_dict
 from common import CCPP_STAGES, CCPP_INTERNAL_VARIABLES, CCPP_STATIC_API_MODULE, CCPP_INTERNAL_VARIABLE_DEFINITON_FILE
 from common import STANDARD_VARIABLE_TYPES, STANDARD_INTEGER_TYPE, CCPP_TYPE
 from common import SUITE_DEFINITION_FILENAME_PATTERN
@@ -157,9 +157,14 @@ def clean_files(config, namespace):
         os.path.join(config['static_api_dir'], static_api_file),
         config['static_api_sourcefile'],
         ]
-    # Not very pythonic, but the easiest way w/o importing another Python module
-    cmd = 'rm -vf {0}'.format(' '.join(files_to_remove))
-    execute(cmd)
+    for f in files_to_remove:
+        try:
+            os.remove(f)
+        except FileNotFoundError:
+            pass
+        except Exception as e:
+            logging.error(f"Error removing {f}: {e}")
+            success = False
     return success
 
 def get_all_suites(suites_dir):

scripts/ccpp_suite.py

@@ -774,7 +774,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:

scripts/common.py

@@ -78,35 +78,6 @@
 # Maximum number of concurrent CCPP instances per MPI task
 CCPP_NUM_INSTANCES = 200
 
-def execute(cmd, abort = True):
-    """Runs a local command in a shell. Waits for completion and
-    returns status, stdout and stderr. If abort = True, abort in
-    case an error occurs during the execution of the command."""
-
-    # Set debug to true if logging level is debug
-    debug = logging.getLogger().getEffectiveLevel() == logging.DEBUG
-
-    logging.debug('Executing "{0}"'.format(cmd))
-    p = subprocess.Popen(cmd, stdout = subprocess.PIPE,
-                         stderr = subprocess.PIPE, shell = True)
-    (stdout, stderr) = p.communicate()
-    status = p.returncode
-    if debug:
-        message = 'Execution of "{0}" returned with exit code {1}\n'.format(cmd, status)
-        message += '    stdout: "{0}"\n'.format(stdout.decode(encoding='ascii', errors='ignore').rstrip('\n'))
-        message += '    stderr: "{0}"'.format(stderr.decode(encoding='ascii', errors='ignore').rstrip('\n'))
-        logging.debug(message)
-    if not status == 0:
-        message = 'Execution of command {0} failed, exit code {1}\n'.format(cmd, status)
-        message += '    stdout: "{0}"\n'.format(stdout.decode(encoding='ascii', errors='ignore').rstrip('\n'))
-        message += '    stderr: "{0}"'.format(stderr.decode(encoding='ascii', errors='ignore').rstrip('\n'))
-        if abort:
-            raise Exception(message)
-        else:
-            logging.error(message)
-    return (status, stdout.decode(encoding='ascii', errors='ignore').rstrip('\n'),
-                    stderr.decode(encoding='ascii', errors='ignore').rstrip('\n'))
-
 def split_var_name_and_array_reference(var_name):
     """Split an expression like foo(:,a,1:ddt%ngas)
     into components foo and (:,a,1:ddt%ngas)."""

scripts/metavar.py

@@ -1677,17 +1677,40 @@ 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
+        # variable with unique name. There are two instances when new names are
+        # created:
+        # - Same <local_name> used in different DDTs.
+        # - Different <standard_name> using the same <local_name> in a Group.
+        #   During the Group analyze phase, <gen_unique> is True.
         lname = newvar.get_prop_value('local_name')
         lvar = self.find_local_name(lname)
         if lvar is not None:
+            # Check if <lvar> is part of a different DDT than <newvar>.
+            # The API uses the full variable references when calling the Group Caps,
+            # <lvar.call_string(self))> and <newvar.call_string(self)>.
+            # Within the context of a full reference, it is allowable for local_names
+            # to be the same in different data containers.
+            newvar_callstr = newvar.call_string(self)
+            lvar_callstr   = lvar.call_string(self)
+            if newvar_callstr and lvar_callstr:
+                if newvar_callstr != lvar_callstr:
+                    if not gen_unique:
+                        exists_ok = True
+                    # end if
+                # end if
+            # end if
             if gen_unique:
                 new_lname = self.new_internal_variable_name(prefix=lname)
                 newvar = newvar.clone(new_lname)
+                # Local_name needs to be the local_name for the new
+                # internal variable, otherwise multiple instances of the same
+                # local_name in the Group cap will all be overwritten with the
+                # 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)

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
 
 ###############################################################################
 
@@ -2273,7 +2272,7 @@ def manage_variable(self, newvar):
                         ParseSource(_API_SOURCE_NAME,
                                     _API_LOCAL_VAR_NAME, newvar.context),
                         self.run_env)
-        self.add_variable(local_var, self.run_env, exists_ok=True)
+        self.add_variable(local_var, self.run_env, exists_ok=True, gen_unique=True)
         # Finally, make sure all dimensions are accounted for
         emsg = self.add_variable_dimensions(local_var, _API_LOCAL_VAR_TYPES,
                                             adjust_intent=True,
@@ -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:

test/unit_tests/test_common.py

@@ -34,18 +34,6 @@ class CommonTestCase(unittest.TestCase):
 
     """Tests functionality of functions in common.py"""
 
-    def test_execute(self):
-        """Test execute() function"""
-
-        # Input for successful test: ls command on this file
-        self.assertEqual(common.execute(f"ls {TEST_FILE}"),(0,f"{TEST_FILE}",""))
-
-        # Input for failing test (no exception): exit 1 from a subshell
-        self.assertEqual(common.execute(f"(exit 1)",abort=False),(1,"",f""))
-
-        # Input for failing test (raise exception): exit 1 from a subshell
-        self.assertRaises(Exception,common.execute,f"(exit 1)",abort=True)
-
     def test_split_var_name_and_array_reference(self):
         """Test split_var_name_and_array_reference() function"""
 

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

test/var_compatibility_test/effr_diag.F90

@@ -36,9 +36,10 @@ end subroutine effr_diag_init
    !> \section arg_table_effr_diag_run  Argument Table
    !! \htmlinclude arg_table_effr_diag_run.html
    !!
-   subroutine effr_diag_run( effrr_in,  errmsg, errflg)
+   subroutine effr_diag_run( effrr_in,  scalar_var, errmsg, errflg)
 
       real(kind_phys),    intent(in)    :: effrr_in(:,:)
+      integer,            intent(in)    :: scalar_var
       character(len=512), intent(out)   :: errmsg
       integer,            intent(out)   :: errflg
       !----------------------------------------------------------------
@@ -49,6 +50,10 @@ subroutine effr_diag_run( effrr_in,  errmsg, errflg)
 
       call cmp_effr_diag(effrr_in, effrr_min, effrr_max)
 
+      if (scalar_var .ne. 380) then
+         errmsg	= 'ERROR: effr_diag_run():  scalar_var should be 380'
+         errflg	= 1
+      endif
    end subroutine effr_diag_run
 
    subroutine cmp_effr_diag(effr, effr_min, effr_max)

test/var_compatibility_test/effr_diag.meta

@@ -41,6 +41,13 @@
   kind = kind_phys
   intent = in
   top_at_one = True
+[ scalar_var ]
+  standard_name = scalar_variable_for_testing_c
+  long_name = unused scalar variable C
+  units = m
+  dimensions = ()
+  type = integer
+  intent = in
 [ errmsg ]
   standard_name = ccpp_error_message
   long_name = Error message for error handling in CCPP

test/var_compatibility_test/effr_post.F90

@@ -36,9 +36,10 @@ end subroutine effr_post_init
    !> \section arg_table_effr_post_run  Argument Table
    !! \htmlinclude arg_table_effr_post_run.html
    !!
-   subroutine effr_post_run( effrr_inout,  errmsg, errflg)
+   subroutine effr_post_run( effrr_inout,  scalar_var, errmsg, errflg)
 
       real(kind_phys),    intent(inout) :: effrr_inout(:,:)
+      real(kind_phys),    intent(in)    :: scalar_var
       character(len=512), intent(out)   :: errmsg
       integer,            intent(out)   :: errflg
       !----------------------------------------------------------------
@@ -50,6 +51,11 @@ subroutine effr_post_run( effrr_inout,  errmsg, errflg)
       ! Do some post-processing on effrr...
       effrr_inout(:,:) = effrr_inout(:,:)*1._kind_phys
 
+      if (scalar_var .ne. 1013.0) then
+         errmsg	= 'ERROR: effr_post_run():  scalar_var should be 1013.0'
+         errflg	= 1
+      endif
+
    end subroutine effr_post_run
 
  end module effr_post

test/var_compatibility_test/effr_post.meta

@@ -40,6 +40,14 @@
   type = real
   kind = kind_phys
   intent = inout
+[ scalar_var ]
+  standard_name = scalar_variable_for_testing_b
+  long_name = unused scalar variable B
+  units = m
+  dimensions = ()
+  type = real
+  kind = kind_phys
+  intent = in
 [ errmsg ]
   standard_name = ccpp_error_message
   long_name = Error message for error handling in CCPP