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

+Rename G%Rad_Earth in grid types #26

Closed

Conversation

Hallberg-NOAA
Copy link
Member

Renamed G%Rad_Earth to G%Rad_Earth_mks in both the ocean_grid_type and the
dyn_horgrid_type to deliberately cause any code using these variables instead of
G%Rad_Earth_L to fail to compile as a step toward eliminating the un-scaled
variable. Also modified the two places where the mct and NUOPC drivers were
using G%Rad_Earth to instead use G%Rad_Earth_L. Were it not for the fact that
there could be code that is using this variable that is not visible to the main
branch of MOM6, this variable would have been simply eliminated. All answers
are bitwise identical, but there is a deliberate change to an element in two
important transparent types.

  Renamed G%Rad_Earth to G%Rad_Earth_mks in both the ocean_grid_type and the
dyn_horgrid_type to deliberately cause any code using these variables instead of
G%Rad_Earth_L to fail to compile as a step toward eliminating the un-scaled
variable.  Also modified the two places where the mct and NUOPC drivers were
using G%Rad_Earth to instead use G%Rad_Earth_L.  Were it not for the fact that
there could be code that is using this variable that is not visible to the main
branch of MOM6, this variable would have been simply eliminated.  All answers
are bitwise identical, but there is a deliberate change to an element in two
important transparent types.
@codecov
Copy link

codecov bot commented Dec 4, 2021

Codecov Report

Merging #26 (cfb01af) into dev/gfdl (e2c81e9) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl      #26   +/-   ##
=========================================
  Coverage     29.06%   29.06%           
=========================================
  Files           240      240           
  Lines         71319    71319           
=========================================
  Hits          20731    20731           
  Misses        50588    50588           
Impacted Files Coverage Δ
src/core/MOM_grid.F90 60.89% <ø> (ø)
src/framework/MOM_dyn_horgrid.F90 32.17% <0.00%> (ø)
src/core/MOM_transcribe_grid.F90 31.32% <50.00%> (ø)
src/initialization/MOM_grid_initialize.F90 62.88% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2c81e9...cfb01af. Read the comment docs.

marshallward
marshallward previously approved these changes Dec 9, 2021
@marshallward marshallward dismissed their stale review December 9, 2021 06:09

Detected failure in SIS2

@marshallward
Copy link
Member

It looks like the removal of Rad_Earth from dyn_hor_grid is causing problems in SIS2, which presumably still has those references. Do we need a companion PR to SIS2?

@Hallberg-NOAA
Copy link
Member Author

The required SIS2 PR is now available at NOAA-GFDL/SIS2#160.

However, given the need to coordinate this change with other code (like SIS2) that is not under the same version control system as MOM6, I am going to withdraw this PR, replacing it now with one that uses G%Rad_Earth_L in the two places in the mct and nuopc couplers where it had used G%Rad_Earth, and then in several months there will be another PR that either renames G%Rad_Earth.

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