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

*+Fix problems in mixedlayer_restrat_Bodner #385

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

Hallberg-NOAA
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA commented Jun 24, 2023

Fixed several problems with the recently added Bodner mixed layer restratification parameterization code.

  • Corrected the dimensional rescaling in the expressions for psi_mag by adding a missing factor of US%L_to_Z.

  • A logical branch was added based on the correct mask for land or OBC points to avoid potentially ill-defined calculations of the magnitude of the Bodner parameterization streamfunction, some which were leading to NaNs.

  • Set a tiny but nonzero default value for MIN_WSTAR2 to avoid NaNs in some calculations of the streamfunction magnitude.

  • Revised the expression for dd within the mu function in a mathematically equivalent way to avoid any possibility of taking a fractional exponential power of a tiny negative number due to truncation errors, which was leading to NaNs in some cases while developing and debugging the other changes that are not included in this commit. This does not appear to change any answers in the existing test cases, perhaps because the mixed layer restratification "tail" is not being activated by setting TAIL_DH to be larger than 0.

  • Corrected or added variable units in comments in the mixedlayer_restrat control structure.

This commit could change answers (and avoid NaNs) in some cases with USE_BODNER23=True, MLE_TAIL_DH > 0 or MLE%TAIL_DH > 0, and there will be changes to the MOM_parameter_doc files for some cases, but given how recently this code was added, it is expected that all answers are bitwise identical in the existing test cases.

  Fixed several problems with the recently added Bodner mixed layer
restratification parameterization code.

- Corrected the dimensional rescaling in the expressions for psi_mag by adding a
  missing factor of US%L_to_Z.

- A logical branch was added based on the correct mask for land or OBC points to
  avoid potentially ill-defined calculations of the magnitude of the Bodner
  parameterization streamfunction, some which were leading to NaNs.

- Set a tiny but nonzero default value for MIN_WSTAR2 to avoid NaNs in some
  calculations of the streamfunction magnitude.

- Revised the expression for dd within the mu function in a mathematically
  equivalent way to avoid any possibility of taking a fractional exponential
  power of a tiny negative number due to truncation errors, which was leading to
  NaNs in some cases while developing and debugging the other changes that are
  not included in this commit.  This does not appear to change any answers in
  the existing test cases, perhaps because the mixed layer restratification
  "tail" is not being activated by setting TAIL_DH to be larger than 0.

- Corrected or added variable units in comments in the mixedlayer_restrat
  control structure.

  These could change answers (and avoid NaNs) in some cases with
USE_BODNER23=True, MLE_TAIL_DH > 0 or MLE%TAIL_DH > 0, and there will be changes
to the MOM_parameter_doc files for some cases, but given how recently this code
was added, it is expected that all answers are bitwise identical in the existing
test cases.
@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working answer-changing A change in results (actual or potential) Parameter change Input parameter changes (addition, removal, or description) labels Jun 24, 2023
@Hallberg-NOAA Hallberg-NOAA requested a review from adcroft June 24, 2023 16:12
@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

Merging #385 (459e1c8) into dev/gfdl (70a75ff) will increase coverage by 0.00%.
The diff coverage is 90.00%.

❗ Current head 459e1c8 differs from pull request most recent head 90306ae. Consider uploading reports for the commit 90306ae to get more accurate results

@@            Coverage Diff            @@
##           dev/gfdl     #385   +/-   ##
=========================================
  Coverage     38.21%   38.22%           
=========================================
  Files           269      269           
  Lines         76403    76407    +4     
  Branches      14031    14033    +2     
=========================================
+ Hits          29201    29203    +2     
- Misses        41960    41961    +1     
- Partials       5242     5243    +1     
Impacted Files Coverage Δ
...ameterizations/lateral/MOM_mixed_layer_restrat.F90 76.80% <90.00%> (-0.16%) ⬇️

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

@marshallward
Copy link
Member

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

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.

Verbally approved to me by @adcroft

@marshallward marshallward merged commit b0289fe into NOAA-GFDL:dev/gfdl Jun 28, 2023
@Hallberg-NOAA Hallberg-NOAA deleted the Bodner_bugfix branch July 26, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answer-changing A change in results (actual or potential) bug Something isn't working Parameter change Input parameter changes (addition, removal, or description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants