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 DENSE_WATER_EAST_SPONGE_SALT #239

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

Hallberg-NOAA
Copy link
Member

Added a missing scale factor in the DENSE_WATER_EAST_SPONGE_SALT get_param call in dense_water_initialize_sponges, and added comments describing the local variables (and their units) throughout the dense_water_initialization module. Comments were added noting that the variable set by DENSE_WATER_SILL_HEIGHT is unused, and that perhaps this should have been DENSE_WATER_SILL_DEPTH. Units arguments were also added to two of the unlogged get_param calls in this module. Without this change, this test case would not reproduce with dimensional rescaling due to a scale factor that was omitted when salinity was being rescaled on May 3, 2022, which became a part of PR #122 to dev/gfdl, but answers should not change when dimensional rescaling of salinities is not used. All answers and output in the MOM6-examples test suite are bitwise identical.

@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #239 (f09350a) into dev/gfdl (6eab2b6) will increase coverage by 0.03%.
The diff coverage is 0.00%.

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

@@             Coverage Diff              @@
##           dev/gfdl     #239      +/-   ##
============================================
+ Coverage     37.05%   37.08%   +0.03%     
============================================
  Files           263      263              
  Lines         73109    73388     +279     
  Branches      13561    13677     +116     
============================================
+ Hits          27088    27219     +131     
- Misses        41108    41136      +28     
- Partials       4913     5033     +120     
Impacted Files Coverage Δ
src/user/dense_water_initialization.F90 0.00% <0.00%> (ø)
config_src/infra/FMS1/MOM_domain_infra.F90 36.11% <0.00%> (+10.05%) ⬆️
src/tracer/MOM_tracer_flow_control.F90 25.00% <0.00%> (+25.00%) ⬆️

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

@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working answer-changing A change in results (actual or potential) labels Nov 15, 2022
  Added a missing scale factor in the DENSE_WATER_EAST_SPONGE_SALT get_param
call in dense_water_initialize_sponges, and added comments describing the local
variables (and their units) throughout the dense_water_initialization module.
The variable set by DENSE_WATER_SILL_HEIGHT was unused and it probably was
always intended to be DENSE_WATER_SILL_DEPTH, which it now is.  Units arguments
were also added to two of the unlogged get_param calls in this module.  Without
this change, this test case would not reproduce with dimensional rescaling due
to a scale factor that was omitted when salinity was being rescaled on May 3,
2022, which became a part of PR mom-ocean#122 to dev/gfdl, but answers should not change
when dimensional rescaling of salinities is not used.  All answers and output in
the MOM6-examples test suite are bitwise identical.
@marshallward
Copy link
Member

marshallward commented Nov 23, 2022

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

Changed parameters did not appear in regression testing.

@marshallward marshallward merged commit 6765e27 into NOAA-GFDL:dev/gfdl Nov 23, 2022
@Hallberg-NOAA Hallberg-NOAA deleted the dense_water_init_fix branch February 2, 2023 13:48
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants