Refactor MOM_barotropic for clearer OBCs#861
Merged
Hallberg-NOAA merged 4 commits intoNOAA-GFDL:dev/gfdlfrom Mar 20, 2025
Merged
Refactor MOM_barotropic for clearer OBCs#861Hallberg-NOAA merged 4 commits intoNOAA-GFDL:dev/gfdlfrom
Hallberg-NOAA merged 4 commits intoNOAA-GFDL:dev/gfdlfrom
Conversation
Hallberg-NOAA
added
the
refactor
theresa-cordero
left a comment
theresa-cordero
left a comment
There was a problem hiding this comment.
These changes appear reasonable to me and I am ready to approve. They should be tested with more OBC options to make sure there are no answer changes. @yichengt900 would you be able to test this with the regression tests for the CEFI regional configurations?
|
@theresa-cordero, this PR passed the CEFI regression test suite. |
theresa-cordero
approved these changes
Mar 19, 2025
Hallberg-NOAA
force-pushed
the
refactor_btstep_OBC
branch
from
7fce5d3 to
decbc47
Compare
marshallward
approved these changes
Mar 19, 2025
Switched the order of the application of the open boundary conditions in btstep and the accumulation of the time integrated transports and the time averaged velocities, which avoids the need to reset these integrals after the open boundary conditions have been applied. This permits the elimination of 6 2-d arrays that are not needed and the elimination of 6 arguments each to store_u_for_OBCs and store_v_for_OBCs. Also moved the code resetting the diagnostic pressure gradient force at the OBC faces to the end of btloop_update_u and btloop_update_v, thereby eliminating separate OBC-related code blocks. All answers are bitwise identical and no public interfaces are changed.
Split apply_velocity_OBCs into apply_u_velocity_OBCs and apply_v_velocity_OBCs to allow for shorter lists of arguments and more targeted routines. Also moved the calls to store_u_for_OBCs and store_v_for_OBCs to be just before the calls to btloop_update_u and btloop_update_v as a step toward possibly combining these routines. All answers are bitwise identical and no public interfaces are changed.
Moved the calculation of the barotropic transports (uhbt and vhbt) out of btloop_update_u and btloop_update_v, and moved the code storing the previous velocities in ubt_prev and vbt_prev into these same routines, thereby eliminating the tiny routines store_u_for_OBCs and store_u_for_OBCs. The code calculating ubt_trans and vbt_trans was moved to occur alongside the calculation of the transports. Also reordered the descriptions of the arguments to btloop_update_u and btloop_update_v to match the order of the arguments themselves. All answers are bitwise identical and no public interfaces are changed.
Restrict the size of 6 barotropic diagnostics to not have wide halos as this led to unnecessary copies, and avoid the use of duplicated wide-halo arrays for the calculation of uhbtav, CS%ubtav, vhbtav and CS%vbtav, thereby eliminating 4 unneeded arrays. The code setting ubt_int, uhbt_int, vbt_int and vhbt_int was moved into apply_u_velocity_OBCs and apply_v_velocity_OBCs, eliminating a pair of loops that look for where OBCs are applied, so that there are now just two such loops for each of the u- and v-velocities inside of the barotropic time stepping loop (ideally there would just be one). The now unused routines store_u_for_OBCs and store_v_for_OBCs were eliminated. All answers are bitwise identical, and no interfaces are changed.
Hallberg-NOAA
force-pushed
the
refactor_btstep_OBC
branch
from
decbc47 to
2308364
Compare
Member
Author
|
This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26793. |
This was referenced Apr 23, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 series of 4 commits refactors
btstep()and other related routines in MOM_barotropic.F90 for simpler application of open boundary conditions. Swapping of the order of certain OBC-related calculations allowed for fewer temporary arrays to be used to store previous values without changing answers. These changes include moving the transport calculations out ofbtloop_update_u()andbtloop_update_v()with a substantial shortening (from 8 lines to 5) of their argument lists and the elimination of the routinesstore_u_for_OBCs()andstore_v_for_OBCs(). What had previously been the single routineapply_velocity_OBCs()is now split intoapply_u_velocity_OBCs()andapply_v_velocity_OBCs(), in preparation for further targeting of the OBC calculations. The accumulation and posting of diagnostics was moved to the end of the barotropic time stepping loop for greater code clarity. Several diagnostic arrays are now declared with the standard memory sizes so they can be used directly without additional copies. As a result of these changes, a total of 8 diagnostic 2-d arrays are now declared with standard-sized halos, while another 14 2-d arrays were eliminated altogether.All answers are bitwise identical, and no external interfaces are changed. The commits in this PR include: