+(*)Added finer grained continuity interfaces #524
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hallberg-NOAA
added
bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev/gfdl #524 +/- ##
============================================
+ Coverage 37.49% 38.08% +0.58%
============================================
Files 271 270 -1
Lines 79533 77736 -1797
Branches 14816 14417 -399
============================================
- Hits 29819 29603 -216
+ Misses 44175 42660 -1515
+ Partials 5539 5473 -66 ☔ View full report in Codecov by Sentry. |
Hallberg-NOAA
force-pushed
the
continuity_interfaces
branch
from
b82a6fd to
a2115d9
Compare
|
Could this PR be broken into two parts so that the changes are in MOM_continuity_PPM are separated from the renaming of the files so the changes can be reviewed more easily? I would suggest having commits 1-8 in this PR and commit 9 as a different PR. |
I went through this PR commit by commit and it seemed an easier way to review. |
Added the new public interfaces continuity_fluxes and continuity_adjust_vel to make use of the continuity code without actually updating the layer thicknesses, and made the existing routines zonal_mass_flux and meridional_mass_flux in the continuity_PPM module public. Continuity_fluxes is overloaded to provide either the layer thickness fluxes or the vertically summed barotropic fluxes. As a part of this change, the LB arguments to meridional_mass_flux and zonal_mass_flux were made optional and moved toward the end of the list of arguments. The intent of the ocean_grid_type argument to 11 routines in the continuity_PPM module was changed from inout to in. Missing factors of por_face_area[UV] were also added to merid_face_thickness and zonal_face_thickness when the visc_rem arguments are not present or where OBCs are in use. This could change answers, but it seems very unlikely that any impacted cases are in use yet. Answers are bitwise identical all known cases, but there are 4 new public interfaces, and some bugs were fixed in cases that are not likely in use yet.
In recognition of the fact that there is only a single continuity scheme that is accessed via the MOM_continuity module, this commit refactors MOM_continuity to use 3 simple pass-through interfaces (continuity, continuity_init and continuity_stencil) to the corresponding routines from MOM_continuity_PPM, while the MOM_continuity control structure is now alias for continuity_PPM_CS and is opaque in the MOM_continuity module. As a part of these changes, the runtime parameter CONTINUITY_SCHEME was obsoleted with a warning value of "PPM". All answers are bitwise identical, and all public interfaces look the same from the outside, but there is one fewer entry in the MOM_parameter_doc.all files.
Refactored MOM_continuity_PPM to create the separate publicly visible routines zonal_edge_thickness and meridional_edge_thickness, and also renamed the internal routines zonal_face_thickness and merid_face_thickness to zonal_flux_thickness, meridional_flux_thickness and made them publicly visible as well. This commit also renames a number of internal edge thickness variables from h_L and h_R to h_S and h_N or h_W and h_E for greater clarity, since left and right are not so well defined on the grid as north, south, east and west. All answers are bitwise identical, but there are 4 new publicly visible interfaces.
Moved the calls to zonal_edge_thickness and meridional_edge_thickness out of zonal_mass_flux and meridional_mass_flux to facilitate the reuse at some later date of the PPM thickness reconstructions. As a part of this, there are new edge thickness arguments to zonal_mass_flux and meridional_mass_flux. The interfaces to zonal_edge_thickness and meridional_edge_thickness are new publicly visible and are used in MOM_continuity. This commits also changes the name of the loop_bounds_type to cont_loop_bounds_type and makes it public but opaque adds the publicly visible function set_continuity_loop_bounds to enable the continuity loop bounds to be set from outside of the continuity_PPM module. Reflecting these changes there are new calls to zonal_edge_thickness and meridional_edge_thickness in the 3 routines in MOM_continuity and in continuity_PPM, and new arrays for holding the edge thicknesses in these routines. All answers are bitwise identical, but there are new publicly visible interfaces and types and changes to other publicly visible interfaces. However, no changes are required outside of MOM_continuity and MOM_continuity_PPM.
Added the new publicly visible routines zonal_BT_mass_flux and meridional_BT_mass_flux to return the vertically summed transports that the continuity solver would generate. Also revised the routine continuity_2d_fluxes in MOM_continuity to make use of these new routines. Because these new routines are not yet being used, all answers are bitwise identical, but there are new public interfaces.
Added the new publicly visible routines continuity_zonal_convergence and continuity_meridional_convergence to increment layer thicknesses using the continuity loop bounds type to specify extents. Also revised continuity_PPM to use these new routines. These changes will allow for the reuse of some of the reconstructions in calls that replace calls to continuity with the unwrapped contents. All answers are bitwise identical, but there are two new public interfaces.
Move continuity_fluxes and continuity_adjust_vel from MOM_continuity.F90 to MOM_continuity_PPM.F90, but with these interfaces also offered via the MOM_continuity module so that no changes are required outside of these two files. In addtion, 11 of the recently added public interfaces from MOM_continuity_PPM are also made available as pass-through interfaces from MOM_continuity. All answers are bitwise identical.
Hallberg-NOAA
force-pushed
the
continuity_interfaces
branch
from
a2115d9 to
d2b9a37
Compare
|
I have revised this PR to avoid the final step of combining the continuity.F90 and continuity_PPM.F90 files to facilitate a more straightforward review of this PR via GitHub, which had been doing a diff on the wrong files. |
Hallberg-NOAA
force-pushed
the
continuity_interfaces
branch
from
d2b9a37 to
475934f
Compare
|
I have now revised this PR into the final form that will work with the new split_RK2b time-stepping option, which is now working as intended and will follow soon in a separate PR. |
Added the new optional arguments du_cor and dv_cor to continuity_PPM and zonal_mass_flux or meridional_mass_flux to return the barotropic velocity increments that make the summed barotropic transports match uhbt or vhbt. Also cleaned up and simplified the logic of some of the flags used to apply specified open boundary conditions, adding a new 1-d logical array thereby avoiding working unnecessarily on some loops or repeatedly checking for specified open boundary condition points. Two openMP directives were also simplified. All answers are bitwise identical, but there are new optional arguments to three publicly visible routines.
Hallberg-NOAA
force-pushed
the
continuity_interfaces
branch
from
475934f to
11cd707
Compare
theresa-morrison
approved these changes
Dec 13, 2023
marshallward
approved these changes
Dec 14, 2023
This was referenced May 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request consists of a series of eight commits that uses the MOM_continuity module as a simple pass-through with some renaming for the MOM_continuity_PPM module and adds a total of 14 new publicly visible routines or types so that the various sub-steps within the continuity update can be called from outside. These changes will provide greater flexibility as we reconsider the coupling between the baroclinic and barotropic modes, and perhaps allow for greater reuse of intermediate steps in the calculations (e.g., the PPM reconstructions) for computational efficiency.
The new publicly visible routines include:
continuity_fluxes(),continuity_adjust_vel(),zonal_mass_flux(),meridional_mass_flux(),zonal_edge_thickness(),meridional_edge_thickness(),continuity_zonal_convergence(),continuity_meridional_convergence(),zonal_flux_thickness(),meridional_flux_thickness(),zonal_BT_mass_flux(),meridional_BT_mass_flux()andset_continuity_loop_bounds(). In addition thecont_loop_bounds_typeis now also publicly visible, although it remains opaque. The intent of the ocean_grid_type argument to 11 routines in the continuity module was changed from inout to in, and some of the argument orders in the previously private routines were alteredto allow for the introduction of a few optional arguments now that they are publicly visible.
In addition some missing factors of
por_face_area[UV]were added tomerid_face_thickness()andzonal_face_thickness()when thevisc_remarguments are not present or where OBCs are in use. This could change answers, but it seems very unlikely that any impacted cases are in use yet.The new optional arguments
du_coranddv_corwere also added tocontinuity_PPM()andzonal_mass_flux()ormeridional_mass_flux()to return the barotropic velocity increments that will make the summed layer transports matchuhbtorvhbt.All answers in any known test case are bitwise identical. All previous public interfaces to MOM_continuity are preserved, but there are multiple new publicly visible routines and new optional arguments to 3 routines. The commits in this PR include: