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

+Remove dyn_horgrid_type%Rad_Earth and ocean_grid_type%Rad_Earth #769

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

Hallberg-NOAA
Copy link
Member

Remove the unused element Rad_Earth from ocean_grid_type and dyn_horgrid_type. The dimensionally rescaled equivalent element G%Rad_Earth_L is extensively used, and it will continue to exist. G%Rad_Earth_L was introduced in November 27, 2021 as a dimensionally rescaled version of G%Rad_Earth, while the unscaled version was retained because at the time, the Rad_Earth element of the dyn_horgrid_type is also used in SIS2. However, SIS2 has not used G%Rad_Earth since December 23, 2021, so after 3 years we can now safely remove this unused element.

Any cases on other branches that might be impacted by this change will not compile. All answers are bitwise identical, but there is one less element in two transparent types.

@Hallberg-NOAA Hallberg-NOAA added the refactor Code cleanup with no changes in functionality or results label Dec 8, 2024
Copy link

@theresa-morrison theresa-morrison left a comment

Choose a reason for hiding this comment

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

Approved. I suggest we open an issue or a similar PR to SIS2 to remove Rad_Earth there as well.

@Hallberg-NOAA
Copy link
Member Author

There is no need for the equivalent PR on SIS2 because G%Rad_Earth was already removed from the SIS2 code on December 23, 2021!

  Remove the unused element Rad_Earth from ocean_grid_type and dyn_horgrid_type.
The dimensionally rescaled equivalent element G%Rad_Earth_L is extensively used,
and it will continue to exist.  G%Rad_Earth_L was introduced in November 27,
2021 as a dimensionally rescaled version of G%Rad_Earth, while the unscaled
version was retained because at the time, the Rad_Earth element of the
dyn_horgrid_type is also used in SIS2.  However, SIS2 has not used G%Rad_Earth
since December 23, 2021, so after 3 years we can now safely remove this unused
element.

  Any cases on other branches that might be impacted by this change will not
compile.  All answers are bitwise identical, but there is one less element in
two transparent types.
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 38.15%. Comparing base (14f2c97) to head (3c8dfd7).
Report is 1 commits behind head on dev/gfdl.

Files with missing lines Patch % Lines
src/core/MOM_transcribe_grid.F90 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #769      +/-   ##
============================================
- Coverage     38.15%   38.15%   -0.01%     
============================================
  Files           296      296              
  Lines         87217    87215       -2     
  Branches      16280    16280              
============================================
- Hits          33278    33277       -1     
+ Misses        47953    47952       -1     
  Partials       5986     5986              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Hallberg-NOAA
Copy link
Member Author

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26095.

@Hallberg-NOAA Hallberg-NOAA merged commit 40a59f7 into NOAA-GFDL:dev/gfdl Jan 16, 2025
12 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the remove_G_Rad_Earth branch January 19, 2025 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code cleanup with no changes in functionality or results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants