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

.testing: Use Fortran to generate tc inputs #250

Merged

Conversation

marshallward
Copy link
Member

This patch removes the build_{grid,data}.py scripts from .testing's tc4, along with the setup of the Python infrastructure used in the .testing Makefile and GitHub Actions CI.

The Python scripts have been replaced with equivalent Fortran programs which generate identical netCDF output.

A new rule (preproc) has been added to the .testing top Makefile for generating the model input files.

The netCDF compiler dependenices are configured with autoconf, currently duplicating the macros in ac/configure.ac. (NOTE: It may be possible to share these with a common macro in ac/m4.

The configure script and Makefile are currently generated from configure.ac and autoreconf. In the future, we could simply pre-generate configure and add it to the repository.

This patch was motivated by the inability to install recent netCDF-Python packages on systems with older gcc compilers, including our main production machine. We could have possibly resolved this by adding compiler configuration to pip, or perhaps reported the issue to the netCDF-python project for them to resolve. But the costs of relying on all this Python infrastructure is starting to exceed the benefits, and I would recommend we excise it from our test suite.

This patch removes the `build_{grid,data}.py` scripts from .testing's
tc4, along with the setup of the Python infrastructure used in the
.testing Makefile and GitHub Actions CI.

The Python scripts have been replaced with equivalent Fortran programs
which generate identical netCDF output.

A new rule (`preproc`) has been added to the .testing top Makefile for
generating the model input files.

The netCDF compiler dependenices are configured with autoconf, currently
duplicating the macros in `ac/configure.ac`. (NOTE: It may be possible
to share these with a common macro in ac/m4.

The configure script and Makefile are currently generated from
`configure.ac` and `autoreconf`.  In the future, we could simply
pre-generate `configure` and add it to the repository.

This patch was motivated by the inability to install recent
netCDF-Python packages on systems with older gcc compilers, including
our main production machine.  We could have possibly resolved this by
adding compiler configuration to pip, or perhaps reported the issue to
the netCDF-python project for them to resolve.  But the costs of relying
on all this Python infrastructure is starting to exceed the benefits,
and I would recommend we excise it from our test suite.
@marshallward marshallward force-pushed the test_preproc_remove_python branch from 0961ed1 to f68ce08 Compare November 20, 2022 22:57
@codecov
Copy link

codecov bot commented Nov 20, 2022

Codecov Report

Merging #250 (f68ce08) into dev/gfdl (d46de87) will decrease coverage by 0.05%.
The diff coverage is 10.86%.

@@             Coverage Diff              @@
##           dev/gfdl     #250      +/-   ##
============================================
- Coverage     37.23%   37.17%   -0.06%     
============================================
  Files           263      263              
  Lines         73060    73221     +161     
  Branches      13605    13638      +33     
============================================
+ Hits          27201    27218      +17     
- Misses        40841    40978     +137     
- Partials       5018     5025       +7     
Impacted Files Coverage Δ
...c/external/GFDL_ocean_BGC/generic_tracer_utils.F90 0.00% <0.00%> (ø)
src/ice_shelf/MOM_ice_shelf.F90 0.00% <0.00%> (ø)
src/ice_shelf/MOM_ice_shelf_dynamics.F90 0.00% <0.00%> (ø)
src/ice_shelf/MOM_ice_shelf_initialize.F90 0.00% <0.00%> (ø)
src/initialization/MOM_coord_initialization.F90 55.81% <0.00%> (-0.66%) ⬇️
src/initialization/MOM_shared_initialization.F90 29.20% <0.00%> (-0.10%) ⬇️
src/initialization/MOM_state_initialization.F90 21.50% <0.00%> (-0.15%) ⬇️
src/ocean_data_assim/MOM_oda_driver.F90 0.00% <0.00%> (ø)
...c/parameterizations/lateral/MOM_internal_tides.F90 0.00% <0.00%> (ø)
...meterizations/vertical/MOM_internal_tide_input.F90 0.00% <0.00%> (ø)
... and 8 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

I have reviewed this PR, and it looks good to me. This PR does fix our (currently broken) TC testing, and I have also verified that it works interactively.

I am willing to accept this PR, but I would prefer that it be refactored at some point to follow the MOM6 code and documentation standards.

@Hallberg-NOAA Hallberg-NOAA merged commit 2ef7ba1 into NOAA-GFDL:dev/gfdl Nov 21, 2022
@marshallward marshallward deleted the test_preproc_remove_python branch February 20, 2023 16:21
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.

2 participants