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

new loss term for internal waves: scattering to higher modes #33

Closed
wants to merge 98 commits into from

Conversation

raphaeldussin
Copy link

this term uses the residual from the reflection/transmission coefficients to compute the energy loss due to scattering to higher modes on the slopes.

marshallward and others added 8 commits December 4, 2021 10:15
This patch fixes an error in the `sync` flag of the cpu clocks.

Previously, we would set the sync bit of a flag based on the presence of
sync, rather than testing if the value was true.  This would cause
potential hangs in any clock that set `sync`, including `.false.`.

This patch correctly replaces the single `ibset` call with an if-block
to either `ibset` or `ibclr`.
  Added the public interface unit_no_scaling_init() to the MOM_unit_scaling
module to initialize a non-scaling unit_scale_type without requiring a
param_file_type argument.  This should be useful for certain trivial unit tests,
and it makes it more plausible to make the unit_scale_type arguments mandatory.
As a part of this change, the new internal subroutine set_unit_scaling_combos()
was carved out of unit_scaling_init to avoid code duplication.  Also added
comments describing the effective units of the various scaling factors with the
standard MOM6 notation.  All answers and output are bitwise identical.
  Provide optional US arguments to 4 existing calls to set_grid_metrics() and
MOM_initialize_topography(), enabling these arguments to become mandatory in the
next commit.  In some cases this requires a call to unit_no_scaling_init() or
the removal of a call to rescale_dyn_horgrid_bathymetry().  In addition, a
mis-scaled value was corrected in the initialization of the ODA control
structures private thickness array; this latter issue is not a problem for
Boussinesq models without dimensional rescaling (i.e. the tested cases), but
could change answers in other cases with data assimilation.  All answers in
the MOM6-examples or TC test cases are bitwise identical.
  Made the unit_scale_type arguments non-optional for 28 routines.  These
arguments had been optional in the first place to manage the coordination
between the MOM6 and SIS2 repositories, but SIS2 has been using these optional
arguments for several years now, and they can be made mandatory without imposing
any disruptions.  This change simplifies and clarifies the code.  All answers
and output are bitwise identical.
This patch updates the PNG file of the consortiums in the docs to
include the NASA-GMAO group.
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #33 (76780ae) into dev/gfdl (e2c81e9) will decrease coverage by 0.03%.
The diff coverage is 25.33%.

❗ Current head 76780ae differs from pull request most recent head 6a208f1. Consider uploading reports for the commit 6a208f1 to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl      #33      +/-   ##
============================================
- Coverage     29.06%   29.03%   -0.04%     
============================================
  Files           240      244       +4     
  Lines         71319    71949     +630     
============================================
+ Hits          20731    20892     +161     
- Misses        50588    51057     +469     
Impacted Files Coverage Δ
config_src/drivers/solo_driver/MOM_driver.F90 68.01% <0.00%> (ø)
...fig_src/external/GFDL_ocean_BGC/generic_tracer.F90 0.00% <ø> (ø)
...external/stochastic_physics/stochastic_physics.F90 0.00% <0.00%> (ø)
src/ALE/MOM_regridding.F90 21.36% <0.00%> (-0.10%) ⬇️
src/core/MOM_PressureForce_Montgomery.F90 6.51% <0.00%> (ø)
src/core/MOM_barotropic.F90 37.70% <0.00%> (ø)
src/core/MOM_boundary_update.F90 40.47% <0.00%> (ø)
src/core/MOM_checksum_packages.F90 30.82% <0.00%> (-1.51%) ⬇️
src/core/MOM_forcing_type.F90 43.05% <0.00%> (-4.21%) ⬇️
src/core/MOM_verticalGrid.F90 70.21% <ø> (ø)
... and 98 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2c81e9...6a208f1. Read the comment docs.

Hallberg-NOAA and others added 21 commits December 9, 2021 13:58
  Removed clocks that were being called from inside of j-loops in two modules.
These are inefficient and can cause the model to hang in some cases if used, and
there are better ways to get timing information at this level.  If there is
interest in the timing breakdown at this level, the code should be restructured
to move the key blocks outside of the j-loops.  The run-time parameter
ALLOW_CLOCKS_IN_OMP_LOOPS is no longer being used so it is now obsoleted. All
answers are bitwise identical, but there are changes to some MOM_parameter_doc
files.
  Added a deallocate call for eta_PF_start in step_MOM_dyn_split_RK2() to
avoid a possible memory leak.  All answers are bitwise identical.
  Set find_salt_root even if SHELF_THREE_EQN = .False. to avoid using an
uninitialized logical to determine which parameters are logged.  Without this
the contents of some MOM_parameter_doc.all files could depend on the state of
uninitialized memory and was compiler dependent in some cases.  All answers are
bitwise identical, but in some cases the contents of MOM_parameter_doc files
could be corrected.
  The runtime parameter ETA_TOLERANCE_AUX was being read but was never used, so
it is being obsoleted.  However, because some experiments were using this and
there are effectively no changes in behavior, a warning will be issued instead
of a fatal error if this parameter is set.  All answers are bitwise identical,
but there are changes to some MOM_parameter_doc files.
  Return the diabatic_aux_CSp from extract_diabatic_member it is present as an
optional argument.  Somehow this was omitted when this routine was created, but
without this correction the offline tracer mode returns a segmentation fault.
Also, added the proper conversion factor in the register_diag_field call for
e_predia, and internally calculate the interface heights in units of [Z ~> m]
for dimensional consistency testing.  All answers are bitwise identical in
cases that ran before.
  Issue a warning with a helpful message if opacity_from_chl is called with no
shortwave fluxes, and added logical tests to avoid a segmentation fault later in
this routine.  This should not happen, as it makes no sense, but it was
occurring with the offline tracer code, and can be avoided by setting
PEN_SW_NBANDS=0 if there are no shortwave fluxes to penetrate.  Also turned the
real dimensional parameter op_diag_len into a variable and set it immediately
before where it is used.  Many spelling errors were also corrected in
MOM_opacity.F90.  All answers are identical in cases that ran before.
  Corrected the comments describing the optional arguments to advect_tracer and
fixed a few spelling errors in comments.  All answers are bitwise identical.
The arrays passed to data_overrride need to be sized as the compute
domain. Added indices to pass to data_override.
  Corrected the expressionfs for the por_face_areaU arguments being passed to zonal_face_thickness to
avoid the array out-of-bounds index errors highlighted in MOM6 issue mom-ocean#24. Also
added comments noting where the por_face_area arrays should probably be included
in the effective (relative) face thickness calculations that are later used for
finding the vertically averaged accelerations by the barotropic solver.  All
answers and output are bitwise identical in cases that work, but this should
avoid some run-time or compile-time errors with some compiler settings.
  Corrected bugs in the offline tracer code that were preventing it from
reproducing across processor counts (and perhaps working sensibly at all). All
answers and output in the MOM6-examples regression suite are bitwise identical.
Although answers with the offline tracer code will change because of the bug
fix, because of some bugs that were fixed in another recent commit, it was
previously impossible to have run the offline tracer cases at all.
  Substantially refactored the offline tracer code.  This refactoring includes
adding grid and unit_scale_type arguments to several routines related to the
offline tracer code.  An offline tracer advection test case is now consistent
across processor layouts and pass the dimensional rescaling tests (including the
chksums in debug mode), and there are comments describing all the real variables
and their dimensions in the offline tracer routines.  All answers and output are
bitwise identical.
  Minorly revised the algorithms used for offline tracer advection for
rotational consistency, and for exact reproducibility across PE layouts by using
reproducing sums to detect convergence.  This also includes some changes to use
roundoff to detect convergence instead of fixed values.   Also replaced some
divisions with multiplication by a reciprocal.  In addition, some of the
optional arguments to advect_tracer that are only used for offline tracer
advection were renamed or revised for clarity and reordered for the convenience
of the non-offline-tracer code.

  Although answers with the offline tracer code will change slightly because of
