Skip to content
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

GFDL to main (2023-04-06) #1598

Closed
wants to merge 194 commits into from
Closed

Conversation

marshallward
Copy link
Collaborator

This pull request contains several new featuers and bugfixes, alongside a very large number of commits related to standardization of dimensional analysis. Despite the size, these should have no impact on any experiments beyond adjustments to configuration parameter files.

Note that this patch introduces the new netCDF I/O layer to handle simple operations which do not require - or simply do not work with - the FMS I/O layer. Reviewers should carefully verify any experiments which depend on datasets, particularly anything which may be outside of our standard test suite.


Features

Bugfixes

Refactor

Build/Test

Admin

Contributors

Hallberg-NOAA and others added 30 commits October 28, 2022 13:23
  Created the new module remapping_attic to hold older versions of remapping
code that are no longer used by MOM6.  The subroutines is PosSumErrSignificant,
remapByProjection, remapByDeltaZ and integrateReconOnInterval were moved to
remapping_attic, where they can be tested by calling remapping_attic_unit_tests.
The hard-coded old_algorithm logical module variable and the code it wraps were
also eliminated.  Also added a schematic description of the units of the real
variables in the various routines in MOM_remapping and corrected some spelling
errors.  All answers are bitwise identical.
  Moved interpolate_column and reintegrate_column (without changing anything)
from MOM_diag_vkernels.F90 to MOM_remapping.F90 and incorporated the tests
that had been in diag_vkernels_unit_tests into remapping_unit_tests.  The
entire MOM_diag_vkernels.F90 file was then removed.  All answers are bitwise
identical, although the module for two public routines was changed and a third
was eliminated.
  Remove missing_value arguments to interpolate_column and reintegrate_column,
instead using 0 for the values in vanished cells.  This change helps to address
github.com/mom-ocean/issues/769.  Also added comments schematically
describing some of the argument units.  Because 0 was already being used for the
missing value (except in unit tests), all solutions are bitwise identical.
  Added the new subroutine check_remapped_values with the duplicative error
checking code in remapping_core_h and remapping_core_w, both to reduce code
volume and promote code coverage, and to make the substance of these two
routines easier to follow.  All answers are bitwise identical.
- Adds `.gitlab/pipeline-ci-tool.sh` to enact most of the stages of the gitlab CI pipeline
  - enables interactive/command-line reproduction of the pipeline
  - `.gitlab/pipeline-ci-tool.sh` is documented in .gitlab/README.md with instructions
    on how to use at the command line and what each function is doing
- All commands formerly in .gitlab-ci.yml are now one-line invocations of `.gitlab/pipeline-ci-tool.sh`
  so .gitlab-ci.yml is now considerably smaller and easier to read with statements like
  `.gitlab/pipeline-ci-tool.sh mrs-compile debug_gnu` or `.gitlab/pipeline-ci-tool.sh check-params gnu`
- Previously, all results were compared again the stored regression answers. This meant that
  any error (e.g. layout) would show up as a fail of all types. We use the regression answers to
  check the repro-symmetric mode and then compare everything else to repro-symmetric or other results
  as appropriate. This allows us to distinguish between types of errors. The GH actions are doing it
  this way, and we originally did this in the first forms of the pipeline, but in the last re-factor
  I lazily switched to using the regression answers for everything.
- The subroutine categorize_axes cannot find the axes in ice restart files and gives warnings
WARNING from PE     0: categorize_axes: Failed to identify x- and y- axes in the axis list (xaxis_1, yaxis_1, Time) of a variable being read from INPUT/ice_model.res.nc
- This leads to an incorrect initializations and a subsequent sat.vap.press.overflow crash when using
infra/FMS2
- Same experiment runs fine with infra/FMS1
- After the fix the infra/FMS1 and infra/FMS2 answers are bitwise
identical
- The subroutine categorize_axes cannot find the axes in ice restart files and gives warnings
    WARNING from PE     0: categorize_axes: Failed to identify x- and y- axes in the axis list (xaxis_1, yaxis_1, Time) of a variable being read from I
NPUT/ice_model.res.nc
- This leads to an incorrect initializations and a subsequent sat.vap.press.overflow crash when using
    infra/FMS2
- Same experiment runs fine with infra/FMS1
- After the fix the infra/FMS1 and infra/FMS2 answers are bitwise
    identical
Added a line initializing the string Cartesian to a blank string in categorize_axes, so that it not be uninitialized when it is used a few lines later.
  Set the interpolation weights inside of interpolate_column to explicitly be
the complement of one another, thereby saving an extra division at each point
and reducing the number of variables that need to be stored, in preparation for
the creation of a separate subroutine to find interface positions.  This commit
is mathematically equivalent to what was there before, and the extensive unit
testing of interpolate_column is still passing, but it changes the value of some
interpolated interface diagnostics at the level of roundoff (but not the MOM6
solutions themselves, as they do not depend on interpolate_column yet).
This patch introduces a new autoconf macro, AX_FC_CHECK_C_LIB, which
confirms if the Fortran compiler can be linked to the netCDF C library.
As with other netCDF tests, the nc-config tool is used if necessary (and
available).

This resolves some recent issues on platforms where netCDF and
netCDF-Fortran are installed in separate locations, with different
library directories (-L).

It also resolves some false assumptions in configure.ac which presumed
equivalent access by the configured C and Fortran compilers.
Previously, we would test if the C compiler could be linked to netCDF,
and then assume that the Fortran compiler shared the same relationship.
We now use the Fortran compiler for both C and Fortran tests.

This patch fixes many issues observed on MacOS systems, including some
persistent problems on the GitHub Actions MacOS tests.  For example, we
can now use the default GCC 12 compilers, rather than forcing a rollback
to GCC 11.
This patch fixes some issues with testing of C bindings in Fortran.
Specifically, some tests are using a C compiler which may be
unconfigured, causing unexpected errors.

The autoconf script now uses the Fortran compiler to test these
bindings, rather than using the C compiler to test for their existence.
A new macro (AX_FC_CHECK_BIND_C) was added to run these tests.

This achieves the actual goal (test of Fortran binding) on top of the
original goal (availability of C function), while ensuring that the actual
compiler of interest (FC) is used in the test.

Two C-based tests are still present in the script for testing the size
of jmp_buf and sigjmp_buf.  The C compiler is now configured with the
AX_MPI macro, and is only used to determine the size of these structs.
* Setup OBC segments for COBALT/OBGC tracers

    - These are updates required to setup OBC segments for OBGC tracers.
    - Since COBALT package has more than 50 tracers using the MOM6 table
      mechanism for setting up OBC segments is not feasible. Rather, this
      update delegates such setup to mechanims used in ocean_BGS tracers
      leaving MOM6 mechanism for native tracers intact.
    - Fixed issues caught by MOM6 githubCI

* Add capability to change obc segment update period

- COBALT tracers do not need as frequent segment bc updates and can
  use a larger update period to speed up the model.
  This commit introduces a new parameter DT_OBC_SEG_UPDATE_OBGC
  that can be adjusted for obc segment update period.
- This commit applies the change only to BGC tracers but can easily
  be changed to apply for all.

* Insert missing US%T_to_sec

- The unit conversion factor was missing causing a crash in a newer test.

* Updates from Andrew Ross

- Avoid low initial values in the tracer reservoirs

* Per Andrew Ross review

* corrected indentation per review

* Avoid using module vars per review request

- Reviewer asked to avoid using module variables with "save" attributes.
- This commit hides the module variables inside the existing OBC type.

* Coding style corrections per review

* Modification per review: do_not_log if .not.associated(CS%OBC)

Co-authored-by: Robert Hallberg <[email protected]>
In this PR an option is added to use ice viscosity computed from the observed surface velocity, computed by the model and use a constant value (for debugging purposes). A new (char) parameter "ICE_VISCOSITY_COMPUTE" is introduced; its values can be "MODEL" (the ice viscosity computed by the model); "OBS" the ice viscosity is computed at the preprocessing step and read from a file (its name is defined by the parameter "ICE_STIFFNESS_FILE") into a variable with a name defined by "A_GLEN_VARNAME" parameter; "CONSANT" is a constant value defined by a parameter "A_GLEN". These changes are in MOM_ice_shelf_dynamics.F90. Minor changes are done to MOM_ice_shelf_initialize.F90 to correct units, scales.
  Added calls to get_param to set 12 input variable names in files via runtime
parameters, including TIDEAMP_VARNAME, TEMP_COORD_VAR, SALT_COORD_VAR,
THICKNESS_IC_VAR, INTERFACE_IC_RESCALE, TEMP_IC_VAR, SALT_IC_VAR, BASIN_VAR,
TIDAL_DISSIPATION_VAR, ROUGHNESS_VARNAME, TIDEAMP_VARNAME and KH_BG_2D_VARNAME.
Also added two new runtime parameters, THICKNESS_IC_RESCALE and
INTERFACE_IC_RESCALE, to allow input thickness and interface height fields to be
rescaled.  A number of spelling errors in comments or output messages in the
files that were being modified as a part of this commit, including changes in
the documentation that appears in MOM_parameter_doc files.  All answers are
bitwise identical, but there are new entries and minor changes in many
MOM_parameter_doc files.
  Added calls to get_param to set 4 more input variable names in files via
runtime, including U_IC_VAR, V_IC_VAR, OPEN_DY_CU_VAR and OPEN_DX_CV_VAR.  Also
added or amended comments describing internal variables to describe their units
more consistently in MOM_shared_initialization.  All answers are bitwise
identical, but there may be new entries in some MOM_parameter_doc files.
  Corrected a bug in converting depths read from an input file from units of cm
to m when the ER03 version of tidal mixing is used.  This commit will change
answers when INT_TIDE_DISSIPATION = True, USE_CVMix_TIDAL = True, and
TIDAL_ENERGY_TYPE = "ER03".  There are no such configurations in the
MOM6-examples pipeline tests, and it is not clear whether or where such a
configuration has ever been used.

  This bug was introduced into dev/gfdl on Nov. 19, 2018 as a part of PR mom-ocean#883 in
commit 967e470, which was supposed to
be a refactoring of this portion of the code without changing answers, but
introduced this bug.  This commit should restore solutions with impacted
configurations to what they would have been before that earlier commit.
This patch removes the `build_{grid,data}.py` scripts from .testing's
tc4, along with the setup of the Python infrastructure used in the
.testing Makefile and GitHub Actions CI.

The Python scripts have been replaced with equivalent Fortran programs
which generate identical netCDF output.

A new rule (`preproc`) has been added to the .testing top Makefile for
generating the model input files.

