Skip to content

(*) Merge GFDL to main (2022-10-27) #229

Merged
alperaltuntas merged 87 commits into
NCAR:dev/ncarfrom
gustavo-marques:merge_main_08Dec22
Dec 14, 2022
Merged

(*) Merge GFDL to main (2022-10-27) #229
alperaltuntas merged 87 commits into
NCAR:dev/ncarfrom
gustavo-marques:merge_main_08Dec22

Conversation

@gustavo-marques
Copy link
Copy Markdown
Collaborator

A full description of what's been merged is provided at mom-ocean#1586

This PR will change answers for the CESM tests because the following parameters were removed from MOM_parameter_doc.all:

  • KDML
  • KVML

In MOM_interface, these parameters were set to KVML = 1.0E-04 m2 s-1 and KDML = 0.0 m2 s-1. I've verified that setting KVML = 0.0 (which is equivalent to what's being done in this PR) does not result in significant changes in a G compset. Differences in key metrics over 10 years are documented in these notebooks.

To accommodate these changes, this PR must be evaluated in conjunction with ESCOMP/MOM_interface#128

jiandewang and others added 30 commits June 22, 2022 08:33
…n-20220602

update to main 20220602 commit
…n-20220603

update to main branch 20220629 commit
…ESMF-managed threading.

authored-by: Gerhard Theurich <gerhard.j.theurich@noaa.gov>
…n-20220729

(1) update to MOM6 main branch 20220729 commit (GFDL-candidate-20220721)  (2) cap update: supporting traditional and ESMF-managed threading
* Change dumbbell initialization
* Change in Dumbbell Layer Mode
* Fix sponge diagnostics
* Fix ALE Sponge Diagnostics
* A minor style change removing spaces around = in optional. function arguments.
* Fix ALE sponge diagnostics
* character declaration fix
  - Ice shelf needs to be initialized prior to
    ocean state initialization. This fixes any cases with
    an ice shelf using the FMScap.
  Added the new runtime parameter CHANNEL_DRAG_MAX_BBL_THICK, to allow the
CHANNEL_DRAG parameterization to use the full dynamically determined depth of
the bottom boundary layer, or to use a fixed maximum thickness, regardless of
the settings of other parameters, although for now the default value is set
based on the settings of USE_JACKSON_PARAM and DRAG_AS_BODY_FORCE in order to
reproduce existing solutions by default.  There are new entries in some
MOM_parameter_doc files.  All answers in the MOM6-examples test suite are
bitwise identical, and this test suite has been tested and works with revised
values of this parameter.
…in-20220820

update to MOM6 20220820 main branch
* Porous barrier variables, which are grouped in porous_barrier_ptrs,
are changed from pointers to allocatables.
* Copies (aliases) of these variables in MOM_CS are removed.
* The name porous_barrier_ptrs is yet to be changed to minimize the
number of files needed to be edited.
* The interface porous_widths is deleted and subroutine por_widths is
renamed as porous_widths.
* porous_barrier_CS is added to control input and diagnostics of the
 module
* Diagnostics for both interface and layer averages weights are added in
subroutine porous_widths.
* An _init subroutine is added to facilitate reading parameters and
registering diagnostics
* checksum debugs are added within subroutine porous_widths.
This commit primarily fixes indexing bugs in subroutine porous_widths.

* Loop range is subroutine porous_widths is changed from data domain to
computation domain.
* find_eta call is now with a proper halo to cover all velocity points.
* Halo update for porous barrier fields is added in step_MOM_*.

Other changes:
* mask_depth, a component of porous_barrier_CS is now used to as
the criterion for masking. This removes the limit that the cell edge
depth has to be below sea surface.
* Output variable eta_cor was unused and is now changed to a local.
* Some unused indexing variables are removed.
parameters

* subroutine set_subgrid_topo_at_vel_from_file is added to read max, min
and avg depth at the edges from file.
* The subroutine is only called when a new runtime parameter
SUBGRID_TOPO_AT_VEL is True. Default is false.
* id_clock is added (as a private variable) to time porous barrier
calculations.
* Some small format fixes
* A runtime parameter PORBAR_ETA_INTERP is added to control different
methods for calculating interface height at the velocity points from
adjacent tracer cells.
* Two small thickness variables are added and scaled the unit of eta.
This is to assit the harmonic mean calculation, but eventually the
if-test eta_s - eta_prev>0 should be relaced by Angstrom.
* Runtime parameter POROUS_BARRIER_MASKING_DEPTH is renamed to make it
a liitlle bit shorter.
* log_version call is added to separate out porous_barriers module in
parameter file.
The main loops in porous_widths are simpified and cleaned up.
* calc_por_layer iss slightly modified to reduced the if-blocks in
the main loops.
* k-loops are moved out.
* To assist this new structure, two 2-D arrays are added to the stack.
* A new function eta_at_uv is added to treat different interpolations.
This is currently done at every point within the loop, and it is
probably adding too much an overhead. It is better pre-calculate
 eta_[uv] all together, which would increase the stack size.
The averaged weight over a layer (por_face_area[UV]) and the weight
at the interface (por_layer_width[UV]) are now calculated
separately, as the only two updates in step_mom are in fact
using only one of them. This reduces
some unnecessary calculations.

* Two new subroutines replaced the original `porous_widths`. They could
be combined with interface if we remove porous_barrier_ptrs type, and
use variables (of different nz) as output.
* There is a place holder switch do_next in the two calc_por subs. This
is used to further reduce calculations for all layers above D_max.
Will be tested and implemented in the next commit.
* A new subroutine calc_eta_at_uv is added to calculate eta at uv.
This simplifies the code, but also increases stack and some performance
overhead.
* A new runtime parameter USE_POROUS_BARRIER is added to control this
feature. The default is True at the moment to assist potential test. It
should be changed to false in the future.
* A boolean array is added to avoid unnecessary porous barrier
 calculations above the shallowest points.
* The layer-averaged weights are bounded by 1.0 explicitly, to avoid
the cases it goes beyond due to roundoff errors.
* For very thin layers (Angstrom), layer-averaged weights are set to
zeros for simplicity.
  * Perhaps it is more reasonable to use the inferace weight for these
  cases.
  * It might be useful to make the thin layer definition a runtime
  parameter.
* A bug related the size of eta_[uv] is fixed.

This commit is potentially answer-changing when the porouss barrier
module is used.
* The recently introduced ANSWER_DATE functionality is used to maintain a
version that should be bit-for-bit reproducible for previous porous barrier
related runs.
* Rename porous_barrier_ptrs to porous_barrier_type
* Some small documentation edits
* Fixed a bug that eta_[uv] was not properly initialized, which caused issues
with global chksums with DEBUG=True.
* Documents added to comply with Doxygen tests
* Fix nudged OBCs for tracers
Failed codecov.io uploads now report the stderr output.

This will allow us to start diagnosing the intermittent failures and,
possibly, find a solution.
herrwang0 and others added 19 commits October 13, 2022 06:01
The actual calculation of SHT is stripped out of the tidal SAL
subroutine to make module MOM_spherical_harmonics self-contained.

* Forward and inverse spherical harmonic transform calculations are
imported from MOM_tidal_forcing module, as two new subroutines are in
module MOM_spherical_harmonics. This generalization allows the
spherical harmonic transforms to applications other than SSH based SAL.
* The main loops in the two new subroutine are also simplified.
* Child variables in the control structure for SHT are now private.
* The associated Legendre polynomials in the diagonal (n=m) is now
precomputed and stored, rather than recalculating at every step. This
does not have the memory cost as fully precomputing all the polynomials,
but still reduces some repeat calculations. The performance appears to
be improved with this change.
* SAL calculation is simplified as forward transform, scaling and
inverse transform.
* A few variables names inherited from MPAS-O are changed.
* A new module is introduced with the sole purpose of storing Love
numbers. This makes module MOM_tidal_forcing less chunkier and easier
to read.

* A new function in module MOM_spherical_harmonics is introduced to
calculate the starting index of the first element for each order m,
replacing a previous function of a similar purpose but a bit more
complicated.
* Option to do reproducing sums for forward spherical harmonic transform
is added, which is controlled by a runtime parameter.

* New timers are added to monitor the performance of the more expensive
global sum.
* Associated Legendre polynomials calculation is vectorized.

* Some redundant variables are removed.

* Fix filename suffix for module MOM_spherical_harmonics
* In MOM_tidal_forcing module, spherical harmonic coefficients (for SAL)
are now parts of tidal_forcing_CS to avoid repeated allocations. The
same applies to the Love number scaling factors.

* Allocations for arrays used for reproducing sums are moved to
subroutine spherical_harmonics_init in the MOM_spherical_harmonics
module.
* Documentations are added for all modules related to SHT SAL.
References from the MPAS-O group will need to be updated once they are
published.

* Variables names are shortened and changed to follow MOM6 style.
(from camel to snake)

* Change RhoE and RhoW in Love number scaling to runtime parameters

* Correct a bug in a_recur size

* Local arrays in SHT subroutines are properly initialized.
* Love number scaling coefficients multiplication is re-ordered to avoid
ambiguity and to follow MOM6 style.
* Coefficients for Pmm is simplified.

Both changes introduce bit-wise level answer change to previous SHT SAL
related unpublished commits.

* Patches for Doxygen
  Started the refactoring of code related to ALE_main, including:

 - Split remap_all_state_vars into remap_tracers and remap_velocities
 - Created the new public subroutine pre_ALE_diagnostics, and call it from
   step_MOM
 - Eliminate the unused argument conv_adjust to regridding_main
 - Eliminated check_grid, and moved these tests into regridding_main
 - Eliminate check_remapping_grid, and replace it with calls directly to
   check_grid_column inside of remapping_main

All answers are bitwise identical, but there are new public interfaces and
changes to the arguments of other public interfaces.
  Continued the refactoring of code related to ALE_main, including:

 - Added the new public subroutine pre_ALE_adjustments
 - Added the new arguments h_new, dzRegrid, and PCM_cell to ALE_main
 - Added the new arguments h_new and dzRegrid to ALE_offline_main
 - Moved the code copying the new thickness to the primary thickness out of
   ALE_main and into step_MOM_thermo

All answers are bitwise identical, but there are new public interfaces and
changes to the arguments of other public interfaces.
  Move the post_data call for dzRegrid to immediately follow its calculation in
regridding main.  All answers and diagnostics are bitwise identical, but because
this commit changes the order in which some diagnostics are posted, the TC
testing falsely reports a failure due to changed diagnostics.
  Replaced ALE_main with ALE_regrid and calls to ALE_remap_tracers and
ALE_remap_velocities from step_MOM_thermo.  Also eliminated ALE_main_offline and
ALE_offline_tracer_final, as they can now be replaced with a calls to ALE_regrid
and a call to ALE_remap_tracers.  Also added unit descriptions to a number of
comments describing variables in MOM_ALE.F90.  All answers are bitwise
identical, but there are 3 new publicly visible subroutines (ALE_regrid,
ALE_remap_tracers, and ALE_remap_velocities) and three previous interfaces
(ALE_main, ALE_main_offline and ALE_offline_tracer_final) have been eliminated.
  Revised or extended comments to correct or more clearly document the units of
49 internal variables in modules in the core directory.  All answers are bitwise
identical.
  Added separate restart variables for some of the oblique OBC restart variables
at north-south or east-west faces.  Before this fix, some of the full 3-d arrays
for restarts were being overwritten for oblique OBC segments that join at
adjacent corners on the north-east side of tracer points, as is noted in
github.com/NOAA-GFDL/issues/208.  The discussion surrounding this issue
confirms that there are cases using oblique OBCs (like some North-West Atlantic
cases) that do not past the restart reproducibility tests.  Some halo updates
were also corrected, in accordance with the introduction of these new variables
and their proper staggering locations.  In a number of places, the proper
case-sensitive horizontal indexing convention, as described in
github.com/NOAA-GFDL/MOM6/wiki/Code-style-guide#soft-case-convention, is now
being used.  This commit also corrected the comments describing a number of the
variables in the OBC_segment_type to make it clearer where they are discretized.

  This changes the names of oblique OBC-related variables in the restart files,
but since the previous version did not reproduce across restarts, there does not
seem to be any point in retaining those incorrect answers.  All answers in the
MOM6-examples test suite are bitwise identical, but this will change (fix)
solutions with oblique OBCs and OBC_RAD_VEL_WT < 1.
  Added two new arrays (OBCmaskCu and OBCmaskCv) to the ocean_grid_type and
dyn_horgrid_type to mask out values over land or at open boundary condition
points.  Without open boundary conditions, these arrays are identical to
mask2dCu and mask2dCv.  These arrays are used in some of the lateral
parameterization modules to zero out certain gradient-dependent fluxes at open
boundary points.  With these changes, the Bering test case solutions no longer
exhibit any dependence on whether DEBUG_OBC is true or false or the value of
OBC_SILLY_THICK, so this commit should help to address some of the issues
discussed in github.com/NOAA-GFDL/issues/208.  All answers are bitwise
identical in the MOM6-examples test cases, but they change for some other tests
that use open boundary conditions more extensively, and there are new arrays in
some transparent types.
  Eliminated OBC arguments that are no longer used by three internal routines in
MOM_lateral_mixing_coeffs.  All answers are bitwise identical.
This patch moves the local `use_tides` flag of the split RK2 solver,
which tracks the TIDES parameter, into the RK2 control structure (CS),
and uses it to conditionally call `tidal_forcing_end().

The `tidal_forcing_init()` function conditionally allocates the
spherical harmonic fields, but `tidal_forcing_end()` is always called,
which will in turn always attempt to deallocate the fields.  In some
compilers, under certain levels of debugging, this can raise a runtime
error.
Testing to see if GH actions is failing due to MPI installation
@gustavo-marques
Copy link
Copy Markdown
Collaborator Author

Cherry-picked 3808641 to fix issue in GH actions due to MPI installation.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 9, 2022

Codecov Report

Base: 37.19% // Head: 37.19% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (3808641) compared to base (1eb6be9).
Patch coverage: 33.79% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           dev/ncar     #229    +/-   ##
==========================================
  Coverage     37.19%   37.19%            
==========================================
  Files           261      263     +2     
  Lines         72392    73147   +755     
  Branches      13550    13625    +75     
==========================================
+ Hits          26925    27206   +281     
- Misses        40470    40929   +459     
- Partials       4997     5012    +15     
Impacted Files Coverage Δ
src/core/MOM_PressureForce_Montgomery.F90 10.71% <0.00%> (ø)
src/core/MOM_continuity.F90 60.71% <ø> (ø)
src/core/MOM_dynamics_unsplit.F90 79.43% <ø> (ø)
src/core/MOM_dynamics_unsplit_RK2.F90 81.06% <ø> (ø)
src/core/MOM_variables.F90 46.96% <ø> (ø)
src/framework/posix.F90 50.00% <0.00%> (-5.27%) ⬇️
src/initialization/MOM_shared_initialization.F90 29.29% <0.00%> (-1.25%) ⬇️
src/parameterizations/lateral/MOM_MEKE.F90 41.92% <ø> (ø)
...parameterizations/lateral/MOM_interface_filter.F90 0.00% <0.00%> (ø)
...ameterizations/lateral/MOM_spherical_harmonics.F90 0.00% <0.00%> (ø)
... and 62 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marshallward
Copy link
Copy Markdown

@gustavo-marques I am going to do an update to main today in a separate PR. That will also fix the macos issue. Should we do it that way rather than cherry-picks in this PR?

@marshallward
Copy link
Copy Markdown

Actually, it looks like the macos change was unneeded here, I was confusing the regression test.

Let's just go with this for now and we'll do things properly next time it happens.

@alperaltuntas
Copy link
Copy Markdown
Member

testing...

@alperaltuntas alperaltuntas merged commit 7f02468 into NCAR:dev/ncar Dec 14, 2022
alperaltuntas added a commit that referenced this pull request Apr 2, 2026
* check-cleanliness CI test

* Oops, forgot to update gitmodules. (DUPLICATION IS EVIL)

* call git-fleximod update before git diff check

* Revert: Oops, forgot to update gitmodules. (DUPLICATION IS EVIL)

* switch to git-fleximod test for hash consistency checking

* update MOM6 submodule but forget to update gitmodules. This should result in CI failure

* revert MOM6 submodule. This should fix in CI failure
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.