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

Rescale internal variables in reset_face_lengths_list #276

Merged

Conversation

Hallberg-NOAA
Copy link
Member

Apply dimensional rescaling immediately after reading the open face width and porous barrier depth variables in reset_face_lengths_list() to facilitate the detection of improperly scaled variables or incorrect unit declarations throughout the code. Also eliminated unnecessary local copies of rescaling factors in the MOM_porous_barriers module. All answers are bitwise identical.

  Apply dimensional rescaling immediately after reading the open face width and
porous barrier depth variables in reset_face_lengths_list to facilitate the
detection of improperly scaled variables or incorrect unit declarations
throughout the code.  Also eliminated unnecessary local copies of rescaling
factors in the MOM_porous_barriers module.  All answers are bitwise identical.
@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #276 (e516640) into dev/gfdl (7869030) will decrease coverage by 0.00%.
The diff coverage is 9.52%.

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

@@             Coverage Diff              @@
##           dev/gfdl     #276      +/-   ##
============================================
- Coverage     37.10%   37.09%   -0.01%     
============================================
  Files           263      263              
  Lines         73582    73586       +4     
  Branches      13718    13718              
============================================
- Hits          27299    27294       -5     
- Misses        41249    41257       +8     
- Partials       5034     5035       +1     
Impacted Files Coverage Δ
src/initialization/MOM_shared_initialization.F90 28.82% <0.00%> (-0.38%) ⬇️
src/core/MOM_porous_barriers.F90 31.25% <100.00%> (-1.53%) ⬇️
src/framework/MOM_document.F90 74.33% <0.00%> (-0.23%) ⬇️

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

@Hallberg-NOAA Hallberg-NOAA added the enhancement New feature or request label Dec 18, 2022
Copy link

@sditkovsky sditkovsky left a comment

Choose a reason for hiding this comment

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

These changes seem reasonable to me. It makes sense to perform the MOM_porous_barriers in Z rather than eta (especially since Z_to_eta was hardcoded to 1.0 anyway. Great that that is removed).

The changes to reset_face_lengths_list() are also reasonable. Much cleaner to rescale the variables at read-in than during the calculations.

@marshallward
Copy link
Member

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

@marshallward marshallward merged commit 747eeff into NOAA-GFDL:dev/gfdl Dec 23, 2022
@marshallward
Copy link
Member

Approved on behalf of @sditkovsky

@Hallberg-NOAA Hallberg-NOAA deleted the rescale_face_lengths_list branch February 2, 2023 13:41
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.

3 participants