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

Main to dev #1529

Merged
merged 9 commits into from
Oct 20, 2021
Merged

Main to dev #1529

merged 9 commits into from
Oct 20, 2021

Conversation

marshallward
Copy link
Collaborator

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

Typo fix in parameter doc.

klindsay28 and others added 9 commits October 9, 2021 19:50
…date-2021-10-04

correct long_name for tracer_dfy for passive tracers when diag_form == 1
PRs mom-ocean#1428 and mom-ocean#1457 extended the topography clipping to allow flooding
but missed the use case for positive-only depths where the MASKING_DEPTH
parameter alone was in use. There were two bugs:

1. The new code assumed that MINIMUM_DEPTH would be deeper than
   MASKING_DEPTH (which is intuitive). However, the point of MASKING_DEPTH
   was only to specify the determination of the land mask. The new code
   assigned depths the value of MASKING_DEPTH which broke cases that were
   using MASKING_DEPTH as documented and were leaving MINIMUM_DEPTH=0.

2. The values of variable masking_depth were altered and subsequently
   not consistent with the logged parameters. A warning was issued but
   the behavior was nevertheless not as intended.

Changes:
1. Removed the test that masking_depth > min_depth, and warning
2. Adjusted the condition and assigned value when clipping depths.
   This now uses the shallower of min_depth and masking_depth to decide
   when to clip and for the value to use otherwise.
   The expression for the land mask is unaltered.
3. Corrected documentation to retain original purpose of MASKING_DEPTH
4. Added some comments for declaration with units
5. Added some clarifying comments in code

Todo:
- resolve the need for the alternative negative depth pathway associated
  with the 0.5*min_depth expression.
- @klindsay28 spotted two issues for the NW2 tracers
  1. Use of the wrong logical variable
  2. Incorrect comment
- Using the wrong logical meant that the ideal age pacakge was being
  called in addition to the NW2 package, but did not affect the NW2
  tracers themselves.
- Following feedback from @herrwang0, we have removed the possibility
  for a user to try using negative depths without the MASKING_DEPTH
  parameter being set appropriately. This avoids the asymmetric use
  of MINIMUM_DEPTH that was proposed. A FATAL is now issued.
- Corrected a spelling error in a comment.
- Removed an unused "use" that should have been done in previous commit.
Recover topography clipping when not specifying MINIMUM_DEPTH
Correct logical associated with NW2 tracers
…ate-2021-10-04

Dev gfdl main candidate 2021 10 04
@marshallward
Copy link
Collaborator Author

Overriding review requirement, since content has already been reviewed.

Doing a rebase merge to flush out all the "merge pull request" intermediate commits.

@marshallward marshallward merged commit 5d82809 into mom-ocean:dev/gfdl Oct 20, 2021
@marshallward marshallward deleted the main_to_dev branch October 20, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants