Ice shelf travis upgrade #1249

MJHarrison-GFDL · 2020-11-11T15:43:53Z

Ice shelf updates for Travis

Greater consistency with MOM6 coding standards
Improved modularity of ice shelf code
Updates for Travis testing

- Greater consistency with MOM6 coding standards. - Improved modularity. - Code updates for future Travis test.

codecov-io · 2020-11-11T16:01:51Z

Codecov Report

Merging #1249 (f9af211) into dev/gfdl (42a9eaf) will decrease coverage by 0.30%.
The diff coverage is 31.86%.

@@             Coverage Diff              @@
##           dev/gfdl    #1249      +/-   ##
============================================
- Coverage     46.08%   45.77%   -0.31%     
============================================
  Files           214      225      +11     
  Lines         69399    71456    +2057     
============================================
+ Hits          31984    32711     +727     
- Misses        37415    38745    +1330

Impacted Files	Coverage Δ
...g_src/external/GFDL_ocean_BGC/FMS_coupler_util.F90	`0.00% <0.00%> (ø)`
...fig_src/external/GFDL_ocean_BGC/generic_tracer.F90	`0.00% <0.00%> (ø)`
...c/external/GFDL_ocean_BGC/generic_tracer_utils.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/kdtree.f90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/ocean_da_core.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/ocean_da_types.F90	`0.00% <0.00%> (ø)`
config_src/external/ODA_hooks/write_ocean_obs.F90	`0.00% <0.00%> (ø)`
config_src/solo_driver/MESO_surface_forcing.F90	`0.00% <0.00%> (ø)`
config_src/solo_driver/user_surface_forcing.F90	`0.00% <0.00%> (ø)`
src/ALE/MOM_remapping.F90	`70.27% <0.00%> (ø)`
... and 236 more

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 6b41926...f9af211. Read the comment docs.

src/ice_shelf/MOM_ice_shelf_initialize.F90

…ers file

src/core/MOM.F90

config_src/coupled_driver/ocean_model_MOM.F90

Hallberg-NOAA · 2020-11-13T15:03:42Z

src/ice_shelf/MOM_ice_shelf_diag_mediator.F90

