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

+Add and use WAVE_INTERFACE_ANSWER_DATE #295

Merged
merged 9 commits into from
Jan 9, 2023

Conversation

Hallberg-NOAA
Copy link
Member

This PR is a series of commits to enable the use of more robust code in the the MOM_wave_interface module, and to specify runtime parameters for this code. The changes in this PR include:

  • Add the runtime parameter WAVE_INTERFACE_ANSWER_DATE. For now the default (20221231) is to recover the previous settings, but after coordination with the MOM6 community, the default should change to reflect the global DEFAULT_ANSWER_DATE used elsewhere in MOM6.

  • Removed the unused element La_SL from the wave_parameters_CS

  • Add the 3 runtime parameters CHARNOCK_MIN, CHARNOCK_SLOPE_U10 and CHARNOCK_0_WIND_INTERCEPT to specify the curve fit in the Charnock coefficient calculation.

  • Add three new runtime parameters (LANGMUIR_STOKES_BACKGROUND, SURFBAND_MIN_THICK_AVG and SURFBAND_OVERRIDE_LAND_SPEED) to replace previously hard-coded dimensional parameter in the Langmuir number and Stokes drift calculations.

  • Modify Update_Stokes_Drift to use more robust expressions when WAVE_INTERFACE_ANSWER_DATE >= 20230101

  • Modify get_StokesSL_LiFoxKemper to use more robust expressions when WAVE_INTERFACE_ANSWER_DATE >= 20230102

  • Use more efficient expressions in ust_2_u10_coare3p5 when WAVE_INTERFACE_ANSWER_DATE >= 20230103

  • Eliminate the hard-coded wave_parameters_CS StkLevelMode element and associated code

    By default, all answers are bitwise identical, but there are a number of additions to the MOM_parameter_doc files for configurations that have USE_WAVES=True or use statistical waves for Langmuir enhancements to turbulence via USE_LA_LI2016=True. Although this PR was introduced as a series of 7 commits to facilitate the evaluation and review of these changes, it is a good candidate for a squash-merge onto the dev/gfdl branch of MOM6.

    The commits in this PR include:

  • 14b923994 +Eliminate wave_parameters_CS%StkLevelMode

  • 4e000046d +Make Update_Stokes_Drift more robust

  • 111c0e64e +Make get_StokesSL_LiFoxKemper more robust

  • 1e90888a3 +Add WAVE_INTERFACE_ANSWER_DATE runtime parameter

  • 7cf89e0b8 +Add LANGMUIR_STOKES_BACKGROUND runtime parameter

  • 890f84d78 +Add 3 Charnock coefficient runtime parameters

  • 5a5fff3c4 Remove La_SL from wave_parameters_CS

@Hallberg-NOAA Hallberg-NOAA added documentation Improvements or additions to documentation enhancement New feature or request Parameter change Input parameter changes (addition, removal, or description) labels Jan 2, 2023
@codecov
Copy link

codecov bot commented Jan 2, 2023

Codecov Report

Merging #295 (019a84b) into dev/gfdl (ca72fcc) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 019a84b differs from pull request most recent head 6257616. Consider uploading reports for the commit 6257616 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #295      +/-   ##
============================================
- Coverage     37.06%   37.05%   -0.01%     
============================================
  Files           262      263       +1     
  Lines         73618    73698      +80     
  Branches      13727    13731       +4     
