-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup, adding unit tests for ccpp_track_variables.py
, activate "chunked data" CI test, add missing MPI target in src/CMakeLists.txt
#555
Changes from 11 commits
df36dd2
0e0bfb3
5ec9f95
67dd0b6
68b9003
1a0a354
e382e6b
830f796
c615827
55972ff
0c65498
9b94679
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,6 @@ def parse_suite(sdf, run_env): | |
the ordered list of schemes for the suite specified by the provided sdf""" | ||
run_env.logger.info(f'Reading sdf {sdf} and populating Suite object') | ||
suite = Suite(sdf_name=sdf) | ||
run_env.logger.info(f'Reading sdf {sdf} and populating Suite object') | ||
success = suite.parse(make_call_tree=True) | ||
if not success: | ||
raise Exception(f'Parsing suite definition file {sdf} failed.') | ||
|
@@ -100,9 +99,6 @@ def create_var_graph(suite, var, config, metapath, run_env): | |
run_env.logger.debug(f"reading .meta files in path:\n {metapath}") | ||
metadata_dict=create_metadata_filename_dict(metapath) | ||
|
||
run_env.logger.debug(f"reading metadata files for schemes defined in config file: " | ||
f"{config['scheme_files']}") | ||
|
||
# Loop through call tree, find matching filename for scheme via dictionary schemes_in_files, | ||
# then parse that metadata file to find variable info | ||
partial_matches = {} | ||
|
@@ -170,20 +166,29 @@ def create_var_graph(suite, var, config, metapath, run_env): | |
|
||
return (success,var_graph) | ||
|
||
def main(): | ||
def track_variables(sdf,metadata_path,config,variable,debug): | ||
"""Main routine that traverses a CCPP suite and outputs the list of schemes that use given | ||
variable, broken down by group""" | ||
variable, broken down by group | ||
|
||
args = parse_arguments() | ||
Args: | ||
sdf (str) : The full path of the suite definition file to parse | ||
metadata_path (str) : path to CCPP scheme metadata files | ||
config (str) : path to CCPP prebuild configuration file | ||
variable (str) : variable to track through CCPP suite | ||
debug (bool) : Enable extra output for debugging | ||
|
||
Returns: | ||
None | ||
""" | ||
|
||
logger = setup_logging(args.debug) | ||
logger = setup_logging(debug) | ||
|
||
#Use new capgen class CCPPFrameworkEnv | ||
run_env = CCPPFrameworkEnv(logger, host_files="", scheme_files="", suites="") | ||
|
||
suite = parse_suite(args.sdf,run_env) | ||
suite = parse_suite(sdf,run_env) | ||
|
||
(success, config) = import_config(args.config, None) | ||
(success, config) = import_config(config, None) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You also do not need the tuple to unpack the variables. Same questions as above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it doesn't hurt, or? If so, then does it matter? |
||
if not success: | ||
raise Exception('Call to import_config failed.') | ||
|
||
|
@@ -194,10 +199,10 @@ def main(): | |
if not success: | ||
raise Exception('Call to gather_variable_definitions failed.') | ||
|
||
(success, var_graph) = create_var_graph(suite, args.variable, config, args.metadata_path, run_env) | ||
(success, var_graph) = create_var_graph(suite, variable, config, metadata_path, run_env) | ||
if success: | ||
print(f"For suite {suite.sdf_name}, the following schemes (in order for each group) " | ||
f"use the variable {args.variable}:") | ||
f"use the variable {variable}:") | ||
for group in var_graph: | ||
if var_graph[group]: | ||
print(f"In group {group}") | ||
|
@@ -206,4 +211,7 @@ def main(): | |
|
||
|
||
if __name__ == '__main__': | ||
main() | ||
|
||
args = parse_arguments() | ||
|
||
track_variables(args.sdf,args.metadata_path,args.config,args.variable,args.debug) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
#! /usr/bin/env python3 | ||
""" | ||
----------------------------------------------------------------------- | ||
Description: Contains unit tests for ccpp_track_variables.py script | ||
|
||
Assumptions: Assumes user has correct environment for running ccpp_track_variables.py script. | ||
This script should not be run directly, but rather invoked with pytest. | ||
|
||
Command line arguments: none | ||
|
||
Usage: pytest test_track_variables.py # run the unit tests | ||
----------------------------------------------------------------------- | ||
""" | ||
import sys | ||
import os | ||
import pytest | ||
|
||
TEST_DIR = os.path.dirname(os.path.abspath(__file__)) | ||
SCRIPTS_DIR = os.path.abspath(os.path.join(TEST_DIR, os.pardir, "scripts")) | ||
SAMPLE_FILES_DIR = "test_track_variables" | ||
SUITE_FILE = f'{SAMPLE_FILES_DIR}/suite_TEST_SUITE.xml' | ||
SMALL_SUITE_FILE = f'{SAMPLE_FILES_DIR}/suite_small_suite.xml' | ||
CONFIG_FILE = f'{SAMPLE_FILES_DIR}/ccpp_prebuild_config.py' | ||
if not os.path.exists(SCRIPTS_DIR): | ||
raise ImportError(f"Cannot find scripts directory {SCRIPTS_DIR}") | ||
|
||
sys.path.append(SCRIPTS_DIR) | ||
|
||
from ccpp_track_variables import track_variables | ||
|
||
def test_successful_match(capsys): | ||
"""Tests whether test_track_variables.py produces expected output from sample suite and | ||
metadata files for a case with a successful match (user provided a variable that exists | ||
within the schemes specified by the test suite)""" | ||
expected_output = """For suite test_track_variables/suite_small_suite.xml, the following schemes (in order for each group) use the variable air_pressure: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A doc string explaining the purpose of the test (what is being tested, test limits, etc.) would be helpful here and for all the test functions below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed |
||
In group group1 | ||
scheme_1_run (intent in) | ||
scheme_1_run (intent in)""" | ||
track_variables(SMALL_SUITE_FILE,SAMPLE_FILES_DIR,CONFIG_FILE,'air_pressure',False) | ||
streams = capsys.readouterr() | ||
expected_output_list = expected_output.splitlines() | ||
streams_err_list = streams.err.splitlines() | ||
for (err, expected) in zip(streams_err_list, expected_output_list): | ||
assert err.strip() == expected.strip() | ||
|
||
def test_successful_match_with_subcycles(capsys): | ||
"""Tests whether test_track_variables.py produces expected output from sample suite and | ||
metadata files for a case with a successful match (user provided a variable that exists | ||
within the schemes specified by the test suite). In this case, the test suite file | ||
contains subcycles, so the output should reflect this.""" | ||
|
||
expected_output = """For suite test_track_variables/suite_TEST_SUITE.xml, the following schemes (in order for each group) use the variable surface_air_pressure: | ||
In group group1 | ||
scheme_3_run (intent inout) | ||
scheme_3_timestep_finalize (intent inout) | ||
scheme_3_timestep_finalize (intent out) | ||
scheme_4_run (intent in) | ||
scheme_3_run (intent inout) | ||
scheme_3_timestep_finalize (intent inout) | ||
scheme_3_timestep_finalize (intent out) | ||
scheme_4_run (intent in) | ||
In group group2 | ||
scheme_4_run (intent in) | ||
scheme_4_run (intent in) | ||
scheme_4_run (intent in)""" | ||
track_variables(SUITE_FILE,SAMPLE_FILES_DIR,CONFIG_FILE,'surface_air_pressure',False) | ||
streams = capsys.readouterr() | ||
expected_output_list = expected_output.splitlines() | ||
streams_err_list = streams.err.splitlines() | ||
for (err, expected) in zip(streams_err_list, expected_output_list): | ||
assert err.strip() == expected.strip() | ||
|
||
|
||
def test_partial_match(capsys): | ||
"""Tests whether test_track_variables.py produces expected output from sample suite and | ||
metadata files for a case with a partial match: user provided a variable that does not | ||
exist in the test suite, but is a substring of one or more other variables that do | ||
exist.""" | ||
|
||
expected_output = """Variable surface not found in any suites for sdf test_track_variables/suite_TEST_SUITE.xml | ||
|
||
ERROR:ccpp_track_variables:Variable surface not found in any suites for sdf test_track_variables/suite_TEST_SUITE.xml | ||
|
||
Did find partial matches that may be of interest: | ||
|
||
In scheme_2_init found variable(s) ['surface_emissivity_data_file'] | ||
In scheme_2_run found variable(s) ['surface_roughness_length', 'surface_ground_temperature_for_radiation', 'surface_air_temperature_for_radiation', 'surface_skin_temperature_over_ice', 'baseline_surface_longwave_emissivity', 'surface_longwave_emissivity', 'surface_albedo_components', 'surface_albedo_for_diffused_shortwave_on_radiation_timestep'] | ||
In scheme_3_init found variable(s) ['flag_for_mellor_yamada_nakanishi_niino_surface_layer_scheme'] | ||
In scheme_3_timestep_init found variable(s) ['flag_for_mellor_yamada_nakanishi_niino_surface_layer_scheme'] | ||
In scheme_3_run found variable(s) ['do_compute_surface_scalar_fluxes', 'do_compute_surface_diagnostics', 'surface_air_pressure', 'reference_air_pressure_normalized_by_surface_air_pressure'] | ||
In scheme_3_timestep_finalize found variable(s) ['surface_air_pressure'] | ||
In scheme_4_run found variable(s) ['surface_air_pressure'] | ||
In scheme_B_run found variable(s) ['flag_nonzero_wet_surface_fraction', 'sea_surface_temperature', 'surface_skin_temperature_after_iteration_over_water'] | ||
""" | ||
track_variables(SUITE_FILE,SAMPLE_FILES_DIR,CONFIG_FILE,'surface',False) | ||
streams = capsys.readouterr() | ||
expected_output_list = expected_output.splitlines() | ||
streams_err_list = streams.err.splitlines() | ||
for (err, expected) in zip(streams_err_list, expected_output_list): | ||
assert err.strip() == expected.strip() | ||
|
||
|
||
def test_no_match(capsys): | ||
"""Tests whether test_track_variables.py produces expected output from sample suite and | ||
metadata files for a case with no match (user provided a variable that does not exist | ||
within the schemes specified by the test suite)""" | ||
|
||
expected_output = """Variable abc not found in any suites for sdf test_track_variables/suite_TEST_SUITE.xml | ||
|
||
ERROR:ccpp_track_variables:Variable abc not found in any suites for sdf test_track_variables/suite_TEST_SUITE.xml""" | ||
track_variables(SUITE_FILE,SAMPLE_FILES_DIR,CONFIG_FILE,'abc',False) | ||
streams = capsys.readouterr() | ||
expected_output_list = expected_output.splitlines() | ||
streams_err_list = streams.err.splitlines() | ||
for (err, expected) in zip(streams_err_list, expected_output_list): | ||
assert err.strip() == expected.strip() | ||
|
||
|
||
def test_bad_config(capsys): | ||
"""Tests whether test_track_variables.py fails gracefully when provided a config file that does | ||
not exist.""" | ||
with pytest.raises(Exception) as excinfo: | ||
track_variables(SUITE_FILE,SAMPLE_FILES_DIR,f'{SAMPLE_FILES_DIR}/nofile','abc',False) | ||
assert str(excinfo.value) == "Call to import_config failed." | ||
|
||
|
||
if __name__ == "__main__": | ||
print("This test file is designed to be run with pytest; can not be run directly") | ||
sys.exit(1) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
#!/usr/bin/env python | ||
|
||
# CCPP prebuild config for CCPP track variables tool test | ||
|
||
|
||
############################################################################### | ||
# Definitions # | ||
############################################################################### | ||
|
||
HOST_MODEL_IDENTIFIER = "TEST" | ||
|
||
# Add all files with metadata tables on the host model side and in CCPP, | ||
# relative to basedir = top-level directory of host model. This includes | ||
# kind and type definitions used in CCPP physics. Also add any internal | ||
# dependencies of these files to the list. | ||
VARIABLE_DEFINITION_FILES = [ | ||
# actual variable definition files | ||
'../src/ccpp_types.F90', | ||
] | ||
|
||
# Add all physics scheme files relative to basedir | ||
SCHEME_FILES = [ ] | ||
|
||
# Default build dir, relative to current working directory, | ||
# if not specified as command-line argument | ||
DEFAULT_BUILD_DIR = 'build' | ||
|
||
# Auto-generated makefile/cmakefile snippets that contain all type definitions | ||
TYPEDEFS_MAKEFILE = '{build_dir}/CCPP_TYPEDEFS.mk' | ||
TYPEDEFS_CMAKEFILE = '{build_dir}/CCPP_TYPEDEFS.cmake' | ||
TYPEDEFS_SOURCEFILE = '{build_dir}/CCPP_TYPEDEFS.sh' | ||
|
||
# Auto-generated makefile/cmakefile snippets that contain all schemes | ||
SCHEMES_MAKEFILE = '{build_dir}/CCPP_SCHEMES.mk' | ||
SCHEMES_CMAKEFILE = '{build_dir}/CCPP_SCHEMES.cmake' | ||
SCHEMES_SOURCEFILE = '{build_dir}/CCPP_SCHEMES.sh' | ||
|
||
# Auto-generated makefile/cmakefile snippets that contain all caps | ||
CAPS_MAKEFILE = '{build_dir}/CCPP_CAPS.mk' | ||
CAPS_CMAKEFILE = '{build_dir}/CCPP_CAPS.cmake' | ||
CAPS_SOURCEFILE = '{build_dir}/CCPP_CAPS.sh' | ||
|
||
# Directory where to put all auto-generated physics caps | ||
CAPS_DIR = '{build_dir}' | ||
|
||
# Directory where the suite definition files are stored | ||
SUITES_DIR = '.' | ||
|
||
# Optional arguments - only required for schemes that use | ||
# optional arguments. ccpp_prebuild.py will throw an exception | ||
# if it encounters a scheme subroutine with optional arguments | ||
# if no entry is made here. Possible values are: 'all', 'none', | ||
# or a list of standard_names: [ 'var1', 'var3' ]. | ||
OPTIONAL_ARGUMENTS = {} | ||
|
||
# Directory where to write static API to | ||
STATIC_API_DIR = '{build_dir}' | ||
STATIC_API_CMAKEFILE = '{build_dir}/CCPP_API.cmake' | ||
STATIC_API_SOURCEFILE = '{build_dir}/CCPP_API.sh' | ||
|
||
# Directory for writing HTML pages generated from metadata files | ||
METADATA_HTML_OUTPUT_DIR = '{build_dir}' | ||
|
||
# HTML document containing the model-defined CCPP variables | ||
HTML_VARTABLE_FILE = '{build_dir}/CCPP_VARIABLES_BLOCKED_DATA.html' | ||
|
||
# LaTeX document containing the provided vs requested CCPP variables | ||
LATEX_VARTABLE_FILE = '{build_dir}/CCPP_VARIABLES_BLOCKED_DATA.tex' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
[ccpp-table-properties] | ||
name = scheme_1 | ||
type = scheme | ||
dependencies = ../../hooks/machine.F | ||
|
||
######################################################################## | ||
[ccpp-arg-table] | ||
name = scheme_1_run | ||
type = scheme | ||
[p_lay] | ||
standard_name = air_pressure | ||
long_name = mean layer pressure | ||
units = Pa | ||
dimensions = (horizontal_loop_extent,vertical_layer_dimension) | ||
type = real | ||
kind = kind_phys | ||
intent = in | ||
[p_lev] | ||
standard_name = air_pressure_at_interface | ||
long_name = air pressure at model layer interfaces | ||
units = Pa | ||
dimensions = (horizontal_loop_extent,vertical_interface_dimension) | ||
type = real | ||
kind = kind_phys | ||
intent = in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the tuple? Doesn't Python do that anyway? Is that a style thing and if so, do we want to be consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gold2718 Sorry, I'm not sure what you mean. This function as called from main() expects two return values, and so currently when this block of code is executed (when there is no configuration file) it fails with a TypeError because it is not returning the expected values.
This change fixes the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the lack of clarity. All I was saying is that the parentheses are not needed so I was wondering why you use them. The same goes below where you call this function.
Is this a style thing we want to adopt? A quick look shows that the prebuild code seems to consistently use the parentheses syntax while the capgen code is . . . . mixed (I'm sure for good reasons :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. I did not realize this until you brought this up, but parentheses are not strictly necessary for tuples, so this would be purely stylistic. Since the existing return tuples use parens I'll stick with that for now, especially since PEP8 doesn't seem to specify a preference from what I can tell.