this refactoring, because of some bugs that were fixed in another recent commit,
it was previously impossible to have run the offline tracer cases at all.  All
answers and output in the MOM6-examples regression suite are bitwise identical.
+(*)Make unit_scale_type arguments non-optional
Hallberg-NOAA and others added 27 commits February 15, 2022 16:19
  Added two new modules, the MOM6-specific MOM_check_scaling.F90 and the generic
framework module MOM_scaling_check.F90, to assess the uniqueness of the unit
scaling factors for all of the variables used by MOM6.  If there are overlaps in
scaling factors for different units, this also identifies and suggests alternate
scaling factors with less overlaps.  This commit includes the introduction of
the new publicly visible routines check_scaling_factors(), scales_to_powers()
and check_MOM6_scaling_factors.  This new capability does not do anything for
sufficiently low levels of model verbosity, and it is silent if the scaling
factors are unique, or if less than 2 dimensions are being rescaled.  All
answers and output files are bitwise identical, but there can be additional
messages to stdout.
  Renamed the framework module MOM_scaling_check.F90 to MOM_unique_scales.F90 to
help differentiate it from MOM_check_scaling.F90, and renamed the subroutine
check_scaling_factors() as check_scaling_uniqueness().  Also added
_Dimensional_consistency.dox to describe the dimensional consistency testing.
This commit should address the issues raised in the review of MOM6 PR mom-ocean#49.  All
answers and output are bitwise identical.
Pointers to the diabatic driver's energetic PBL field are now only
associated when `use_energetic_PBL` is true.

The `energetic_PBL` field was also renamed to `ePBL` to avoid potential
conflict with the `energetic_PBL` subroutine.

Thanks to Alper Altuntas for detecting this issue and the proposed fix.

Co-Authored-By: Alper Altuntas <alperaltuntas@gmail.com>
  Corrected memory declarations in MOM_regridding.F90 in cases where the
vertical size of the input and output grids are not the same.  Although this is
not know to have caused any particular problems, these inconsistencies could
lead to segmentation faults in cases where the target grid (e.g., diagnostic
output) is larger than the input grid (e.g., the model's native grid).  In some
cases, certain grid generation options were only written to work with the same
size of input and output grids, and error handling has been added to these cases
to gracefully bring down the model if they are used with different grid sizes.
All answers are bitwise identical in the MOM6-examples test suite, but it is
conceivable that this could correct subtle (memory-related) issues in some
configurations.
 - default behavior returns field size using fms1 format.
…om-ocean#65)

* Eliminate GET_ALL_PARAMS in hor_visc_init

  Added do_not_log arguments to get_param calls in MOM_hor_visc.F90 that are
only used conditionally, and eliminated the unlogged GET_ALL_PARAMS runtime
parameter and get_all variable in hor_visc_init().  By design, all logging of
parameters after this commit is identical to before, even for variables that are
inactive and therefore should not be logged.  In several places, there were some
problems, mostly with the GME code, that have been noted in comments marked with
'###'.  Also cleaned up the code alignment and eliminated unneeded temporary
variables in a few places in hor_visc().  All solutions are bitwise identical,
and no output is changed.

* Move call to get get_KH outside k-loop

The call to get the 3-d GME diffusivity arrays and the subsequent
blocking halo update was moved outside of the k-loop.

* Increase loop range for calculation of GME fluxes

* Makes GME filter rotationally invariant

* Makes the GME filter rotationally invariant
* Adds a runtime param to allow the user to control how many
smoothing passes should be done.

* Rearranges the get_param calls related to Leith

The get_param calls for Leith were not in the correct location.
This commit fixes that.

* Adding halo updates

* Fixes do loops indices and adds diagnostics

* Adds option to save barotropic tension and strain;

* Fixes many i and j loops indices associated with GME to avoid halo
problems and unnecessary halo updates. With these changes, the
model is now conserving mass and tracers when USE_GME = True.

* Fixes issues related to the merging with dev/gfdl

* Fixes calculation of FrictWork_GME

The calculation now mimics the calculation of FrictWork and it
includes the energy diffusion term.

* Removes dependency of FrictWork_GME calculation on MEKE

* Adding sh_xy_sq and sh_xx_sq in the OMP directives

Co-authored-by: Robert Hallberg <Robert.Hallberg@noaa.gov>
Subroutine adjustEtaToFitBathymetry had a hard-wired parameter
(hTolerance = 0.1) controlling the tolerance when adjusting the
thickness to fit the bathymetry. This patch adds an user-controlled
parameter (THICKNESS_TOLERANCE), which replaces hTolerance.
THICKNESS_TOLERANCE is only activated when ADJUST_THICKNESS=True.
  Actually calculate the mean temperature and salinity reported by
MOM_state_stats().  Due to an oversight, these means were always being reported
as 0.  This changes the output when the debugging flag DEBUG_CONSERVATION=True.
All answers are bitwise identical.
  Added the new function global_mass_int_EFP(), which is analogous to
global_mass_integral but returns its result in extended fixed point (EFP_type)
format and always uses reproducing sums, to facilitate layout-invariant
global integrals but with the potential for deferred global reductions so that
this last step can be combined for various global reductions for efficiency.
All answers are bitwise identical, but there is a new public interface.
  Use global_mass_integral for the debugging diagnostics of the tracer amounts
before and after diffusion in lateral_boundary_diffusion, and replaced a call to
write(*,*) with a call to MOM_mesg to actually write the message.  The
global_mass_integral uses reproducing sums, and is invariant to layout, while
MOM_mesg is preferable for output because it will allow us to more cleanly
control how output is handled and which processors do the writing.  All
solutions are bitwise identical, although some debugging output will change.
  Use reproducing sums for tabulating tracer stocks, and move the global sum for
the tracer stocks form write_energy into call_tracer_stocks.  This involves
changes to the type of an argument (from real to EFP_type) for two arguments to
the internal routine store_stocks.  Existing tracer stock packages will still
work, but to benefit from the reproducing sums, they will also have to change
their reported values from real to EFP_type.  This is demonstrated for two
packages (advection_test_tracer and ideal_age_example), where the stocks are now
found with calls to global_mass_int_EFP(), replacing the previous explicit
sums.  With this change, the reported stock values from these packages are
identical for different PE layouts and can be much more accurate than before,
but they are different from the previously reported values at roundoff (for
positive-definite tracers), but it could be larger for tracers with a near-zero
mean value.  All solutions are bitwise identical, but output changes.
  Modified the remaining tracer packages to use the reproducing stocks.  The
reported stock values from these packages will have changed slightly, but they
now reproduce across PE layouts.  All solutions are bitwise identical, but
output changes.
  Removed trailing white space on two lines.  All answers are bitwise identical.
This patch includes several minor changes to the MOM_file_parser and
supporting modules in order to accommodate stronger unit testing.

It includes the following API changes:

- Removal of `static_value` from `get_param`

- Redefined `link_parameter` and `parameter_block` as private

- New functions: `all_across_PEs()`, `any_across_PEs()`

`static_value` was not used in any known experiments (outside of
internal GFDL testing), and the two derived types describe internal
operations within `MOM_file_parser`, so we do not expect any disruptions
from these changes.

A detailed summary of the changes are listed below.

- `assert()` is now used to detect same files with different IO units.

  Detection of reopenend files of the same name but different IO unit
  has been changed from `MOM_error(FATAL, ...)` to `assert()`, to
  reflect that this should be a logically impossible result.

- Bugfix: Reopened files are now reported to all PEs.

  If an open file is re-opened, then only the root PE will detect this
  and will `return` immediately.  However, the others will proceed into
  `populate_param_data` and will get stuck in a broadcast waiting for
  root.

  We fix this by communicating the reopened state to all PEs and allow
  all ranks to return before re-processing the data.

  Note that this could also be resolved by allowing all ranks to track
  IO unit numbers, but for now we do not attempt to change this
  behavior.

- `newunit=` used to generate parameter file IO unit

  The parameter IO unit is now generated by `newunit=` rather than an
  explicit search for an unused IO unit.  Note that this is a Fortran
  2008 feature.

  Testing around available IO units has also been removed.

- Removal of generic IO error handling

  Generic "IO error" tests, and corresponding `err=` arguments, have
  been removed in most cases.  We now rely on the Fortran runtime to
  provide diagnostics on these errors, which should typically exceed any
  information that MOM6 could provide.

- Removal of purported `namelist` support

  There were several blocks of code provided to support namelist syntax,
  but did not appear to be working, nor was there any known instance of
  it being used by anyone, so it has been removed.

- `#define/undef/=` syntax testing across ranks

  Previously, only the root PE would test for consistency of the
  #define-like syntax, even though all ranks have this information.
  This required a second, awkwardly placed syntax test later in the
  subroutine.

  This test is redefined to run over all ranks, and the subsequent test
  has been removed.

- `define/override` test reordering

  The `found_override` test when coupled to a `#define`-like declaration
  was unreachable due to the presence of an even stronger test related
  to valid syntax.

  This test has been moved to provide more detailed information about
  the nature of the error.

- `link_parameter`, `parameter_block` defined as private

  Internal derived types of `MOM_file_parser` are redefined as private.
  This preserves the integrity of instances of these types, and also
  prevents creation of implicit object code required to access them
  externally.

- Removal of `static_value` from `get_param` interface

  The `static_value` argument of `get_param` has been removed, since it
  is functionally equivalent to `default`.  While this is an API change,
  there is no known case of anyone using this argument.

- The `param_type%doc` fields are now properly deallocated after closed.

- Quotes have been added around some filename error warnings, to help
  detect issues related to whitespace.

- `any_across_PEs` and `all_across_PEs`

   New functions for calling `any()` and `all()` across PE ranks have
   been added.  Behavior is in line with other functions, such as
   `min_across_PEs`.
  Added a number of options to MOM_ALE to mimic options that are found in
Hycom.  By default, all answers are bitwise identical, but there are several
new runtime parameters.  The code changes include:

 . Added the ability to use a different remapping scheme for velocities than
   for tracers, using the new runtime parameter VELOCITY_REMAPPING_SCHEME

 . Added the new runtime option PARTIAL_CELL_VELOCITY_REMAP to use partial cell
   thicknesses for remapping at velocity points, which triggers a call to the
   new internal routine apply_partial_cell_mask.

 . Added the new internal routine mask_near_bottom_vel to allow MOM6 to mimic
   Hycom in zeroing out the velocities in thin layers in a bottom boundary
   layer with a thickness given by the new runtime parameter
   REMAP_VEL_MASK_BBL_THICK, while the definition of thin is specified by
   REMAP_VEL_MASK_H_THIN.  Setting these to be negative (as is the default)
   avoids this.

 . Modified the interface to remap_all_state_vars to take just the ALE_CS, which
   then provides the remapping control structure that is appropriate for the
   tracers or velocities, rather than also passing this in as an argument.

 . Eliminated some unnecessary internal variables, and added others to be
   more explicit avoid array syntax copies in arguments.
  Enforce a minimum thickness of 0.5*Angstrom in the mixed_layer_restrat
routines.  The streamfunctions in these routines attempt to limit the
thicknesses to exceed Angstrom, but they can be less than this due to roundoff.
The new limit prevents thicknesses from becoming negative when Angstrom is set
to 0, but should not change any answers for test cases with positive values of
Angstrom.  Also added some comments describing variables and their units and
simplified the OMP directives.

  Also corrected error messages in MOM_diabatic_aux.F90 to identify the file or
module where these messages come from, and modified an error message in
applyTracerBoundaryFluxesInOut so that it is written if the localized fault does
not happen to occur on the root PE.

  All answers in the existing MOM6-examples regression suite are bitwise
identical.
@raphaeldussin
Copy link
Author

this branch has history issues, closing and will re-open with a clean branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.