The netCDF compiler dependenices are configured with autoconf, currently
duplicating the macros in `ac/configure.ac`. (NOTE: It may be possible
to share these with a common macro in ac/m4.

The configure script and Makefile are currently generated from
`configure.ac` and `autoreconf`.  In the future, we could simply
pre-generate `configure` and add it to the repository.

This patch was motivated by the inability to install recent
netCDF-Python packages on systems with older gcc compilers, including
our main production machine.  We could have possibly resolved this by
adding compiler configuration to pip, or perhaps reported the issue to
the netCDF-python project for them to resolve.  But the costs of relying
on all this Python infrastructure is starting to exceed the benefits,
and I would recommend we excise it from our test suite.
GitLab CI includes the internal testing suite (.testing) and included an
explicit setup of the Python environment (`make work/local-env`).  The
rule has since been removed, and the command now fails.

This patch removes those steps, since we no longer use Python in the
tests.

It also slightly reworks the reporting of test output.  Instead of
re-running `make test`, it uses the `make test.summary` rule to report
the final result.
  Added a new logical argument to interpolate_column to specify whether the
interpolated interface values outside of massless layers should be masked to
zero.  Also refactored the code in interpolate_column to separate out the
determination of the interface position from the interpolation and the masking
to facilitate the extension of this code to use higher order interpolation in
planned subsequent changes.  All answers are bitwise identical, although there
is a new mandatory argument for a public interface.
  Added ALE_remap_interface_vals and ALE_remap_vertex_vals to handle the
interpolation of variables that are at the interfaces atop tracer cells or above
the corners of the tracers cells from one grid to another.  Because these are
not yet used (but have been tested in calls that will be added with the next
commit) all answers are bitwise identical, but there are two new publicly
visible routines.
  Added REMAP_AUXILIARY_VARS to control whether to remap the accelerations that
are used in the predictor step of the split RK2 time stepping scheme.  Also
added the new routines remap_dyn_split_rk2_aux_vars, remap_OBC_fields and
remap_vertvisc_aux_vars to do the remapping, and code to call these routines
when REMAP_AUXILIARY_VARS is true. By default, REMAP_AUXILIARY_VARS is false,
and all answers are bitwise identical, but the entire MOM6-examples regression
suite has been run with this set to true, and they do appear to give physically
plausible answers in all cases, partially addressing the issue noted at
github.com//issues/203.  New entries are added to the
MOM_parameter_doc files, and there are three new publicly visible routines, but
by default answers do not change.
* Adds the option to set the diffusivity KHTH as horizontally varying
* Can be enabled via READ_KHTH = True, filename is provided by user via KHTH_FILE
* Will return error if user sets both READ_KHTH = True and KHTH > 0
* full file path is now set as INPUTDIR/KHTH_FILE, where both
  INPUTDIR and KHTH_FILE are runtime parameters
thickness diffusivity --> isopycnal height diffusivity
  Corrected the units written to the output files for 4 diagnostics (CAu_Stokes,
CAv_Stokes, area_shelf_h and sfc_mass_flux) and added missing units arguments to
the get_param calls for some (mostly unlogged) parameters.  The logged calls
where units are added include those for EKE_MAX, NDIFF_DRHO_TOL, NDIFF_X_TOL,
and IMPULSE_SOURCE_TIME, while some unnecessary carriage returns were removed in
the descriptions of some of these and closely related parameters.  Also added
units to the comment describing the AGlen argument to initialize_ice_AGlen.  All
answers are bitwise identical, but there can be minor changes in the metadata of
some files, and some MOM_parameter_doc and available_diags files might exhibit
minor changes.
  Added a missing scale factor in the DENSE_WATER_EAST_SPONGE_SALT get_param
call in dense_water_initialize_sponges, and added comments describing the local
variables (and their units) throughout the dense_water_initialization module.
The variable set by DENSE_WATER_SILL_HEIGHT was unused and it probably was
always intended to be DENSE_WATER_SILL_DEPTH, which it now is.  Units arguments
were also added to two of the unlogged get_param calls in this module.  Without
this change, this test case would not reproduce with dimensional rescaling due
to a scale factor that was omitted when salinity was being rescaled on May 3,
2022, which became a part of PR #122 to dev/gfdl, but answers should not change
when dimensional rescaling of salinities is not used.  All answers and output in
the MOM6-examples test suite are bitwise identical.
  Removed meaningless units arguments from 31 get_param calls for integer,
character, or logical parameters.  All answers are bitwise identical, but some
entries in the various parameter_doc files are changed.
Hallberg-NOAA and others added 29 commits February 2, 2023 14:15
  Amended comments to document the units of 7 internal variables, arguments or
elements of types in MOM_sponge.F90.  Only comments are changed, and all
answers are bitwise identical.
  Added or amended comments to document the units of numerous internal variables
in MOM_internal_tides.F90, and corrected a few spelling errors in comments.
Only comments are changed, and all answers are bitwise identical.
  Added or amended comments to document the units of 13 internal variables in
MOM_tidal_forcing.F90, and corrected a few spelling errors in comments.  Only
comments are changed, and all answers are bitwise identical.
  Changed the terminology in the thickness_diffuse module to be more accurate,
specifically changing phrases like "thickness diffusion" to "isopycnal height
diffusion".  The two are only the same in the limit of a uniform bottom depth,
and isopycnal coordinate and a vertically uniform diffusivity, and the new
phrases reflect what is actually being done.  In addition, the "Brunt-Vaisala
frequency" is now being described as the "buoyancy frequency" for greater
clarity of language and less use of jargon.  Some units were also added to the
descriptions using the standard syntax used elsewhere in the code.  For now,
these changes are restricted to the internal comments, so that there are no
changes in the MOM_parameter_doc or output files.  All answers are bitwise
identical.
  This commit fixed a number of dimensional rescaling issues that were
introduced to the MOM_MEKE code switch the SmartRedis related options.
Specifically, it adds a missing scale factor for the time_interp_external call
for the offline data Eddy kinetic energy used in MEKE when EKE_SOURCE=file.  It
also corrects the documented units and conversion factors for 4 diagnostics
related to machine learning within MEKE.  This commit also adds or amends
comments to document the units of 27 internal variables in MOM_MEKE.F90, and
corrected a few spelling errors in comments.   All answers and output are
bitwise identical in the existing MOM6-examples test suite, but there may be
other examples where the units or rescaling of diagnostics are corrected or (in
cases using EKE_SOURCE=file) where dimensional consistency testing issues are
corrected for the solutions themselves.
  Added or amended comments to document the units of 75 variables in 23 files
in the src/core, src/initialization, src/ice_shelf and src/user directories that
had been overlooked when the other variables in these files had their units
documented.  Only comments are changed, and all answers are bitwise identical.
  Added or amended comments to document the units of about 250 variables in 6
files in the src/equation_of_state directories.  Also revised comments to make
it clear that the MOM6 code for the UNESCO equation of state is based on the
Jackett and MacDougall (1995) refit to the UNESCO equation of state (which uses
potential temperature as a state variable) as opposed to the original UNESCO
equation of state, as documented in an appendix to Gill (1982) (which uses
in-situ temperature as a state variable).  Also corrected a handful of spelling
errors in comments.  Only comments are changed, and all answers are bitwise
identical.
- After a recent update to how the build environment is defined within
MOM6-examples, the "no libraries" build test (unique to MOM6) is failing
because the Makefile now no longer contains the environment. This commit
overrides a CPP macro that points to the bash script that is needed.
  This commit makes a set of changes so that the chksums output by the OM4_05
configuration with DEBUG = True reproduce between 252 and 256 PE runs (note that
the solutions themselves already reproduced).  This is includes adding a new
optional omit_corners argument to the MOM_state_chksum and MOM_thermo_chksum
routines, and revising the haloshift, omit_corners or symmetric arguments on 6
subroutine calls, to check more appropriate loop ranges.  All answers are
bitwise identical, but there is a new optional argument to three publicly
visible debugging routines.
  Added or amended comments to document the units of 30 variables in
FMS_cap/MOM_surface_forcing_gfdl.F90, FMS_cap/ocean_model_MOM.F90,
solo_driver/MOM_surface_forcing and unit_drivers/MOM_sum_driver.F90.  Only
comments are changed, and all answers are bitwise identical.
  Added or amended comments to document the units of about 172 parameters or
variables in MOM_EOS_NEMO.F90.  Only comments are changed, and all answers are
bitwise identical.
  This commit adds an optional unscale argument to the log_param_real
interfaces.  All answers and output are bitwise identical.
  Use the new unscale optional argument in 10 log_param calls for real
variables.  All answers and output are bitwise identical.
  Because get_param is case-sensitive, CVMix_shear_is_used can incorrectly
return false if USE_PP81 = True and USE_LMD94 = False.  This would cause such
cases to fail to reproduce across restarts or use uninitialize or incorrect
arrays, but because the Pacanowski and Philader parameterization is very
out-dated it appears not to be used in any active test case.  Moreover, no one
has reported any such issues yet.  Therefore, rather than adding a flag to
reproduce the old (unreproducing) results, this commit simply corrects this bug.
The (case-sensitive) parameter "Use_PP81" was also formally obsoleted.  All
answers are bitwise identical in all known MOM6 configurations, but this could
lead to answer changes in certain unlikely configurations.
This patch introduces `read_netCDF_data`, a new method for reading
netCDF datasets using the native netCDF I/O interface.  It is designed
to resemble the existing `MOM_read_data` function.

Motivation
----------

Legacy input files may contain content which is not supported by the
newest framework I/O (FMS).  In order to retain support for these input
files, particularly over a wider range of compilers, this patch provides
an alternative method for reading these files.

Interface
---------

As with `MOM_read_data`, the function is provided with a netCDF filepath
and a variable name, and returns the values to a provided variable.

The `global_file` and `file_may_be_4d` flags have been dropped, since
they are related to specific FMS2 compatibility issues.  (Global vs
domain-decomposed reads is controlled by the presence of a `MOM_domain`)

Limited domain-decomposed I/O is supported, providing parallel I/O over
a single file, to the extent supported by the filesystem.
Parallelization over multiple files, as in FMS I/O, is not supported.
Each FMS PE (MPI rank) reads its own segment, as defined by its
MOM_domain.

Output can be saved to either compute or data domains; as in FMS, the
appropriate placement is inferred from the size of the output array.

Support is currently limited to time-independent 2D arrays with
center-cell indexing.  That is, the `position` and `timelevel` arguments
are not yet supported.  The subroutines raise an error if these are
provided, as an indication that they may support them in the future.

Implementation
--------------

Internally, the function opens a `MOM_netcdf_file`, generates its
field/axis manifest, and reads the field contents.  As with
`MOM_read_data`, an internal rotation may be applied.  The file is
closed upon completion.

(This behavior is designed to emulate the existing `MOM_read_data`; in a
future implementation, we may want to use a persistent file handle which
reduces the number of I/O operations.)

Opening a `MOM_netcdf_file` now supports a `MOM_domain` argument, which
is used to determine the index bounds of its local segment of the global
domain.  This is used to compute appropriate bounds for the native
netCDF IO calls.

As part of these changes, the `get_file_fields` function has been
separated into itself and a new function, `update_file_contents`, which
populates the internal axis and field metadata list.

Usage
-----

The following fields have been moved to the native netCDF IO:

* `tideamp` (tidal mixing, FMS surface forcing)
* `gustiness` (solo and FMS surface forcing)
* `h2` (roughness in tidal mixing)

This only comprises the fields which must be handled natively in order
for the GFDL regression suite to pass with the PGI compiler; more files
could be moved to native I/O in the future.

Bugfixes
--------

Some bugfixes to the netCDF I/O are also included:

* `filename` attribute is now only written in an writeable state

* Previously, `get_file_fields` (and now `update_file_contents`) assumed
  that every axis had an equivalent variable, which could lead to
  potential errors if an axis had no equvalent field, such as index
  bounds.

  We now count the number of variables with matching dimension names,
  tagged as axes, rather than assuming that every axis has a variable,
  and exclude them from the field list.

* Not a bugfix, but `hor_index_init` was modified so that `param_file`
  is now an optional input.  This function is used in `MOM_netcdf_file`,
  where `param_file` is not available.  The argument is only used to
  call `log_param`.

Previous usage of these functions was restricted to writing output with
well-defined content, so were unaffected by these issues.
The introduction of makedep means that Python is now a dependency of the
MOM6 compilation.  On some systems, we cannot assume that `python` is
the name of the interpreter, and strictly should not even assume that it
exists.

This is notably an issue on the recent MacOS M1 systems, which use
`python3` as its interpreter, and do not provide one named `python`.

This patch adds macros to the MOM6 and FMS autoconf builds to detect if
Python is on the system, and makes a few attempts to determine the name
of the interpreter (python, python3, python2).  Perhaps more can be done
here, but this is probably sufficient in nearly all cases.
reopen_MOM_file includes an inquire test to determine whether we are
attempting to reopen an existing file.  If missing, it will attempt to
create the missing file.

The switch from FMS to netCDF I/O exposed a race condition here, where
one rank may create the file, and another delayed rank may incorrectly
identify this new file as already existing.  This resulted in a
segmentation fault.

Unsure why this was not detected before; it could be that FMS was more
resilient to the possibility of missing files.  Regardless, the `exists`
value was volatile and could lead to potential error.

This patch introduces a temporary fix to the issue by checking the root
PE and threading value.  When threading is single-file, only the root PE
participates in the existence test and file creation.  This accounts for
the case where either the root PE or any larger subset containing the
root PE calls the function.  It does not account for the more exotic
case where a non-root PE many wish to create a file.

If threading is set to MULTIPLE (i.e. IO domains) then an error is
raised, since there's currently no safe way to implement an equivalent
`inquire()` test.

We can revisit this function when stronger controls around threaded IO
are introduced.  But for now, I believe that this is the best that we
can do.
Erratic slowdowns in our Lustre filesystem mean that test runtimes are
largely unpredictable.  This patch increases the runtimes from 20min to
1hr, to better cope with this issue.
  Corrected the units in the get_param call for WAVE_HEIGHT_SCALE_FACTOR, and
corrected the units descriptions in comments of 22 wind stress related variables
in 6 driver routines, from [R L Z T-1 ~> Pa] to [R L Z T-2 ~> Pa], but the
actual conversion factors in the code are correct.  Also fixed 42 other
inconsistent units in comments in 28 files scattered throughout the MOM6 code.
WAVE_HEIGHT_SCALE_FACTOR was added in December 2022 as a part of PR #289 to
dev/gfdl. These inconsistent units were detected because they do not match the
patterns of other valid units; most are recent additions.  Apart from a single
unit in a get_param call, only comments are changed, and all answers are bitwise
identical.
  Restored an else that was inadvertently deleted as a part of code clean up in
MOM-ocean/MOM6 PR mom-ocean#1127 on June 5, 2020.  This bug causes bulkmixedlayer to be
called twice (with cumulative effects) when 0. < ML_MIX_FIRST < 1., and not to
be called at all when ML_MIX_FIRST = 1.  This bug only applies to cases where
the bulk mixed layer is enabled by setting BULKMIXEDLAYER=True and
USE_REGRIDDING=False (i.e., in layered mode configurations with active
thermodynamics), however because the default value of ML_MIX_FIRST = 0, this bug
does not appear to be used in any active test cases, and it went undetected when
it was introduced.  All answers in the MOM6-examples test suite are bitwise
identical, but this could change answers in some cases.
  Revised write_energy so that only the root_PE attempts to open, reopen or
write to a netcdf file.  Although FMS can handle cases where multiple PEs make
the same calls with internal logic, this change avoids requiring such internal
(hidden) logical tests, and instead is more explicit on what is actually
intended.  This change is complementary to MOM6 dev/gfdl PR #328, which adds
internal logic to handle the case where all PEs are making a call to reopen a
single netcdf file.  All answers and output are bitwise identical.
Fixed two horizontal indexing bugs in `get_field_nc`, where the difference
between the array starting index (always 1 in this subroutine) and the values
in the handle argument were not being taken into account when the array was
being passed in with only its computational domain.

Also initialized the internal `unlim_index` array in `get_netcdf_fields` to fix
a problem with using an uninitialized array that was being flagged when run in
debug mode.

With this commit, the model is once again reproducing the expected answers when
rescaling is applied for vertical distances or time.

Co-authored-by: Marshall Ward <[email protected]>
Add "PPM_CW" as an option for INTERPOLATION_SCHEME and REMAPPING_SCHEME.
This implements the original Colella and Woodward (1984) edge calculation
for PPM. It computes 4th order explicit edge values but constrains
them to produce a monotonic profile, which is particularly effective
for remapping.

INTERPOLATION_SCHEME="PPM_CW" is identical to "REMAPPING_PPM_HYBGEN",
but hybgen_PPM_coefs has been replaced by edge_values_explicit_h4cw
and PPM_monotonicity for flexibility and to simplify upgrades.
Answers with existing INTERPOLATION_SCHEME options are unchanged.

REMAPPING_SCHEME="PPM_CW" is a new option which can perform better
than "P1M_H2" when used with INTERPOLATION_SCHEME="PPM_CW".  Answers
with existing REMAPPING_SCHEME options are unchanged.

HYCOM1 regridding walks a monotonic density profile to locate the
new interface locations where the interface density equals the target
density.  However, it assumes that moving one interface has no effect
on the density at all other interfaces and this need not be the case.
When regridding, with HYCOM1_ONLY_IMPROVES=True, an interface is only
moved if this improves the fit to its target density.  The default of
False does not change answers.
  Updated check_MOM6_scaling_factors and compose_dimension_list to reflect that
fact that MOM6 is now doing dimensional consistency testing for temperature (via
[C ~> degC]) and salinity (via [S ~> ppt]), with an expanded dimension of the
scaling key from 6 to 8 and additional calls to add_scaling.  Also updated the
weights on the add_scaling calls, which are essentially counts of the frequency
of the various unit descriptors in the MOM6 code, to reflect only the counts of
variables with doxygen comments (i.e., arguments, function return values and
elements of types) but excluding the user, framework and diagnostics directories
and the passive tracer packages.  All model solutions are bitwise identical, but
there will be updated suggestions for combinations of scaling factors that
minimize the aliasing of the units that are used.
  Removed the coord_slight module and all calls to it, and obsoleted all
run-time parameters that are exclusively related to it.  This code was an
attempt from 2015 to define an appropriate hybrid vertical coordinate for global
climate modeling, but it never worked very well (usually falling apart in the
second year), and it has not been used in any publication or active model for
many years.  The test case that exercised this coordinate in the MOM6-examples
test suite is also being removed via MOM6-examples PR #388.  The coord_SLight
code is being eliminated altogether now to simplify the MOM6 code base and
reduce the volume of untested and unused code.  All answers in all known MOM6
configurations in active use are bitwise identical, although there is a remote
chance that someone somewhere might be using the SLIGHT coordinate.
  Fixed a bug in MOM_calculate_grad_Coriolis() that was causing the model to
hang due to mismatched halo updates when GLOBAL_INDEXING = True.  Also added
missing callTree (a.k.a. granny tracker) calls at the start and end of the same
routine.  All answers are bitwise identical in any cases that worked before.
Resolving conflict in MOM_CVMix_shear.F90

Conflict due to introduction of changes to Ri smoothing in CVMix in
main, alongside changes to units in comments in dev/gfdl.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.