+  !>@{ The following are 3D and 2D axis groups defined for output.  The names indicate
+  !! the horizontal locations (B, T, Cu, or Cv), vertical locations (L, i, or 1) and
+  !! thickness categories (c, c0, or 1).
+  type(axesType) :: axesBL, axesTL, axesCuL, axesCvL


None of the axesType variables on lines 79-82 are defined or used with the ice shelves. They should be deleted, at least for now. If we add a 3-d structure to the ice shelves, new types can be added when they are being properly defined.

Hallberg-NOAA · 2020-11-13T15:12:43Z

src/ice_shelf/MOM_ice_shelf.F90

@@ -181,28 +194,28 @@ module MOM_ice_shelf

  logical :: debug                !< If true, write verbose checksums for debugging purposes
                                  !! and use reproducible sums
+  integer :: id_clock_shelf=-1 !< CPU Clock for the ice shelf code


CPU clocks are typically treated as module variables, because they are not instance specific, but instead track the time in code, and should be summed across all ensemble members. These CPU clocks are among the very few MOM6 prohibitions against the use of module variables.

Hallberg-NOAA · 2020-11-13T15:20:34Z

src/ice_shelf/MOM_ice_shelf.F90

+
+  if (CS%rotate_index) then
+     allocate(tmp2d(CS%Grid_in%isc:CS%Grid_in%iec,CS%Grid_in%jsc:CS%Grid_in%jec));tmp2d(:,:)=0.0
+  else


This line should end with ; tmp2(:,:) = 0.0 like the line above?

This allocation, call to time_interp_external, and rotation could be replaced with rotated_time_interp_external, see MOM_horizontal_regridding.F90.

src/core/MOM.F90

src/ice_shelf/MOM_ice_shelf.F90

marshallward · 2020-11-13T16:02:33Z

src/ice_shelf/MOM_ice_shelf.F90

+                                                  !! structure for the ice shelves
+  type(ocean_grid_type), pointer :: Grid_in => NULL()    !< un-rotated input grid metric
+  type(hor_index_type), pointer :: HI_in => NULL()  !< Pointer to a horizontal indexing structure for
+                                                    !! incoming data which has not been rotated.
+  type(hor_index_type), pointer :: HI => NULL()  !< Pointer to a horizontal indexing structure for
+                                                 !! incoming data which has not been rotated.
+  logical :: rotate_index = .false.   !< True if index map is rotated
+  integer :: turns                    !< The number of quarter turns for rotation testing.
+  type(ocean_grid_type), pointer :: Grid => NULL() !< Grid for the ice-shelf model


Based on the MOM_CS implementation, I have the following comments:

Grid_in should not need to be a pointer. Grid is a pointer because it could either point to Grid_in or to its own (anonymous) rotated value, but Grid_in does not have that constraint.

The ocean grid is named G in MOM, should we follow the same convention here?

HI and HI_in are probably not needed, since they would generally be stored in Grid.

Similarly, turns is stored in Grid%HI%turns so probably does not need to be added.

But if there's reasons for introducing these separately then disregard this comment.

marshallward · 2020-11-13T16:09:34Z

src/ice_shelf/MOM_ice_shelf.F90

                                                !! describe the surface state of the ocean.  The
                                                !! intent is only inout to allow for halo updates.
-  type(forcing),         intent(inout) :: fluxes !< structure containing pointers to any possible
+  type(forcing),         pointer    :: fluxes_in !< structure containing pointers to any possible


This (and other *_in) forcing inputs should probably be declared as target rather than pointer.

marshallward · 2020-11-13T20:26:18Z

This is a more general comment than the ones above, but adding arguments to initialize_ice_thickness in order to support rotation would be a different approach than the one taken in MOM.F90. The approach there would be to try and preserve initialize_ice_thickness in its original form, but also provide a rotate_ice_thickness function which transforms the fields (generated on the input index map) from G_in to G.

IMO this would be the better approach; I can try to implement this if you want.

marshallward · 2020-11-13T20:41:31Z

src/ice_shelf/MOM_ice_shelf_initialize.F90

+  if (rotate) then
+     allocate(tmp1_2d(G_in%isd:G_in%ied,G_in%jsd:G_in%jed));tmp1_2d(:,:)=0.0
+     allocate(tmp2_2d(G_in%isd:G_in%ied,G_in%jsd:G_in%jed));tmp2_2d(:,:)=0.0
+     allocate(tmp3_2d(G_in%isd:G_in%ied,G_in%jsd:G_in%jed));tmp3_2d(:,:)=0.0


allocate_rotated_array may be useful here.

Also, as mentioned in the generic comment, we may want to spin off the rotation of the ice thickness parameters into a separate rotate_ice_thickness function.

src/ice_shelf/MOM_ice_shelf.F90

Hallberg-NOAA · 2020-11-18T18:22:45Z

src/ice_shelf/MOM_ice_shelf.F90

  type(unit_scale_type), intent(in)    :: US   !< A dimensional unit scaling type
  type(ice_shelf_CS),    pointer       :: CS   !< This module's control structure.
-  type(mech_forcing),    intent(inout) :: forces !< A structure with the driving mechanical forces
+  type(mech_forcing),    target        :: forces_in !< A structure with the driving mechanical forces


There should be an intent declaration for the forces_in structure, and similarly for the fluxes_in structure above at line 213, as well as the types passed in at lines 1159 and 1160.

marshallward · 2020-11-18T22:36:05Z

I am working on cleaning up the rotation code. Will try to wrap it up quickly...

@MJHarrison-GFDL I am testing my changes against your tc4 in the old PR #1210.

MJHarrison-GFDL · 2020-11-19T15:49:34Z

Yes . #1249 does not have an active ice shelf. We were going to change answers and turn on the shelf after #1249

…

On Wed, Nov 18, 2020 at 5:36 PM Marshall Ward ***@***.***> wrote: I am working on cleaning up the rotation code. Will try to wrap it up quickly... @MJHarrison-GFDL <https://github.com/MJHarrison-GFDL> I am testing my changes against your tc4 in the old PR #1210 <https://github.com/NOAA-GFDL/MOM6/pull/1210>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/NOAA-GFDL/MOM6/pull/1249#issuecomment-730000861>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABSPZ4GP4X324GULC44QZT3SQRD6HANCNFSM4TSERO5Q> .

-- Matthew Harrison (he/him) USDOC/NOAA/GFDL/Oceans&Cryosphere

…/MOM6 into ice_shelf_travis_upgrade

marshallward · 2020-11-23T18:24:36Z

Going to resubmit? I was just about to start testing it.

MJHarrison-GFDL added 2 commits November 11, 2020 10:29

Ice shelf updates for Travis

893a06e

- Greater consistency with MOM6 coding standards. - Improved modularity. - Code updates for future Travis test.

add ice shelf diag mediator

b3620e1

Hallberg-NOAA reviewed Nov 13, 2020

View reviewed changes

src/ice_shelf/MOM_ice_shelf_initialize.F90 Show resolved Hide resolved

fix get_param call and report all ice shelf parameters to MOM_paramet…

6c522e3

…ers file

Hallberg-NOAA reviewed Nov 13, 2020

View reviewed changes

src/core/MOM.F90 Outdated Show resolved Hide resolved

Hallberg-NOAA reviewed Nov 13, 2020

View reviewed changes

config_src/coupled_driver/ocean_model_MOM.F90 Outdated Show resolved Hide resolved

minor adjustments

40849cd

Hallberg-NOAA reviewed Nov 13, 2020

View reviewed changes

marshallward reviewed Nov 13, 2020

View reviewed changes

src/core/MOM.F90 Show resolved Hide resolved

marshallward reviewed Nov 13, 2020

View reviewed changes

src/core/MOM.F90 Outdated Show resolved Hide resolved

marshallward reviewed Nov 13, 2020

View reviewed changes

src/ice_shelf/MOM_ice_shelf.F90 Outdated Show resolved Hide resolved

MJHarrison-GFDL added 3 commits November 13, 2020 10:51

clocks as module variables

5906028

Multiple mpi ranks not suppported in rotation. Remove pass_var

9cf3f54

cleanup

0f16e80

marshallward reviewed Nov 13, 2020

View reviewed changes

MJHarrison-GFDL added 3 commits November 13, 2020 13:17

cleanup rotation pointers in CS

5c690cd

line length

a4ea5be

forcing_in/fluxes_in use target attribute

04b21e0

marshallward reviewed Nov 13, 2020

View reviewed changes

src/ice_shelf/MOM_ice_shelf.F90 Show resolved Hide resolved

Hallberg-NOAA reviewed Nov 18, 2020

View reviewed changes

add intent attributes

f9af211

Merge branch 'dev/gfdl' into ice_shelf_travis_upgrade

1336397

MJHarrison-GFDL added 2 commits November 23, 2020 13:16

Needed allocation in coupled driver

68ba236

Merge branch 'ice_shelf_travis_upgrade' of github.com:MJHarrison-GFDL…

c591f99

…/MOM6 into ice_shelf_travis_upgrade

MJHarrison-GFDL closed this Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ice shelf travis upgrade #1249

Ice shelf travis upgrade #1249

MJHarrison-GFDL commented Nov 11, 2020

codecov-io commented Nov 11, 2020 •

edited

Loading

Hallberg-NOAA Nov 13, 2020

Hallberg-NOAA Nov 13, 2020

MJHarrison-GFDL Nov 13, 2020

Hallberg-NOAA Nov 13, 2020

marshallward Nov 13, 2020

marshallward Nov 13, 2020 •

edited

Loading

marshallward Nov 13, 2020

marshallward commented Nov 13, 2020 •

edited

Loading

marshallward Nov 13, 2020

Hallberg-NOAA Nov 18, 2020 •

edited

Loading

marshallward commented Nov 18, 2020

MJHarrison-GFDL commented Nov 19, 2020 via email

marshallward commented Nov 23, 2020

Ice shelf travis upgrade #1249

Ice shelf travis upgrade #1249

Conversation

MJHarrison-GFDL commented Nov 11, 2020

codecov-io commented Nov 11, 2020 • edited Loading

Codecov Report

Hallberg-NOAA Nov 13, 2020

Choose a reason for hiding this comment

Hallberg-NOAA Nov 13, 2020

Choose a reason for hiding this comment

MJHarrison-GFDL Nov 13, 2020

Choose a reason for hiding this comment

Hallberg-NOAA Nov 13, 2020

Choose a reason for hiding this comment

marshallward Nov 13, 2020

Choose a reason for hiding this comment

marshallward Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

marshallward Nov 13, 2020

Choose a reason for hiding this comment

marshallward commented Nov 13, 2020 • edited Loading

marshallward Nov 13, 2020

Choose a reason for hiding this comment

Hallberg-NOAA Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

marshallward commented Nov 18, 2020

MJHarrison-GFDL commented Nov 19, 2020 via email

marshallward commented Nov 23, 2020

codecov-io commented Nov 11, 2020 •

edited

Loading

marshallward Nov 13, 2020 •

edited

Loading

marshallward commented Nov 13, 2020 •

edited

Loading

Hallberg-NOAA Nov 18, 2020 •

edited

Loading