Update mom6 to its main repo. 20260828 commit - #183
Merged
sanAkel merged 200 commits intoSep 8, 2026
Conversation
Fix the 3-equation iteration for the buoyancy flux between the ocean and an overlying ice-shelf when ICE_SHELF_BUOYANCY_FLUX_ITT_BUGFIX is true and SHELF_3EQ_GAMMA it false. This code now uses proper bounding of the self-consistent solution, avoiding further amplifying the fluxes in the cases when the differences between the diffusivities of heat and salt to make the buoyancy flux destabilizing for finite turbulent mixing. Both the false-position iterations and the (appropriately chosen) Newton's method iterations have been extensively examined and determined to be working correctly via print statements that have subsequently been removed for efficiency. Previously, the code to determine the 3-equation solution for the buoyancy flux between the ocean and an ice shelf had been skipping iteration altogether or doing un-bounded Newton's method iterations with a sign error in part of the derivative, including taking the square root of negative numbers, leading to the issue described at NOAA-GFDL#945. That issue has now been corrected and can be closed once this commit has been merged into the dev/gfdl branch of MOM6. This commit also changes the names of the runtime parameters to correct the ice shelf flux iteration bugs from ICE_SHELF_BUOYANCY_FLUX_ITT_BUG and ICE_SHELF_SALT_FLUX_ITT_BUG to ICE_SHELF_BUOYANCY_FLUX_ITT_BUGFIX and ICE_SHELF_SALT_FLUX_ITT_BUGFIX to avoid confusion with other ..._BUG parameters where `true` is to turn the bugs on, whereas here `true` fixes them. The old names are retained via `old_name` arguments to the `get_param()` calls, so no existing configurations will be disrupted by these changes. Additionally, an expression to determine a scaling factor to limit ice-shelf bottom slopes in `calc_shelf_driving_stress()` was refactored to avoid the possibility of division by zero. This commit will change (and correct) answers for cases with ICE_SHELF_BUOYANCY_FLUX_ITT_BUGFIX set to true, but as these would often fail with a NaN from taking the square root of a negative value, it is very unlikely that any such configurations are actively being used, and there seems little point in retaining the previous answers. No answers are changed in cases that do not use an active ice shelf. Co-authored-by: Alistair Adcroft <adcroft@users.noreply.github.com>
- rotated OBC%segment%num_fields needs to be set.
Fix a bug that the recently changed default answer date for TIDES_ANSWER_DATE is not properly applied to MOM_self_attr_load. TIDES_ANSWER_DATE is used in MOM_self_attr_load to check if SAL_USE_BPA is used after a timestamp, so its default should be consistent with MOM_PressureForce_FV.
Thanks to both @alperaltuntas and @marshallward who noted that a PPM limiter has the expression `( u2 - u1 ) * ( u1 - u0 ) <- 0.0` which is interpreted as `( u2 - u1 ) * ( u1 - u0 ) < -0.0a. Needless to say, the intended code was `( u2 - u1 ) * ( u1 - u0 ) <= 0.0`. The same typo was copied to three files. The high-order estimate of edge value was previously bounded by (u2,u1) or (u1,u0). The missed conditions of either `( u2 - u1) == 0.` or `( u1 - u0 ) == 0.` would then have been caught by the subsequence test for an interior extrema. Thus, I think the cell was still limited to PCM appropriately. However, the typo obscured the intention of the limiter and I was lucky it still worked.
This commit allows the frequency-dependent drag to be implemented in tensor form, by incorporating the off-diagonal components of the wave drag tensor into the MOM_wave_drag module.
Recon1d_PLM_WLS provides a piecewise linear reconstruction where the slope is the "best" fit as determined by volume-weighted least squares. The reconstruction is NOT limited by neighboring cells. Therefore, this reconstruction is NOT useful for vertical remapping or grid generation. It is instead intended for the pressure gradient calculation; the idea is to disconnect the PLM slope from the values in vanish(ing) layers which appear to be the source of pressure-gradient errors over topographic slopes in z*-coordinate tests. Because the normal limiters do not apply, the only test I could think of was to check that the least squares fit was actually correct. The documentation explains how this was checked (which took a while due to round-off challenges with the loss function).
Corrected the descriptions of variable units in 64 comments spread across 16 files, including a dozen instances where "arbitrary" was misspelled. All answers are bitwise identical and only comments were changed.
Updated the values of about 21 parameters (many of which are repeated across TC test cases) used in the TC testing to test the most recent versions of code that is selected with ANSWER_DATE flags and to avoid testing the buggy versions of code that is regulated by _BUG flags. This includes some changes to broaden the range of equations of state that are being tested and to test some newer versions. This does change the details of the TC tests, but they should (and do) still pass TC regression tests across code versions.
* Added frazil to ice shelf The frazil mass flux to the ice-shelf base is calculated by multiplying frazil energy [J m-2] by the inverse of the timestep times the latent heat of fusion [kg J-1 s-1]. This frazil mass flux is incorporated as a negative water flux from the ice shelf. This negative water flux then acts to add the frazil mass to the ice shelf base (MOM_ice_shelf.F90/change_thickness_using_melt) and remove it from the ocean surface as evaporation (MOM_ice_shelf.F90/add_shelf_flux). Note frazil is reset to zero at the start of each therm timestep in MOM.F90/step_MOM. Some additional changes were also made to how the ice-shelf flux factor is implemented, so that is only scales ice-shelf melt without affecting the frazil mass flux. * Fixed a commented line where fluxes%water_flux should be ISS%water_flux
The PLM reconstruction used within the pressure gradient force now supports the weighted least squares approach for slope estimation. In a catastrophic version of seamount/z where vanished layers slightly inflate, the regular finite volume PLM method is sensitive to values in the vanished layers and leads to a feedback that causes en error growth (spontaneous motion). The PLM-WLS method is insensitive to the vanished layers and in the same test leads only to round-off level noise in the flow.
* Spatially varying bottom drag coefficient The spatially varying bottom drag coefficient can be specified by providing a map of the spatially varying scaling factor. * Spatially varying bottom drag coefficient Fixed the inconsistency at open boundaries when CDRAG_MAP is true.
In a number of cases, total resting column thickness is calucated as
G%bathyT + G%Z_ref, which is largely correct but for wetting, i.e.
G%bathyT < 0. This commit makes a correction for seven cases with this
potential bug.
There is no answer changes if no wetting points are used and G%Z_ref is
zero.
List of modules/processes affected:
* MOM_barotropic
* affects only surface stress when BT_NONLIN_STRESS is False.
* MOM_wave_speed
* h2 calculations in
* subroutine internal_tides_init
* subroutine int_tide_input_int
* subroutine tidal_mixing_init
* MOM_lateral_mixing_coeffs
* MOM_MEKE
In commit b8c807b, we made the test for SSH penetrating the sea floor when using BT_LIMIT_INTEGRAL_TRANSPORT because we thought it could never happen. Unfortunately, floating-point round off allows violations and we were hitting the now fatal error. This commit calculates the precision we can expect for the current SSH and then if the ocean thickness has become negative within this precision, we reset to zero thickness. This should not change answers in that BT_LIMIT_INTEGRAL_TRANSPORT is a new option, and if anyone was using it they would have encountered a FATAL, and this fix does not alter any positive thicknesses.
When debugging with all run-time tests turned on, the integer `num_lines` was flagged as used but uninitialized when being passed to `broadcast()`. I don't think the code was wrong, just that the checks expected the "inout" argument to be set on all processors when the purpose of `broadcast()` is to take the value from root_PE and send to everyone else. I don't know why this hadn't been detected before - maybe compiler version. The fix is trivial and has no impact on production codes or answers.
`oldfn` was not initialized when used in a logical test. This did not matter for numerical results; the logical expression always evaluated to the False correctly due to other parts of the expression. Nevertheless, this variable was technically used uninitialized and a debugging executable doesn't get past this. Hence the fix.
When debugging the ice sheet configuration, a non-zero barotropic transport could not be reconciled with the layer transports because the derivative of net layer transports was zero (d/dv hu). This arose due to all layer flows pointed from vanished to thick so that their marginal thicknesses were individually zero. Adding a floor to the marginal thickness allows the solver to find the adjustment that does reconciles the two estimates. I've made this optional via parameter CONT_USE_H_MARG_MIN, and with default of False. If this situation had occurred before, we surely would have had a crash so it's likely that always applying this floor would not change answers. However, there's the weak possibility that a teeny-tiny transport, smaller than H_subroundoff, has existed in a run and then this answer would change. With the default of False we can be sure there are no answer changes, but it is recommended to use this option for safety.
The CHANNEL_DRAG option was using a harmonic mean to interpolate adjacent bottom depths at velocity points to vorticity points. However, this is not well behaved when the bottom depth is negative (i.e., above sea level), as was noted as a part of PR mom-ocean#975. This commit adds the new runtime parameter CHANNEL_DRAG_SHELFBREAK_DEPTH to set a depth below which a harmonic mean bottom depth is still used to mimic a continental shelfbreak profile, but above which a simple arithmetic mean is used to interpolate bathymetry to vorticity points for use with CHANNEL_DRAG. The expressions vary continuously with depth and avoid the previous problems with division by zero or a badly formed harmonic mean. By default, all answers are bitwise identical in any cases that worked previously, but cases with oceans (or Great Lakes) in basins with bottoms that are above sea-level should now work sensibly when CHANNEL_DRAG is enabled. There is a new runtime parameter in some cases.
Corrected the incorrect or inconsistent unit descriptions of 28 variables, added descriptions of the units of 4 others, and corrected the non-standard syntax (e.g. backwards or in the wrong order) in the description of 35 variables, scattered across 27 files. Only comments are changed and all answers are bitwise identical.
* Fix for ice-shelf friction velocity bugs Fixed an incorrect area used to calculate cell-centered ocean surface velocity under the ice_shelf, which can impact the calculation of ice-shelf friction velocity. Added missing flags to some allocate_surface_state calls so that sfc_state%taux_shelf and sfc_state%tauy_shelf are allocated. This is required for the surface-stress-based (rather than surface-velocity-based) calculation of ice-shelf friction velocity. Also added taux_shelf and tauy_shelf as diagnostics for the surface stress under the ice shelf. * Removed unneeded taux_shelf and tauy_shelf diagnostics * Added ustar_from_vel_bugfix flag, which if true, fixes the ustar from ocean velocity bug
This patch undoes a coupling of the FMS infra layer to the MOM6 framework code. In the current FMS infra layers, the `get_extern_field_info()` and `init_extern_field()` functions require content defined in `src/framework`. This prevents the development of new independent infra layers, which much also depend on infra-agnostic content. In particular, the FMS2 implementation of `get_extern_field_axes()` relies exclusively on the framework function, `get_var_axes_info()`. Both infras also return the `axes_info` type, a MOM-specific framework-level descriptor, rather than the infra `axistype`. This patch resolves these inconsistencies. * `axis_info` no longer appears at infra-level. All relevant functions now reference `axistype`. * `src/framework/MOM_io.F90` now provide functions for translating `axistype` to `axis_info`. Some specific changes are summarized below. * `get_external_field_info` is now a framework-level function of `MOM_interpolate.F90` , using infra-level implementations of `get_extern_field_(size|axes|missing)`. Each is now explicitly defined at the infra-level. * The FMS2 `get_external_field_axes` is now an entirely new function, and is largely a duplicate of `get_var_axes_info()`. The major difference is that it returns a list of `axistype`. It also replaces the fixed x-y-z fetch with a slightly more generic list of axes. (It still requires at least three dimensions, however.) * `set_axis_data` is only used internally by the FMS2 infra. It is included in FMS1 but raises an nonimplementation error. There is one minor API change. * The `name` argument was added to `get_axis_data`. It is now the second argument, to match the style of existing functions, and size was moved to the third argument. Other minor framework references have been removed. * `MOM_error` and `FATAL` now refernce their `MOM_error_infra` equivalents. * `lowercase`, which was previously only defined in FMS1, has been added to the FMS2 infra. Note that this is a duplication of the function in `src/framework/MOM_string_functions.F90`.
When a z-coordinate diagnostic grid is specified via the "PARAM" method of coordinate definition, then the number of levels was always the same as the main model. This commit fixes this by first allowing for upto a 1000 levels in the new grid, checking for the actual requested size, and then allocating to that size. It appears we have no examples using this mode, which is probably how this bug has persisted so long. This "PARAM" method of specifying grids is being used in a range of new CMIP7 diagnostics in both MOM6 and COBALT.
…mom-ocean#1003) * Init all sponge tendency diag IDs to -1 immediately * No need to reset to -1 since initialized when declared * Move init_ALE_sponge_diags to after all tracers have been set up
These two references to members of a pointer don't seem to be hit except under special circumstances but nevertheless I ran in to them when debugging an unrelated problem. There are two references to members of `diag%axes` that assume `diag%axes` are associated, but in the specific case I was debugging this was not the case.
Five vertically integrated diagnostics are requested in CMIP7. These ultimately are to be for four vertical intervals (0-300m, 300-700m, etc.) but we will handle that through addition of a 4-level diagnostic grid, configured at run-time. This commit handles the conversion from temperature or salt to heat content or salt content (by mass) and registers a "vertically extensive" quantity so that the diagnostics know to re-integrate rather than remap. Changes: - Added diagnostics absscint, pfscint, scint, chcint and phcint - Moved registration of temp_int and salt_int to within an existing `if (use_temperature)` block - Made public 2 GSW conversion functions in MOM_EOS
…roducing_sum (and therefore, mpp_sum) is called. Previously, several 2-D arrays were each being passed within their own reproducing_sum calls, which is now avoided by consolidating the 2-D arrays into one 3-D array that is passed to a single reproducing_sum call.
…ux calculation. Needed for runs without frazil.
This patch renames the EFP integer count (or "limbs") from the generic ni to the more numerically accurate efp_digits.
The max_count_prec parameter was used to track the maximum number of summands that could be added before carry overflow could occur. It was also made public so that the unit test could compare the "fast" and "normal" increment_ints functions. The parameter has effectively been replaced with max_summands. This patch eliminates max_count_prec from the module and its API. The block-based reproducing sum no longer requires one to distinguish between the fast and normal methods, so there is no longer a need to identify this case in the unit tests, and the parameter is removed from the public API. max_count_prec has also been replaced by max_summands. Tests related to numPEs now assume that N ranks perform N-1 summations, and the valid range can now be increased by one.
Conflict resolutions:
config_src/drivers/nuopc_cap/mom_ocean_model_nuopc.F90
- updated arguments to allocate_surface_state
- fixed case of use_MARBL_tracers
src/core/MOM_forcing_type.F90
- register_forcing_type_diag argument update
src/core/MOM_variables.F90
- MLD_param and h_ML_param added to visc
src/parameterizations/lateral/MOM_hor_visc.F90
- first Kh_BS store to BS_coeff_q was preserved
- I moved this inside of the use_Leithy if-block
src/parameterizations/stochastic/MOM_stochastics.F90
- [m] units used in place of [nondim]
src/parameterizations/vertical/MOM_CVMix_KPP.F90
- dev/gfdl
src/parameterizations/vertical/MOM_set_viscosity.F90
- dev/gfdl
src/tracer/MOM_tracer_advect.F90
- Tr(m)%OBC_inflow_conc usage removed / %tres always used
- Track flux_type for %ad2d_[xy], %ad_[xy]_resolved, etc
(Note difference in x-y handling for bit repro)
src/tracer/MOM_tracer_registry.F90
- dev/gfdl
ac/deps/Makefile:
- CVMix hash was updated to match the submodule
This pull request adds support for submodule parsing and data
dependencies in makedep.
For a module `parent` in `parent_file.F90` and a submodule `child` in
`child_file.F90`, the following rules are provided.
parent.mod parent.smod: parent_file.o
parent_file.o: parent_file.F90 <other deps>
parent@child.smod parent-child.mod: child.o
child.o: child.F90 <other deps>
This captures rules for multiple compilers:
* GCC produces a `parent.smod` file
* GCC and Intel submodule data is in `parent@child.smod`
* NVIDIA submodule data is in `parent-child.mod`.
These rules are compatible for all of our tested compilers, but may need
to be revised for others.
GCC appears to produce additional `.smod` parent dependencides, which
are not tracked by makedep. The rules are also not entirely clear to me.
This can be revised later if needed.
But the primary feature is working as we hope: Modifications to
`child_file.F90` do not require a rebuild of `parent_file.F90` or any
other dependencies, and development build times are significantly
reduced.
When MOM_io switched to using newunit= in its open() calls, it replaced a static arbitrary file handle with an automatically generated handle. This caused a new file to be created after every call. In write_energy and its ice shelf analogue, the new handles had the same file name, so it would have gone undetected in many cases. In ice shelf development, this was detected because it exceed the open file limit in the OS. This patch now queries if the file has already been opened. Only then does it open the file for appending. This check is not applied at Start_date, since we presumably would not call these functions twice on the initial timestep. But maybe that possibility should be considered? Minor changes: * the handle is now initialized to -1; newunit will never assign this value in a standards-compliant compiler * The files are now closed in the termination functions. Previously we relied on runtime and OS cleanup.
This patch removes open_ASCII_file from the module imports, since it is unused.
FrictWork_bh is currently only computed when id_FrictWork_bh or id_FrictWorkIntz_bh are enabled, or if MEKE%mom_src_bh is allocated. However, MEKE%mom_src accumulation uses FrictWork_bh whenever MEKE%backscatter_Ro_c /= 0. If mom_src is allocated but mom_src_bh is not (e.g., MEKE_bhFrCoeff < 0), FrictWork_bh can be uninitialized here. Assisted-by: Copilot
Reconcile the changes introduced with the flux_type argument to advect_tracer with separate changes that modified the treatment of tracer properties in reservoirs and the use of unnecessary masking in the calculation of the tracer fluxes. Specifically, it restores `advect_this_tracer` tests that were accidentally dropped from the open boundary condition tracer fluxes reconciling changes coming in via dev/ncar and dev/gfdl, and it does not use masking via tests for `do_i` that in some cases was using uninitialized logical values in the halo regions of these arrays. The `advect_this_tracer` tests were introduced with the separate use of `advect_tracer()` for resolved and parameterized tracer transports as a part of (github.com/NCAR/pull/352). The elimination of the use of the `OBC_inflow_conc` elements occured in (github.com/NOAA-GFDL/pull/1117). The elimination of the use of the `do_i` masks when calculating fluxes occurred in (github.com/NOAA-GFDL/pull/1110) and addressed the issue at (github.com/NOAA-GFDL/issues/952). All solutions are bitwise identical, but these changes should avoid some warnings or errors when running with sensitive error handling.
Correct the spacing in the multi-line parameter documentation message for USE_CR_GRID. Without this change separate statements clarifying the meaning this parameter were combined without any space between them. Also added a logical test before a checksum call for h_MLD in mixedlayer_restrat_OM4 to avoid a possible segmentation fault due to a checksum call using an unassociated pointer in some cases with DEBUG = True. (This potential segmentation fault might never have actually been encountered.) This commit will change the output in some MOM_parameter_doc files, but all answers are bitwise identical.
Correct a fatal error message in neutral_diffusion_calc_coeffs() that had been copied over verbatim from hor_bnd_diffusion(). All answers are bitwise identical.
Correct the syntax of some enddo and endif statements in recently added code in register_forcing_type_diags(), auto_determine_io_layout(), diabatic_driver_init() and initialize_MARBL_tracers() to match the syntax documented in the MOM6 style guide and that is already in use elsewhere in these modules. These changes have come in to the main branch via dev/ncar, and hence were not included in similar changes in (github.com/NOAA-GFDL/pull/1075). All answers are bitwise identical.
Add a private subroutine so that dz can be calculated and used by both dynamics and tracer update for OBC segment data. For the less frequently called subroutine read_OBC_tracer_data, call to update_OBC_segment_dz is triggered by providing optional arguments tv and h to either subroutines. This is a preparatory step to use an updated thickness to remap OBC tracer source data.
Adds OBC_TRACER_DZ_BUG (default=enable_bugs) to control whether segment layer thicknesses are recomputed from the current state before reading OBC tracer data at the tracer advection step. When false (fixed): update_OBC_segment_dz(h, tv) is called before reading, giving tracers up-to-date thickness for vertical remapping. When true (buggy): dz is unchanged from the beginning of the last dynamics-step read, when OBC dynamics are updated. This also preserves the legacy behavior of reading OBC tracers in the dynamics step rather than the tracer step (when IGNORE_DT_OBC_SEG_UPDATE_OBGC is false). Note that, setting OBC_TRACER_DZ_BUG=False is sufficient to use the OBC tracer update per tracer-step path regardless of IGNORE_DT_OBC_SEG_UPDATE_OBGC.
* A top-level convenience `Makefile` has been added. * `configure.ac` was updated to find `Makefile.in` related to the root directory, rather than some rando path.
The .testing/ Makefile was reworked to use the top-level configure. This prevents much of the redundant copying of configure.ac and m4/ macros.
* The siglongjmp() function name was incorrectly assigned to SETJMP when missing. This patch corrects sets it to SIGLONGJMP. * Restore MAKEDEP_DIRS (formerly SRC_DIRS) so that it can be used to detect file changes, as detected by rwildcard. * Fix path of ac-clean targets * Remove legacy bin/ and mkmf/ dirs from ac/deps/ * Remove top-level .gitignore (`build/` is not a defined output) * Remove legacy ac/m4/ax_fc_check_lib.m4 (previously replaced by mom6_fc_check_lib) * Fix erratic indenting of in MOM6_FC_CHECK_LIB macro * Add license header to MOM6_FC_CHECK_LIB * .gitignore updated to ignore configure, and *not* ignore `build/` or any other hypothetical build directory.
Set h_marg_min to about -1.057 lightyears when CONT_USE_H_MARG_MIN is False, to recover previous answers from the main branch of MOM6. Before CONT_USE_H_MARG_MIN was added, there was no floor on the marginal thicknesses, but the time that this parameter was added in github.com/NOAA-GFDL/pull/996 in December, 2025, a physically sensible floor of 0 was added for the marginal thicknesses for the continuity transports when it was not true. The marginal thicknesses is the thickness at a particular velocity in the sub-gridscale reconstruction that is integrated to get the time-integrated transport. However, there appear to be some cases where setting this floor to 0 changes answers from those with the main branch of MOM6. At this time we do not know how these negative marginal thicknesses might arise; perhaps they arise from CFL values that exceed 1 while iterating to reconcile the baroclinic and barotropic transports. However, resetting the floor to a huge negative value essentially ensures that no floor is applied, and empirically the answers from the main branch are recovered.
GFDL to main (2026-07-27)
This was referenced Aug 30, 2026
sanAkel
approved these changes
Aug 30, 2026
sanAkel
left a comment
sanAkel
left a comment
Collaborator
There was a problem hiding this comment.
This is a routine PR that has gone through extensive review and testing from @jiandewang himself (in the context of this repository).
|
Are the two failing check on this sub-PR expected to fail, or is there an issue? |
Collaborator
Author
|
@gspetro-NOAA they are expected to fail and will be corrected in future PR. Thanks for checking |
|
@sanAkel Testing succeeded on ufs-community/ufs-weather-model#3344. This PR can be merged. |
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
20 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.
MOM6 main was updated on 20260828 which is originally GFDL's 20260727 PR https://github.com/mom-ocean/MOM6/pull/1708.
Note this PR requires a new BL in UFS due to the changes in two variables (cau and cav) in restart files. Details on the reason and impact of these two changed variables can be found at mom-ocean#1708 (comment)
fail_test_hafs_regional_storm_following_1nest_atm_ocn_wav_mom6_intel case changed its baseline due to a bug fix. Use flag OBC_TRACER_DZ_BUG=T can retain original baseline but here we decided to use OBC_TRACER_DZ_BUG=F (default) which means we will use the bug fixed code.