-
Notifications
You must be signed in to change notification settings - Fork 61
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
+Non-Boussinesq open boundary conditions #460
Conversation
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #460 +/- ##
============================================
- Coverage 37.84% 37.83% -0.02%
============================================
Files 270 270
Lines 78244 78336 +92
Branches 14481 14500 +19
============================================
+ Hits 29611 29636 +25
- Misses 43242 43296 +54
- Partials 5391 5404 +13
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@Hallberg-NOAA I ran an ocean_only/dumbbell/z_sub test (gaea.ncrc4.intel-classic/2022.2.1) and the run appears to be stuck in a communication barrier and is not advancing past initialization. I suspect that the barrier is occurring at the added call to pass_var in update_obc_segment_data. |
8a66adc
to
9de6ce7
Compare
MOM_interface_heights:thickness_to_dz works on a specified halo (0 default). In the subroutine call from MOM_open_boundary:update_OBC_segment_data , adding an optional "halo_size=2" gives reproducing answers in dumbbell. The additional pass_var does not appear to be necessary. |
@Hallberg-NOAA The added optiional halo_size argument is sufficient to pass dumbbell reproducibility tests on 4 cores (2x2 decomposition), and 8 cores (4x2). The additional pass_var can be safely removed. |
Using the wider halo when calculating dz may work for dumbbell, but are we sure that this is also true for every other possible case using OBCs? In particular, I think that the OBCs are used during tracer advection, and in some cases with sufficiently strong flow the full halos are used for multiple iterations to advect tracers. The halo update for dz would address that case, provided we can sort out how to ensure that it is called by all PEs when the OBCs are in use, and not just the ones with a segment that is being updated. |
@Hallberg-NOAA I created a branch with the updates we discussed. Please review and merge onto your branch if you agree with these changes https://github.com/MJHarrison-GFDL/MOM6/tree/nonBous_OBCs_patch |
Renamed OBC related variables to emphasize that they are sea surface heights, and not sea surface heights or total column mass depending on the Boussinesq approximation. Also changed the units of these renamed variables to [Z ~> m] instead of [H ~> m or kg m-2]. The renamed element of the OBC_segment_type is eta (which is now SSH). The renamed and rescaled elements of the BT_OBC_type are H_[uv] (which are now dZ_[uv]) and eta_outer_[uv] (which are now SSH_outer_[uv]). The internal variables H_[uv] and h_in in apply_velocity_OBCs are now dZ_[uv] and ssh_in. Tidal_elev in update_OBC_segment_data and cff_eta in tidal_bay_set_OBC_data were rescaled but not renamed. There is also a new vertical grid type argument to apply_velocity_OBCs for use in changing the scaling of the SSH variables. A total of 11 GV%Z_to_H or GV%H_to_Z rescaling factors were cancelled out as a result of these changes, while 16 new ones were added, but most of these will in turn be dealt with in a follow-on commit that enables the use of OBCs in non-Boussinesq mode. Because any cases that use Flather open boundary conditions with the non-Boussinesq mode issue a fatal error, all answers are bitwise identical, but there are changes to the names and units of elements in a transparent type.
Read in open boundary condition SSH and SSHamp segment data directly in rescaled units of [Z ~> m] instead of [H ~> m or kg m-2], which then has to undergo a subsequent conversion. All answers in Boussinesq mode are bitwise identical and non-Boussinesq mode does not work with OBCs yet.
Added the new subroutine find_col_avg_SpV to return the column-averaged specific volume based on the coordinate mode and the layer averaged specific volumes that are in tv%SpV_avg. All answers are bitwise identical, but there is a new publicly visible routine.
Get Flather open boundary conditions working properly in non-Boussinesq mode. This includes calculating the column-average specific volume in step_MOM_dyn_split_RK2 and passing it as a new argument to btstep. Inside of btstep, this is copied over into a wide halo array and then passed on to set_up_BT_OBC and apply_velocity_OBCs, where it is used to determine the free surface height or the vertical column extent (in [Z ~> m]) from eta for use in the Flather radiation open boundary conditions. In addition, there are several places in MOM_barotropic related to the open boundary conditions where the usual G%bathyT needed to be replaced with its wide-halo counterpart, CS%bathyT. Also, a test was added for massless OBC columns in apply_velocity_OBCs, which are then assumed to be dry rather than dividing by zero. A fatal error message that is triggered in the case of Flather open boundary conditions in non-Boussiesq mode was removed. With this change, all Boussinesq answers are bitwise identical, but non-Boussinesq cases with Flather open boundary conditions are now working and giving answers that are qualitatively similar to the Boussinesq cases.
Add the new element dZtot to the OBC_segment_type to hold the total vertical extent of the water column, and use thickness_to_dz in update_OBC_segment_data to convert the layer thicknesses to the vertical layer extents used to set dZtot. This change leads to the cancellation of 6 unit conversion factors. All answers are bitwise identical in Boussinesq mode, but they will change and become less dependent on the Boussinesq reference density in some Boussinesq cases with certain types of open boundary condition.
502d710
to
c259611
Compare
This adds logical tests to determine the global OBC data update patterns to allow for halo updates related to these updates without having the model hang up with inconsistently blocked message passing calls. This commit corrects a bug with some test cases that was introduced with the halo update after the call to thicknesses_to_dz in update_OBC_segment_data two commits previously.
c259611
to
b6319f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OBO @MJHarrison-GFDL
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/20847 ✔️ |
This sequence of 5 commits extends the MOM6 open boundary condition capabilities to work in fully non-Boussinesq mode. This includes the addition of the new subroutine find_col_avg_SpV, the addition or renaming of elements in the transparent OBC_segment_type to emphasize that they are always sea surface heights, and a change to the dimensional rescaling of variables in several user modules. With this change, test cases that worked previously only in Boussinesq mode now also work in non-Boussinesq mode, while all Boussinesq solutions are bitwise identical. The specific changes in this PR include:
Renamed OBC related variables to emphasize that they are sea surface heights,
and not sea surface heights or total column mass depending on the Boussinesq
approximation. Also changed the units of these renamed variables to [Z ~> m]
instead of [H ~> m or kg m-2].
Renamed eta (in [H]) in the OBC_segment_type to SSH (in [Z])
Renamed 4 elements of the BT_OBC_type from H_[uv] and eta_outer_[uv] (in
[H]) to dZ_[uv] and SSH_outer_[uv] (in [Z])
Replaced the internal variables H_[uv] and h_in in apply_velocity_OBCs (in
[H]) with now dZ_[uv] and ssh_in (in [Z]).
Rescaled the dimensions of tidal_elev in update_OBC_segment_data from [H ~>
m or kg m-2] to [Z ~> m]
Rescaled the dimensions of cff_eta in tidal_bay_set_OBC_data from [H ~> m
or kg m-2] to [Z ~> m]
There is a new vertical grid type argument to apply_velocity_OBCs for use in
changing the scaling of the SSH variables.
Read in open boundary condition SSH and SSHamp segment data directly in
rescaled units of [Z ~> m] instead of [H ~> m or kg m-2], which would then
have to undergo a subsequent conversion.
Added the new subroutine find_col_avg_SpV to return the column-averaged
specific volume based on the coordinate mode and the layer averaged specific
volumes that are in tv%SpV_avg.
Revised the Flather open boundary conditions to work properly in
non-Boussinesq mode. This includes calculating the column-average specific
volume in step_MOM_dyn_split_RK2 and passing it as a new argument to btstep.
Inside of btstep, this is copied over into a wide halo array and then passed
on to set_up_BT_OBC and apply_velocity_OBCs, where it is used to determine
the free surface height or the vertical column extent (in [Z ~> m]) from eta
for use in the Flather radiation open boundary conditions. In addition,
there are several places in MOM_barotropic related to the open boundary
conditions where the usual G%bathyT needed to be replaced with its wide-halo
counterpart, CS%bathyT.
A test was added for massless OBC columns in apply_velocity_OBCs, which are
then assumed to be dry rather than dividing by zero.
A fatal error message that is triggered in the case of Flather open boundary
conditions in non-Boussinesq mode was removed.
Add the new element dZtot to the OBC_segment_type to hold the total vertical
extent of the water column, and use thickness_to_dz in
update_OBC_segment_data to convert the layer thicknesses to the vertical
layer extents used to set dZtot.
A total of 17 factors of GV%H_to_Z or GV%Z_to_H were eliminated by these changes, although another 10 that are only used in Boussinesq mode were added along with 10 factors of GV%H_to_RZ that are only used in non-Boussinesq mode.
With these changes, all Boussinesq answers are bitwise identical, but non-Boussinesq cases with Flather or other open boundary conditions are now working and giving answers that are qualitatively similar to the Boussinesq cases.