============================================
+ Hits          27283    27311      +28     
- Misses        41294    41347      +53     
+ Partials       5041     5040       -1     
Impacted Files Coverage Δ
src/user/MOM_wave_interface.F90 1.24% <0.00%> (-0.06%) ⬇️
src/ALE/regrid_consts.F90 56.25% <0.00%> (ø)
src/framework/MOM_document.F90 73.36% <0.00%> (+0.21%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link

@breichl breichl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion is that the updated answer codes are improvements to the code/algorithms, and the new code does not alter any physical meanings. I made a couple small comments on some suggested variable names for consistency with the traditional drag coefficient definition.

I don't think this next step is necessary for merging this code (since the old answer choice remains), but before rolling forward the default answer date the one change that would be particularly beneficial to evaluate is the second set of reformulations (to the get_StokesSL_LiFoxKemper routine). This code is exercised by OM4, so it would be prudent to evaluate whether the algorithm improvements change any aspects of the simulation. More importantly, the original expressions in that code derive from code that is in CVMix and is used in several other models in the community. So these changes may also be interesting for others.

src/user/MOM_wave_interface.F90 Outdated Show resolved Hide resolved
src/user/MOM_wave_interface.F90 Outdated Show resolved Hide resolved
src/user/MOM_wave_interface.F90 Outdated Show resolved Hide resolved
  Removed the unused element La_SL from the wave_parameters_CS.  Also added or
renamed a few internal variables or amended the comments describing them to
clarify what the code is doing.  All answers are bitwise identical.
  Add 3 runtime parameters (CHARNOCK_MIN, CHARNOCK_SLOPE_U10 and
CHARNOCK_0_WIND_INTERCEPT) to specify the curve fit in the Charnock coefficient
calculation.  By default all answers are bitwise identical, but there are 3 new
runtime parameters in some MOM_parameter_doc.all files.
  Added three new runtime parameters (LANGMUIR_STOKES_BACKGROUND,
SURFBAND_MIN_THICK_AVG and SURFBAND_OVERRIDE_LAND_SPEED) to replace previously
hard-coded dimensional parameter in the Langmuir number and Stokes drift
calculations. By default all answers are bitwise identical, but there are new
runtime parameters in some MOM_parameter_doc.all files.
  Added the new runtime parameter WAVE_INTERFACE_ANSWER_DATE, with a default
value that is temporarily set to use the previous answers.  This is used to
select a more efficient option in ust_2_u10_coare3p5.  The answers with this new
option differ at roundoff, but are otherwise very similar.  By default all
answers are bitwise identical, but there is a new runtime parameters in some
MOM_parameter_doc.all files.
  Modified get_StokesSL_LiFoxKemper to use more robust expressions when
WAVE_INTERFACE_ANSWER_DATE >= 20230102, and reset the value to change the
answers to ust_2_u10_coare3p5 when WAVE_INTERFACE_ANSWER_DATE >= 20230103.  Both
options have been tested independently, and give answers that are similar to but
more robust than the previous expressions.  By default all answers are bitwise
identical, but there is a new use of the runtime parameter
WAVE_INTERFACE_ANSWER_DATE.
  Modified Update_Stokes_Drift to use more robust expressions when
WAVE_INTERFACE_ANSWER_DATE >= 20230101.  This new option may not be as fully
tested as it should be, but it appears to give answers that are similar to but
more robust than the previous expressions.  By default all answers are bitwise
identical, but there is a new use of the runtime parameter
WAVE_INTERFACE_ANSWER_DATE.
  Eliminated the hard-coded wave_parameters_CS StkLevelMode element and
associated code.  Also modified the description of WAVE_INTERFACE_ANSWER_DATE to
describe the meaning of its various settings.  A handful of spelling errors were
also corrected.  All answers are bitwise identical, but there are changes to
some MOM_parameter_doc files.
  Renamed two internal variables to Cd and I_sqrtCd in ust_2_u10_coare3p5 for
greater clarity and corrected their descriptions in comments, following advice
from the review of the pull request that this is a part of.  Also added the
missing units description "nondim" to the get_param call for DHH85_AGE.  All
answers are bitwise identical.
@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17907 ✔️ 🟡

Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on behalf of @breichl

@marshallward marshallward merged commit 78c91cd into NOAA-GFDL:dev/gfdl Jan 9, 2023
@Hallberg-NOAA Hallberg-NOAA deleted the waves_2023_answers branch February 2, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Parameter change Input parameter changes (addition, removal, or description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants