Skip to content
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

Initial pFUnit Integration #326

Open
wants to merge 47 commits into
base: development
Choose a base branch
from

Conversation

mwaxmonsky
Copy link
Collaborator

@mwaxmonsky mwaxmonsky commented Dec 2, 2024

Tag name (required for release branches):
Originator(s): @mwaxmonsky

Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

Describe any changes made to build system: None

Describe any changes made to the namelist: None

List any changes to the defaults for the input datasets (e.g. boundary datasets): None

List all files eliminated and why: None

List all files added and what they do:
A .github/workflows/fortran_unit_tests.yml

  • Initial workflow file to support unit testing in CI

A src/core_utils/CMakeLists.txt
A test/unit-fortran/CMakeLists.txt
A test/unit-fortran/src/core_utils/CMakeLists.txt

  • Initial unit test build infrastructure.

A src/core_utils/string_core_utils.F90
A test/unit-fortran/src/core_utils/test_string_core_utils.pf

  • Refactored string utils and unit tests without dependencies

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)
R085 test/run_unit_tests.sh test/run_python_unit_tests.sh
R100 test/unit/.coveragerc test/unit/python/.coveragerc
R100 test/unit/sample_files/atm_in_files/test_attr_in test/unit/python/sample_files/atm_in_files/test_attr_in
R100 test/unit/sample_files/atm_in_files/test_cmplx_array_atm_in test/unit/python/sample_files/atm_in_files/test_cmplx_array_atm_in
R100 test/unit/sample_files/atm_in_files/test_extra_nml_def.xml test/unit/python/sample_files/atm_in_files/test_extra_nml_def.xml
R100 test/unit/sample_files/atm_in_files/test_extra_nml_same_group.xml test/unit/python/sample_files/atm_in_files/test_extra_nml_same_group.xml
R100 test/unit/sample_files/atm_in_files/test_extra_nml_same_var.xml test/unit/python/sample_files/atm_in_files/test_extra_nml_same_var.xml
R100 test/unit/sample_files/atm_in_files/test_missing_elems.xml test/unit/python/sample_files/atm_in_files/test_missing_elems.xml
R100 test/unit/sample_files/atm_in_files/test_multi_attr_in test/unit/python/sample_files/atm_in_files/test_multi_attr_in
R100 test/unit/sample_files/atm_in_files/test_multi_xml_in test/unit/python/sample_files/atm_in_files/test_multi_xml_in
R100 test/unit/sample_files/atm_in_files/test_nl_duplicate_atm_in test/unit/python/sample_files/atm_in_files/test_nl_duplicate_atm_in
R100 test/unit/sample_files/atm_in_files/test_simple_atm_in test/unit/python/sample_files/atm_in_files/test_simple_atm_in
R100 test/unit/sample_files/atm_in_files/test_simple_nml_def.xml test/unit/python/sample_files/atm_in_files/test_simple_nml_def.xml
R100 test/unit/sample_files/atm_in_files/test_third_nml_def.xml test/unit/python/sample_files/atm_in_files/test_third_nml_def.xml
R100 test/unit/sample_files/atm_in_files/test_user_in test/unit/python/sample_files/atm_in_files/test_user_in
R100 test/unit/sample_files/atm_in_files/test_user_nl_allow_dupl_var test/unit/python/sample_files/atm_in_files/test_user_nl_allow_dupl_var
R100 test/unit/sample_files/atm_in_files/test_user_nl_bad_equals test/unit/python/sample_files/atm_in_files/test_user_nl_bad_equals
R100 test/unit/sample_files/atm_in_files/test_user_nl_dupl_var test/unit/python/sample_files/atm_in_files/test_user_nl_dupl_var
R100 test/unit/sample_files/atm_in_files/test_user_nl_no_equals test/unit/python/sample_files/atm_in_files/test_user_nl_no_equals
R100 test/unit/sample_files/atm_in_files/test_user_nl_simple test/unit/python/sample_files/atm_in_files/test_user_nl_simple
R100 test/unit/sample_files/atm_in_files/test_user_nl_undefined_var test/unit/python/sample_files/atm_in_files/test_user_nl_undefined_var
R100 test/unit/sample_files/autogen_files/two_scheme_banana.F90 test/unit/python/sample_files/autogen_files/two_scheme_banana.F90
R100 test/unit/sample_files/autogen_files/two_scheme_banana.meta test/unit/python/sample_files/autogen_files/two_scheme_banana.meta
R100 test/unit/sample_files/autogen_files/two_scheme_banana_namelist.xml test/unit/python/sample_files/autogen_files/two_scheme_banana_namelist.xml
R100 test/unit/sample_files/banana_namelist.xml test/unit/python/sample_files/banana_namelist.xml
R100 test/unit/sample_files/build_cache_files/bad_ccpp_tag_build_cache.xml test/unit/python/sample_files/build_cache_files/bad_ccpp_tag_build_cache.xml
R100 test/unit/sample_files/build_cache_files/bad_reg_tag_build_cache.xml test/unit/python/sample_files/build_cache_files/bad_reg_tag_build_cache.xml
R100 test/unit/sample_files/build_cache_files/bad_section_tag_build_cache.xml test/unit/python/sample_files/build_cache_files/bad_section_tag_build_cache.xml
R100 test/unit/sample_files/build_cache_files/example_build_cache.xml test/unit/python/sample_files/build_cache_files/example_build_cache.xml
R100 test/unit/sample_files/build_cache_files/update_ccpp_build_cache.xml test/unit/python/sample_files/build_cache_files/update_ccpp_build_cache.xml
R100 test/unit/sample_files/build_cache_files/update_init_gen_build_cache.xml test/unit/python/sample_files/build_cache_files/update_init_gen_build_cache.xml
R100 test/unit/sample_files/build_cache_files/update_reg_build_cache.xml test/unit/python/sample_files/build_cache_files/update_reg_build_cache.xml
R100 test/unit/sample_files/hist_config_files/amwg_hist_config test/unit/python/sample_files/hist_config_files/amwg_hist_config
R100 test/unit/sample_files/hist_config_files/atm_in_flat test/unit/python/sample_files/hist_config_files/atm_in_flat
R100 test/unit/sample_files/hist_config_files/atm_in_multi test/unit/python/sample_files/hist_config_files/atm_in_multi
R100 test/unit/sample_files/hist_config_files/rad_config test/unit/python/sample_files/hist_config_files/rad_config
R100 test/unit/sample_files/hist_config_files/user_nl_cam_flat test/unit/python/sample_files/hist_config_files/user_nl_cam_flat
R100 test/unit/sample_files/hist_config_files/user_nl_cam_multi test/unit/python/sample_files/hist_config_files/user_nl_cam_multi
R100 test/unit/sample_files/kumquat_namelist.xml test/unit/python/sample_files/kumquat_namelist.xml
R100 test/unit/sample_files/namelist_files/banana_namelist.F90 test/unit/python/sample_files/namelist_files/banana_namelist.F90
R100 test/unit/sample_files/namelist_files/banana_namelist.meta test/unit/python/sample_files/namelist_files/banana_namelist.meta
R100 test/unit/sample_files/namelist_files/cam_ccpp_scheme_namelists_double_def.F90 test/unit/python/sample_files/namelist_files/cam_ccpp_scheme_namelists_double_def.F90
R100 test/unit/sample_files/namelist_files/cam_ccpp_scheme_namelists_single_def.F90 test/unit/python/sample_files/namelist_files/cam_ccpp_scheme_namelists_single_def.F90
R100 test/unit/sample_files/namelist_files/kumquat_namelist.F90 test/unit/python/sample_files/namelist_files/kumquat_namelist.F90
R100 test/unit/sample_files/namelist_files/kumquat_namelist.meta test/unit/python/sample_files/namelist_files/kumquat_namelist.meta
R100 test/unit/sample_files/phys_types_dup_section.meta test/unit/python/sample_files/phys_types_dup_section.meta
R100 test/unit/sample_files/phys_types_no_table.meta test/unit/python/sample_files/phys_types_no_table.meta
R100 test/unit/sample_files/physics_types_complete.F90 test/unit/python/sample_files/physics_types_complete.F90
R100 test/unit/sample_files/physics_types_complete.meta test/unit/python/sample_files/physics_types_complete.meta
R100 test/unit/sample_files/physics_types_ddt2.F90 test/unit/python/sample_files/physics_types_ddt2.F90
R100 test/unit/sample_files/physics_types_ddt2.meta test/unit/python/sample_files/physics_types_ddt2.meta
R100 test/unit/sample_files/physics_types_ddt_array.F90 test/unit/python/sample_files/physics_types_ddt_array.F90
R100 test/unit/sample_files/physics_types_ddt_array.meta test/unit/python/sample_files/physics_types_ddt_array.meta
R100 test/unit/sample_files/physics_types_ddt_eul.F90 test/unit/python/sample_files/physics_types_ddt_eul.F90
R100 test/unit/sample_files/physics_types_ddt_eul.meta test/unit/python/sample_files/physics_types_ddt_eul.meta
R100 test/unit/sample_files/physics_types_ddt_fv.F90 test/unit/python/sample_files/physics_types_ddt_fv.F90
R100 test/unit/sample_files/physics_types_ddt_fv.meta test/unit/python/sample_files/physics_types_ddt_fv.meta
R100 test/unit/sample_files/physics_types_ddt_se.F90 test/unit/python/sample_files/physics_types_ddt_se.F90
R100 test/unit/sample_files/physics_types_ddt_se.meta test/unit/python/sample_files/physics_types_ddt_se.meta
R100 test/unit/sample_files/physics_types_parameter.F90 test/unit/python/sample_files/physics_types_parameter.F90
R100 test/unit/sample_files/physics_types_parameter.meta test/unit/python/sample_files/physics_types_parameter.meta
R100 test/unit/sample_files/physics_types_simple.F90 test/unit/python/sample_files/physics_types_simple.F90
R100 test/unit/sample_files/physics_types_simple.meta test/unit/python/sample_files/physics_types_simple.meta
R100 test/unit/sample_files/ref_pres.meta test/unit/python/sample_files/ref_pres.meta
R100 test/unit/sample_files/ref_pres_SourceMods.meta test/unit/python/sample_files/ref_pres_SourceMods.meta
R098 test/unit/sample_files/reg_bad_xml.xml test/unit/python/sample_files/reg_bad_xml.xml
R098 test/unit/sample_files/reg_good_complete.xml test/unit/python/sample_files/reg_good_complete.xml
R100 test/unit/sample_files/reg_good_ddt.xml test/unit/python/sample_files/reg_good_ddt.xml
R100 test/unit/sample_files/reg_good_ddt2.xml test/unit/python/sample_files/reg_good_ddt2.xml
R100 test/unit/sample_files/reg_good_ddt_array.xml test/unit/python/sample_files/reg_good_ddt_array.xml
R096 test/unit/sample_files/reg_good_mf.xml test/unit/python/sample_files/reg_good_mf.xml
R100 test/unit/sample_files/reg_good_simple.xml test/unit/python/sample_files/reg_good_simple.xml
R100 test/unit/sample_files/rotten_namelist.xml test/unit/python/sample_files/rotten_namelist.xml
R100 test/unit/sample_files/write_init_files/ddt2_reg.xml test/unit/python/sample_files/write_init_files/ddt2_reg.xml
R100 test/unit/sample_files/write_init_files/ddt_array_reg.xml test/unit/python/sample_files/write_init_files/ddt_array_reg.xml
R100 test/unit/sample_files/write_init_files/ddt_reg.xml test/unit/python/sample_files/write_init_files/ddt_reg.xml
R100 test/unit/sample_files/write_init_files/host_var_host.F90 test/unit/python/sample_files/write_init_files/host_var_host.F90
R100 test/unit/sample_files/write_init_files/host_var_host.meta test/unit/python/sample_files/write_init_files/host_var_host.meta
R100 test/unit/sample_files/write_init_files/host_var_reg.xml test/unit/python/sample_files/write_init_files/host_var_reg.xml
R088 test/unit/sample_files/write_init_files/mf_reg.xml test/unit/python/sample_files/write_init_files/mf_reg.xml
R100 test/unit/sample_files/write_init_files/missing_ICs_reg.xml test/unit/python/sample_files/write_init_files/missing_ICs_reg.xml
R100 test/unit/sample_files/write_init_files/no_horiz_dim_reg.xml test/unit/python/sample_files/write_init_files/no_horiz_dim_reg.xml
R100 test/unit/sample_files/write_init_files/no_req_var_reg.xml test/unit/python/sample_files/write_init_files/no_req_var_reg.xml
R100 test/unit/sample_files/write_init_files/param_reg.xml test/unit/python/sample_files/write_init_files/param_reg.xml
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_4D.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_4D.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_bvd.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_bvd.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_cnst.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_cnst.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_ddt.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_ddt2.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt2.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_ddt_array.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt_array.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_host_var.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_host_var.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_mf.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_mf.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_no_horiz.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_no_horiz.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_noreq.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_noreq.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_param.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_param.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_parameter.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_parameter.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_protect.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_protect.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_scalar.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_scalar.F90
R100 test/unit/sample_files/write_init_files/phys_vars_init_check_simple.F90 test/unit/python/sample_files/write_init_files/phys_vars_init_check_simple.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_4D.F90 test/unit/python/sample_files/write_init_files/physics_inputs_4D.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_bvd.F90 test/unit/python/sample_files/write_init_files/physics_inputs_bvd.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_cnst.F90 test/unit/python/sample_files/write_init_files/physics_inputs_cnst.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_ddt.F90 test/unit/python/sample_files/write_init_files/physics_inputs_ddt.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_ddt2.F90 test/unit/python/sample_files/write_init_files/physics_inputs_ddt2.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_ddt_array.F90 test/unit/python/sample_files/write_init_files/physics_inputs_ddt_array.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_host_var.F90 test/unit/python/sample_files/write_init_files/physics_inputs_host_var.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_mf.F90 test/unit/python/sample_files/write_init_files/physics_inputs_mf.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_no_horiz.F90 test/unit/python/sample_files/write_init_files/physics_inputs_no_horiz.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_noreq.F90 test/unit/python/sample_files/write_init_files/physics_inputs_noreq.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_param.F90 test/unit/python/sample_files/write_init_files/physics_inputs_param.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_parameter.F90 test/unit/python/sample_files/write_init_files/physics_inputs_parameter.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_protect.F90 test/unit/python/sample_files/write_init_files/physics_inputs_protect.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_scalar.F90 test/unit/python/sample_files/write_init_files/physics_inputs_scalar.F90
R100 test/unit/sample_files/write_init_files/physics_inputs_simple.F90 test/unit/python/sample_files/write_init_files/physics_inputs_simple.F90
R100 test/unit/sample_files/write_init_files/protected_reg.xml test/unit/python/sample_files/write_init_files/protected_reg.xml
R100 test/unit/sample_files/write_init_files/ref_theta.F90 test/unit/python/sample_files/write_init_files/ref_theta.F90
R100 test/unit/sample_files/write_init_files/ref_theta.meta test/unit/python/sample_files/write_init_files/ref_theta.meta
R100 test/unit/sample_files/write_init_files/ref_two.F90 test/unit/python/sample_files/write_init_files/ref_two.F90
R100 test/unit/sample_files/write_init_files/ref_two.meta test/unit/python/sample_files/write_init_files/ref_two.meta
R100 test/unit/sample_files/write_init_files/scalar_var_reg.xml test/unit/python/sample_files/write_init_files/scalar_var_reg.xml
R100 test/unit/sample_files/write_init_files/simple_build_cache_template.xml test/unit/python/sample_files/write_init_files/simple_build_cache_template.xml
R100 test/unit/sample_files/write_init_files/simple_host.F90 test/unit/python/sample_files/write_init_files/simple_host.F90
R100 test/unit/sample_files/write_init_files/simple_host.meta test/unit/python/sample_files/write_init_files/simple_host.meta
R100 test/unit/sample_files/write_init_files/simple_reg.xml test/unit/python/sample_files/write_init_files/simple_reg.xml
R100 test/unit/sample_files/write_init_files/suite_simple.xml test/unit/python/sample_files/write_init_files/suite_simple.xml
R100 test/unit/sample_files/write_init_files/temp_adjust.F90 test/unit/python/sample_files/write_init_files/temp_adjust.F90
R100 test/unit/sample_files/write_init_files/temp_adjust.meta test/unit/python/sample_files/write_init_files/temp_adjust.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_4D.F90 test/unit/python/sample_files/write_init_files/temp_adjust_4D.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_4D.meta test/unit/python/sample_files/write_init_files/temp_adjust_4D.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_bvd.F90 test/unit/python/sample_files/write_init_files/temp_adjust_bvd.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_bvd.meta test/unit/python/sample_files/write_init_files/temp_adjust_bvd.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_cnst.F90 test/unit/python/sample_files/write_init_files/temp_adjust_cnst.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_cnst.meta test/unit/python/sample_files/write_init_files/temp_adjust_cnst.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_no_horiz.F90 test/unit/python/sample_files/write_init_files/temp_adjust_no_horiz.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_no_horiz.meta test/unit/python/sample_files/write_init_files/temp_adjust_no_horiz.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_noreq.F90 test/unit/python/sample_files/write_init_files/temp_adjust_noreq.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_noreq.meta test/unit/python/sample_files/write_init_files/temp_adjust_noreq.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_param.F90 test/unit/python/sample_files/write_init_files/temp_adjust_param.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_param.meta test/unit/python/sample_files/write_init_files/temp_adjust_param.meta
R100 test/unit/sample_files/write_init_files/temp_adjust_scalar.F90 test/unit/python/sample_files/write_init_files/temp_adjust_scalar.F90
R100 test/unit/sample_files/write_init_files/temp_adjust_scalar.meta test/unit/python/sample_files/write_init_files/temp_adjust_scalar.meta
R100 test/unit/sample_files/write_init_files/theta_ddt.F90 test/unit/python/sample_files/write_init_files/theta_ddt.F90
R100 test/unit/sample_files/write_init_files/theta_ddt.meta test/unit/python/sample_files/write_init_files/theta_ddt.meta
R100 test/unit/sample_files/write_init_files/var_4D_reg.xml test/unit/python/sample_files/write_init_files/var_4D_reg.xml
R100 test/unit/sample_files/write_init_files/var_bad_vertdim.xml test/unit/python/sample_files/write_init_files/var_bad_vertdim.xml
R099 test/unit/test_atm_in_paramgen.py test/unit/python/test_atm_in_paramgen.py
R099 test/unit/test_build_cache.py test/unit/python/test_build_cache.py
R099 test/unit/test_cam_autogen.py test/unit/python/test_cam_autogen.py
R099 test/unit/test_cam_config.py test/unit/python/test_cam_config.py
R098 test/unit/test_create_readnl_files.py test/unit/python/test_create_readnl_files.py
R099 test/unit/test_hist_config.py test/unit/python/test_hist_config.py
R099 test/unit/test_registry.py test/unit/python/test_registry.py
R099 test/unit/test_write_init_files.py test/unit/python/test_write_init_files.py

  • Moved python tests and associated files from top level test directory to python specific directory to allow other language based tests.

M src/utils/string_utils.F90

  • Removed unused code (last_sig_char, lower_to_upper, upper_to_lower, last_index, and increment_string), made formatting consistent, and refactored portable string util code to core equivalent so this layer handles non-portable code and then calls core util code.

M .github/workflows/python_unit_tests.yml
M .gitignore
M cime_config/cam_autogen.py

  • Updated python test paths to new directory.

M cime_config/buildlib

  • Adding core_utils directory to list of source directories.

If there are new failures (compared to the test/existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:

derecho/gnu/aux_sima:

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:

CAM-SIMA date used for the baseline comparison tests if different than latest:

@mwaxmonsky mwaxmonsky requested a review from nusbaume December 2, 2024 15:16
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this implemented @mwaxmonsky! I did have a few change requests, but hopefully nothing that is too difficult.

.github/workflows/fortran_unit_tests.yml Show resolved Hide resolved
.github/workflows/fortran_unit_tests.yml Outdated Show resolved Hide resolved
src/core_utils/CMakeLists.txt Show resolved Hide resolved
src/core_utils/string_core_utils.F90 Outdated Show resolved Hide resolved
src/utils/string_utils.F90 Show resolved Hide resolved
src/utils/string_utils.F90 Outdated Show resolved Hide resolved
test/unit-fortran/CMakeLists.txt Outdated Show resolved Hide resolved
@mwaxmonsky mwaxmonsky requested a review from nusbaume December 5, 2024 06:56
@mwaxmonsky mwaxmonsky marked this pull request as ready for review December 16, 2024 21:01
@nusbaume nusbaume requested a review from jimmielin December 16, 2024 21:12
Copy link
Member

@jimmielin jimmielin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mwaxmonsky! This looks great. I have a couple of minor comments that don't have to hold up the PR.

  1. Could the directory restructure be noted in the main text of the PR (list of modified files?) to document the purpose of the changes to the python unit test files?
  2. I understand this is just an initial pfUnit integration. It would be nice to have a test in the future for stringify which is what's slowly replacing to_str in CAM-SIMA, but it doesn't necessarily have to be in this PR.
  3. I saw some functions were removed from string_utils.F90 so I would just make sure all regression tests build & run as expected with the removal of these unused functions.

mattldawson added a commit that referenced this pull request Dec 31, 2024
Tag name (required for release branches):
Originator(s): Matt Dawson

Description (include the issue title, and the keyword ['closes',
'fixes', 'resolves'] followed by the issue number):

Adds the calculation of solar zenith angle and Earth–Sun distance and
makes them available as CCPP standard named variables.

@nusbaume - I wasn't sure if I put the call to recalculate the orbital
properties is correct. If it isn't let me know, and I can move it.

closes #328 
Additionally:
- adds some stubbed-out dependencies needed for MUSICA, until the actual
values are available from other physics schemes or the host model
- adds some testing infrastructure needed for unit tests of this code.
Once #326 is merged in, I can update this PR to use the new unit testing
infrastructure

In draft until ESCOMP/atmospheric_physics#171 is
merged in

Describe any changes made to build system:
none

Describe any changes made to the namelist:
none

List any changes to the defaults for the input datasets (e.g. boundary
datasets):
none

List all files eliminated and why:
none

List all files added and what they do:

List all existing files that have been modified, and describe the
changes:
(Helpful git command: `git diff --name-status
development...<your_branch_name>`)

If there are new failures (compared to the
`test/existing-test-failures.txt` file),
have them OK'd by the gatekeeper, note them here, and add them to the
file.
If there are baseline differences, include the test and the reason for
the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:

derecho/gnu/aux_sima:

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:

CAM-SIMA date used for the baseline comparison tests if different than
latest:

---------

Co-authored-by: Courtney Peverley <courtneyp@ucar.edu>
Co-authored-by: Kuan-Chih Wang <kuanchihw@ucar.edu>
@mwaxmonsky
Copy link
Collaborator Author

@kuanchihwang Apologies for the delay on this but would you or @nusbaume have any examples for stringify(...) that we could test with?

@kuanchihwang
Copy link
Collaborator

@mwaxmonsky

I wrote some unit tests for stringify here. You can adapt them as your needs. Note that however, GNU Fortran >= 12 will fail the tests in test_stringify_characters. It is a compiler bug, not a bug in our code.

Or, you can leave the unit tests for stringify to me. I will try to find a workaround for the compiler bug and open another PR with those unit tests included at a later date.

@mwaxmonsky
Copy link
Collaborator Author

mwaxmonsky commented Jan 8, 2025

@kuanchihwang Thank you so much for getting these written up! I've integrated them all into the unit test harness and am getting two failures (stars here: https://github.com/ESCOMP/CAM-SIMA/pull/326/checks#step:5:165)

Could you take a look and let me know if I transposed the tests incorrectly or if this is an issue with the compiler you mentioned?

Comment on lines 158 to 159
i32arr(:) = [-huge(int32), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(int32)]
i64arr(:) = [-huge(int64), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(int64)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
i32arr(:) = [-huge(int32), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(int32)]
i64arr(:) = [-huge(int64), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(int64)]
i32arr(:) = [-huge(i32arr), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(i32arr)]
i64arr(:) = [-huge(i64arr), -1000, -100, -10, -1, 1, 10, 100, 1000, huge(i64arr)]

@mwaxmonsky
The return value of HUGE(X) is of the same type and kind as X.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other failing test (that has Mohs scale in it) is expected. It is a GNU Fortran compiler bug. Maybe you might want to disable it temporarily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @kuanchihwang! I did not realize that huge(...) needed a variable name and cannot take data types.

I also removed the Mohs scale test from the GNU test suite so it should still run if we get to using intel or flang.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuanchihwang Is there a reference to the bug in GNU >= 12 on https://gcc.gnu.org/bugzilla/ that we can reference here? Just want to make sure we document what the issue is so we can make a note of it in the PR commit message as to why that test is disabled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwaxmonsky
I searched the GCC Bugzilla, but did not find an exact match. This bug is related to passing a character array to an class(*) (i.e., unlimited polymorphic) argument. Below is a minimum reproducible example for this bug:

program reproducer
    implicit none

    character(*), parameter :: c(*) = [ character(5) :: &
        'Hello', &
        'World', &
        '12345', &
        '67890' &
    ]

    call unlimited_polymorphic_argument(c)

    write(*, *) 'PASS'
contains
    subroutine unlimited_polymorphic_argument(x)
        class(*), intent(in) :: x(:)

        integer :: i

        select type (x)
            type is (character(*))
                if (len(x) /= len(c)) then
                    write(*, *) 'FAIL: Expected ', len(c), ', but got ', len(x)
                    stop 1
                end if

                if (size(x) /= size(c)) then
                    write(*, *) 'FAIL: Expected ', size(c), ', but got ', size(x)
                    stop 2
                end if

                do i = 1, size(c)
                    if (x(i) /= c(i)) then
                        write(*, *) 'FAIL: Expected ', c(i), ', but got ', x(i)
                        stop 3
                    end if
                end do
            class default
                stop 4
        end select
    end subroutine unlimited_polymorphic_argument
end program reproducer

GNU Fortran >= 12 gives FAIL: Expected World, but got elloW, but older 7 - 11 are fine.
Intel & Nvidia Fortran compilers are fine.

@mwaxmonsky
Copy link
Collaborator Author

Thanks @mwaxmonsky! This looks great. I have a couple of minor comments that don't have to hold up the PR.

  1. Could the directory restructure be noted in the main text of the PR (list of modified files?) to document the purpose of the changes to the python unit test files?
  2. I understand this is just an initial pfUnit integration. It would be nice to have a test in the future for stringify which is what's slowly replacing to_str in CAM-SIMA, but it doesn't necessarily have to be in this PR.
  3. I saw some functions were removed from string_utils.F90 so I would just make sure all regression tests build & run as expected with the removal of these unused functions.

Sorry for the delay @jimmielin! I updated everything in the PR from your comments but if you could take a second look, since there's significantly more changes since the last review, I'd appreciate it!

Copy link
Member

@jimmielin jimmielin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mwaxmonsky!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants