From 2906220e3de6d6a06bec16a486947c15ea39ae29 Mon Sep 17 00:00:00 2001 From: Courtney Peverley Date: Tue, 23 Apr 2024 19:51:02 -0600 Subject: [PATCH 1/4] add script to check fortran vs metadata without a host model --- scripts/ccpp_capgen.py | 19 +++- .../offline_check_fortran_vs_metadata.py | 101 ++++++++++++++++++ test/capgen_test/temp_adjust.F90 | 6 +- 3 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 scripts/fortran_tools/offline_check_fortran_vs_metadata.py diff --git a/scripts/ccpp_capgen.py b/scripts/ccpp_capgen.py index 042f6d16..68650074 100755 --- a/scripts/ccpp_capgen.py +++ b/scripts/ccpp_capgen.py @@ -312,19 +312,30 @@ def compare_fheader_to_mheader(meta_header, fort_header, logger): # end if for mind, mvar in enumerate(mlist): lname = mvar.get_prop_value('local_name') + mname = mvar.get_prop_value('standard_name') arrayref = is_arrayspec(lname) fvar, find = find_var_in_list(lname, flist) # Check for consistency between optional variables in metadata and # optional variables in fortran. Error if optional attribute is # missing from fortran declaration. + # first check: if metadata says the variable is optional, does the fortran match? mopt = mvar.get_prop_value('optional') if find and mopt: fopt = fvar.get_prop_value('optional') if (not fopt): - errmsg = 'Missing optional attribute in fortran declaration for variable {}, in file {}' + errmsg = 'Missing "optional" attribute in fortran declaration for variable {}, for {}' errors_found = add_error(errors_found, errmsg.format(mname,title)) # end if # end if + # now check: if fortran says the variable is optional, does the metadata match? + if fvar: + fopt = fvar.get_prop_value('optional') + mopt = mvar.get_prop_value('optional') + if (fopt and not mopt): + errmsg = 'Missing "optional" metadata property for variable {}, for {}' + errors_found = add_error(errors_found, errmsg.format(mname, title)) + # end if + # end if if mind >= flen: if arrayref: # Array reference, variable not in Fortran table @@ -511,7 +522,7 @@ def parse_host_model_files(host_filenames, host_name, run_env): return host_model ############################################################################### -def parse_scheme_files(scheme_filenames, run_env): +def parse_scheme_files(scheme_filenames, run_env, known_ddts=None): ############################################################################### """ Gather information from scheme files (e.g., init, run, and finalize @@ -519,7 +530,9 @@ def parse_scheme_files(scheme_filenames, run_env): """ table_dict = {} # Duplicate check and for dependencies processing header_dict = {} # To check for duplicates - known_ddts = list() + if not known_ddts: + known_ddts = list() + # end if logger = run_env.logger for filename in scheme_filenames: logger.info('Reading CCPP schemes from {}'.format(filename)) diff --git a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py new file mode 100644 index 00000000..8e450a0d --- /dev/null +++ b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 + +""" +Recursively compare all fortran and metadata files in user-supplied directory, and report any problems +USAGE: set PYTHONPATH environment variable to the the scripts directory, then: + python offline_check_fortran_vs_metadata.py --directory (--debug) +""" + + +import sys +import os +import logging +import argparse + +# CCPP framework imports +from framework_env import CCPPFrameworkEnv +from fortran_tools import parse_fortran_file +from metadata_table import parse_metadata_file +from ccpp_capgen import find_associated_fortran_file +from ccpp_capgen import check_fortran_against_metadata, parse_scheme_files +from parse_tools import init_log, set_log_level +from parse_tools import register_fortran_ddt_name +from parse_tools import CCPPError, ParseInternalError + +_LOGGER = init_log(os.path.basename(__file__)) +_DUMMY_RUN_ENV = CCPPFrameworkEnv(_LOGGER, ndict={'host_files':'', + 'scheme_files':'', + 'suites':''}) +_CCPP_FRAMEWORK_DDT_TYPES = ["ccpp_hash_table_t", + "ccpp_hashable_t", + "ccpp_hashable_char_t", + "ccpp_constituent_prop_ptr_t"] + + + +def find_files_to_compare(directory): + metadata_files = [] + for root, _, files in os.walk(directory, topdown=True): + for name in files: + if os.path.splitext(name)[1] == '.meta': + metadata_files.append(os.path.join(root, name)) + # end if + # end for + # end for + return metadata_files + +def compare_fortran_and_metadata(scheme_directory, run_env): + ## Check for files + metadata_files = find_files_to_compare(scheme_directory) + # Pre-register base CCPP DDT types: + for ddt_name in _CCPP_FRAMEWORK_DDT_TYPES: + register_fortran_ddt_name(ddt_name) + # end for + # Perform checks + parse_scheme_files(metadata_files, run_env, known_ddts=['ccpp_constituent_prop_ptr_t']) + +def parse_command_line(arguments, description): + """Parse command-line arguments""" + parser = argparse.ArgumentParser(description=description, + formatter_class=argparse.RawTextHelpFormatter) + parser.add_argument("--directory", type=str, required=True, + metavar='top-level directory to analyze - REQUIRED', + help="""Full path to scheme directory""") + parser.add_argument("--debug", action='store_true', default=False, + help="""turn on debug mode for additional verbosity""") + pargs = parser.parse_args(arguments) + return pargs + +def _main_func(): + """Parse command line, then parse indicated host, scheme, and suite files. + Finally, generate code to allow host model to run indicated CCPP suites.""" + pargs = parse_command_line(sys.argv[1:], __doc__) + logger = _LOGGER + if pargs.debug: + set_log_level(logger, logging.DEBUG) + else: + set_log_level(logger, logging.INFO) + # end if + compare_fortran_and_metadata(pargs.directory, _DUMMY_RUN_ENV) + print('All checks passed!') + +############################################################################### + +if __name__ == "__main__": + try: + _main_func() + sys.exit(0) + except ParseInternalError as pie: + _LOGGER.exception(pie) + sys.exit(-1) + except CCPPError as ccpp_err: + if _LOGGER.getEffectiveLevel() <= logging.DEBUG: + _LOGGER.exception(ccpp_err) + else: + _LOGGER.error(ccpp_err) + # end if + sys.exit(1) + finally: + logging.shutdown() + # end try + diff --git a/test/capgen_test/temp_adjust.F90 b/test/capgen_test/temp_adjust.F90 index 5aba4c0b..a619eb80 100644 --- a/test/capgen_test/temp_adjust.F90 +++ b/test/capgen_test/temp_adjust.F90 @@ -27,7 +27,7 @@ subroutine temp_adjust_run(foo, timestep, temp_prev, temp_layer, qv, ps, & REAL(kind_phys), intent(in) :: temp_prev(:) REAL(kind_phys), intent(inout) :: temp_layer(foo) character(len=512), intent(out) :: errmsg - integer, optional, intent(out) :: errflg + integer, intent(out) :: errflg real(kind_phys), optional, intent(in) :: innie real(kind_phys), optional, intent(out) :: outie real(kind_phys), optional, intent(inout) :: optsie @@ -36,9 +36,7 @@ subroutine temp_adjust_run(foo, timestep, temp_prev, temp_layer, qv, ps, & integer :: col_index errmsg = '' - if (present(errflg)) then - errflg = 0 - end if + errflg = 0 do col_index = 1, foo temp_layer(col_index) = temp_layer(col_index) + temp_prev(col_index) From 9ce41eec83690b59a74dbf1677b254acba712086 Mon Sep 17 00:00:00 2001 From: Courtney Peverley Date: Wed, 24 Apr 2024 10:32:56 -0600 Subject: [PATCH 2/4] code cleanup; code review --- scripts/ccpp_capgen.py | 10 ++++++---- .../offline_check_fortran_vs_metadata.py | 16 ++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) mode change 100644 => 100755 scripts/fortran_tools/offline_check_fortran_vs_metadata.py diff --git a/scripts/ccpp_capgen.py b/scripts/ccpp_capgen.py index 68650074..768ec6ff 100755 --- a/scripts/ccpp_capgen.py +++ b/scripts/ccpp_capgen.py @@ -323,8 +323,9 @@ def compare_fheader_to_mheader(meta_header, fort_header, logger): if find and mopt: fopt = fvar.get_prop_value('optional') if (not fopt): - errmsg = 'Missing "optional" attribute in fortran declaration for variable {}, for {}' - errors_found = add_error(errors_found, errmsg.format(mname,title)) + errmsg = f'Missing "optional" attribute in fortran declaration for variable {mname}, ' \ + 'for {title}' + errors_found = add_error(errors_found, errmsg) # end if # end if # now check: if fortran says the variable is optional, does the metadata match? @@ -332,8 +333,9 @@ def compare_fheader_to_mheader(meta_header, fort_header, logger): fopt = fvar.get_prop_value('optional') mopt = mvar.get_prop_value('optional') if (fopt and not mopt): - errmsg = 'Missing "optional" metadata property for variable {}, for {}' - errors_found = add_error(errors_found, errmsg.format(mname, title)) + errmsg = f'Missing "optional" metadata property for variable {mname}, ' \ + 'for {title}' + errors_found = add_error(errors_found, errmsg) # end if # end if if mind >= flen: diff --git a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py old mode 100644 new mode 100755 index 8e450a0d..5a6569c3 --- a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py +++ b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py @@ -2,15 +2,19 @@ """ Recursively compare all fortran and metadata files in user-supplied directory, and report any problems -USAGE: set PYTHONPATH environment variable to the the scripts directory, then: - python offline_check_fortran_vs_metadata.py --directory (--debug) +USAGE: + ./offline_check_fortran_vs_metadata.py --directory (--debug) """ import sys import os +import glob import logging import argparse +import site +# Enable imports from parent directory +site.addsitedir(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) # CCPP framework imports from framework_env import CCPPFrameworkEnv @@ -35,12 +39,8 @@ def find_files_to_compare(directory): metadata_files = [] - for root, _, files in os.walk(directory, topdown=True): - for name in files: - if os.path.splitext(name)[1] == '.meta': - metadata_files.append(os.path.join(root, name)) - # end if - # end for + for file in glob.glob(os.path.join(directory,'**','*.meta'), recursive=True): + metadata_files.append(file) # end for return metadata_files From eee30cb5d26261683369efc0901bb29331893679 Mon Sep 17 00:00:00 2001 From: Courtney Peverley Date: Wed, 24 Apr 2024 11:39:51 -0600 Subject: [PATCH 3/4] allow for skipping of ddt check; skip ddt check from offline script --- scripts/ccpp_capgen.py | 8 ++--- .../offline_check_fortran_vs_metadata.py | 12 +------ scripts/metadata_table.py | 35 ++++++++++++------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/scripts/ccpp_capgen.py b/scripts/ccpp_capgen.py index 768ec6ff..1a381497 100755 --- a/scripts/ccpp_capgen.py +++ b/scripts/ccpp_capgen.py @@ -524,7 +524,7 @@ def parse_host_model_files(host_filenames, host_name, run_env): return host_model ############################################################################### -def parse_scheme_files(scheme_filenames, run_env, known_ddts=None): +def parse_scheme_files(scheme_filenames, run_env, skip_ddt_check=False): ############################################################################### """ Gather information from scheme files (e.g., init, run, and finalize @@ -532,14 +532,14 @@ def parse_scheme_files(scheme_filenames, run_env, known_ddts=None): """ table_dict = {} # Duplicate check and for dependencies processing header_dict = {} # To check for duplicates - if not known_ddts: - known_ddts = list() + known_ddts = list() # end if logger = run_env.logger for filename in scheme_filenames: logger.info('Reading CCPP schemes from {}'.format(filename)) # parse metadata file - mtables = parse_metadata_file(filename, known_ddts, run_env) + mtables = parse_metadata_file(filename, known_ddts, run_env, + skip_ddt_check=skip_ddt_check) fort_file = find_associated_fortran_file(filename) ftables = parse_fortran_file(fort_file, run_env) # Check Fortran against metadata (will raise an exception on error) diff --git a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py index 5a6569c3..fc15fd89 100755 --- a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py +++ b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py @@ -30,12 +30,6 @@ _DUMMY_RUN_ENV = CCPPFrameworkEnv(_LOGGER, ndict={'host_files':'', 'scheme_files':'', 'suites':''}) -_CCPP_FRAMEWORK_DDT_TYPES = ["ccpp_hash_table_t", - "ccpp_hashable_t", - "ccpp_hashable_char_t", - "ccpp_constituent_prop_ptr_t"] - - def find_files_to_compare(directory): metadata_files = [] @@ -47,12 +41,8 @@ def find_files_to_compare(directory): def compare_fortran_and_metadata(scheme_directory, run_env): ## Check for files metadata_files = find_files_to_compare(scheme_directory) - # Pre-register base CCPP DDT types: - for ddt_name in _CCPP_FRAMEWORK_DDT_TYPES: - register_fortran_ddt_name(ddt_name) - # end for # Perform checks - parse_scheme_files(metadata_files, run_env, known_ddts=['ccpp_constituent_prop_ptr_t']) + parse_scheme_files(metadata_files, run_env, skip_ddt_check=True) def parse_command_line(arguments, description): """Parse command-line arguments""" diff --git a/scripts/metadata_table.py b/scripts/metadata_table.py index 946e9782..8753505c 100755 --- a/scripts/metadata_table.py +++ b/scripts/metadata_table.py @@ -179,7 +179,7 @@ def _parse_config_line(line, context): ######################################################################## -def parse_metadata_file(filename, known_ddts, run_env): +def parse_metadata_file(filename, known_ddts, run_env, skip_ddt_check=False): """Parse and return list of parsed metadata tables""" # Read all lines of the file at once meta_tables = [] @@ -196,7 +196,8 @@ def parse_metadata_file(filename, known_ddts, run_env): while curr_line is not None: if MetadataTable.table_start(curr_line): new_table = MetadataTable(run_env, parse_object=parse_obj, - known_ddts=known_ddts) + known_ddts=known_ddts, + skip_ddt_check=skip_ddt_check) ntitle = new_table.table_name if ntitle not in table_titles: meta_tables.append(new_table) @@ -271,7 +272,8 @@ class MetadataTable(): def __init__(self, run_env, table_name_in=None, table_type_in=None, dependencies=None, relative_path=None, known_ddts=None, - var_dict=None, module=None, parse_object=None): + var_dict=None, module=None, parse_object=None, + skip_ddt_check=False): """Initialize a MetadataTable, either with a name, , and type, , or with information from a file (). if is None, and are @@ -317,7 +319,8 @@ def __init__(self, run_env, table_name_in=None, table_type_in=None, sect = MetadataSection(self.table_name, self.table_type, run_env, title=stitle, type_in=self.table_type, module=module, - var_dict=var_dict, known_ddts=known_ddts) + var_dict=var_dict, known_ddts=known_ddts, + skip_ddt_check=skip_ddt_check) self.__sections.append(sect) # end if else: @@ -342,10 +345,10 @@ def __init__(self, run_env, table_name_in=None, table_type_in=None, known_ddts = [] # end if self.__start_context = ParseContext(context=self.__pobj) - self.__init_from_file(known_ddts, self.__run_env) + self.__init_from_file(known_ddts, self.__run_env, skip_ddt_check=skip_ddt_check) # end if - def __init_from_file(self, known_ddts, run_env): + def __init_from_file(self, known_ddts, run_env, skip_ddt_check=False): """ Read the table preamble, assume the caller already figured out the first line of the header using the header_start method.""" curr_line, _ = self.__pobj.next_line() @@ -407,7 +410,8 @@ def __init_from_file(self, known_ddts, run_env): skip_rest_of_section = False section = MetadataSection(self.table_name, self.table_type, run_env, parse_object=self.__pobj, - known_ddts=known_ddts) + known_ddts=known_ddts, + skip_ddt_check=skip_ddt_check) # Some table types only allow for one associated section if ((len(self.__sections) == 1) and (self.table_type in _SINGLETON_TABLE_TYPES)): @@ -623,7 +627,7 @@ class MetadataSection(ParseSource): def __init__(self, table_name, table_type, run_env, parse_object=None, title=None, type_in=None, module=None, process_type=None, - var_dict=None, known_ddts=None): + var_dict=None, known_ddts=None, skip_ddt_check=False): """Initialize a new MetadataSection object. If is not None, initialize from the current file and location in . @@ -693,7 +697,8 @@ def __init__(self, table_name, table_type, run_env, parse_object=None, known_ddts = [] # end if self.__start_context = ParseContext(context=self.__pobj) - self.__init_from_file(table_name, table_type, known_ddts, run_env) + self.__init_from_file(table_name, table_type, known_ddts, run_env, + skip_ddt_check=skip_ddt_check) # end if # Register this header if it is a DDT if self.header_type == 'ddt': @@ -724,7 +729,7 @@ def _default_module(self): # end if return def_mod - def __init_from_file(self, table_name, table_type, known_ddts, run_env): + def __init_from_file(self, table_name, table_type, known_ddts, run_env, skip_ddt_check=False): """ Read the section preamble, assume the caller already figured out the first line of the header using the header_start method.""" start_ctx = context_string(self.__pobj) @@ -809,7 +814,8 @@ def __init_from_file(self, table_name, table_type, known_ddts, run_env): valid_lines = True self.__variables = VarDictionary(self.title, run_env) while valid_lines: - newvar, curr_line = self.parse_variable(curr_line, known_ddts) + newvar, curr_line = self.parse_variable(curr_line, known_ddts, + skip_ddt_check=skip_ddt_check) valid_lines = newvar is not None if valid_lines: if run_env.verbose: @@ -828,7 +834,7 @@ def __init_from_file(self, table_name, table_type, known_ddts, run_env): # end if # end while - def parse_variable(self, curr_line, known_ddts): + def parse_variable(self, curr_line, known_ddts, skip_ddt_check=False): """Parse a new metadata variable beginning on . The header line has the format [ ]. """ @@ -872,7 +878,10 @@ def parse_variable(self, curr_line, known_ddts): pval_str = prop[1].strip() if ((pname == 'type') and (not check_fortran_intrinsic(pval_str, error=False))): - if pval_str in known_ddts: + if skip_ddt_check or pval_str in known_ddts: + if skip_ddt_check: + register_fortran_ddt_name(pval_str) + # end if pval = pval_str pname = 'ddt_type' else: From 9e43b78522f0235a662c74d6084de72babd30817 Mon Sep 17 00:00:00 2001 From: Courtney Peverley Date: Thu, 16 May 2024 09:52:43 -0600 Subject: [PATCH 4/4] code cleanup; review requests --- scripts/ccpp_capgen.py | 6 ++---- scripts/fortran_tools/offline_check_fortran_vs_metadata.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/scripts/ccpp_capgen.py b/scripts/ccpp_capgen.py index 1a381497..f6990133 100755 --- a/scripts/ccpp_capgen.py +++ b/scripts/ccpp_capgen.py @@ -324,17 +324,16 @@ def compare_fheader_to_mheader(meta_header, fort_header, logger): fopt = fvar.get_prop_value('optional') if (not fopt): errmsg = f'Missing "optional" attribute in fortran declaration for variable {mname}, ' \ - 'for {title}' + f'for {title}' errors_found = add_error(errors_found, errmsg) # end if # end if # now check: if fortran says the variable is optional, does the metadata match? if fvar: fopt = fvar.get_prop_value('optional') - mopt = mvar.get_prop_value('optional') if (fopt and not mopt): errmsg = f'Missing "optional" metadata property for variable {mname}, ' \ - 'for {title}' + f'for {title}' errors_found = add_error(errors_found, errmsg) # end if # end if @@ -533,7 +532,6 @@ def parse_scheme_files(scheme_filenames, run_env, skip_ddt_check=False): table_dict = {} # Duplicate check and for dependencies processing header_dict = {} # To check for duplicates known_ddts = list() - # end if logger = run_env.logger for filename in scheme_filenames: logger.info('Reading CCPP schemes from {}'.format(filename)) diff --git a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py index fc15fd89..94570ec7 100755 --- a/scripts/fortran_tools/offline_check_fortran_vs_metadata.py +++ b/scripts/fortran_tools/offline_check_fortran_vs_metadata.py @@ -21,7 +21,7 @@ from fortran_tools import parse_fortran_file from metadata_table import parse_metadata_file from ccpp_capgen import find_associated_fortran_file -from ccpp_capgen import check_fortran_against_metadata, parse_scheme_files +from ccpp_capgen import parse_scheme_files from parse_tools import init_log, set_log_level from parse_tools import register_fortran_ddt_name from parse_tools import CCPPError, ParseInternalError