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

[MPAS-Ocean standalone] Fix bottom depth when filling bathymetry holes #6535

Merged

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Aug 1, 2024

In MPAS-Ocean init mode, when filling holes in the bathymetry (cells with deepest layers that are deeper than any of their neighbors) as part of the Haney-number-constrained vertical coordinate used for ice shelves, the bottom depth was not also being updated correctly for some cells. This merge fixes the issue for cells in the z-level region of the ocean (far from ice-shelf cavities) by setting the bottom depth to the deepest depth within the same z level as the deepest neighbor.

In MPAS-Ocean init mode, when filling holes in the bathymetry
(cells with deepest layers that are deeper than any of their
neighbors) as part of the Haney-number-constrained vertical
coordinate used for ice shelves, the bottom depth was not also
being updated correctly for some cells.  This merge fixes the
issue for cells in the z-level region of the ocean (far from
ice-shelf cavities) by setting the bottom depth to the deepest
depth within the same z level as the deepest neighbor.
@xylar xylar added BFB PR leaves answers BFB MPAS-Ocean standalone Issues and features for standalone MPAS-Ocean code that dont impact E3SM. labels Aug 1, 2024
@xylar xylar requested a review from cbegeman August 1, 2024 09:00
@xylar
Copy link
Contributor Author

xylar commented Aug 1, 2024

Testing

I was able to run dynamic adjustment in Compass with the SORRM r3 mesh (MPAS-Dev/compass#807) using this fix. Without this fix, a single cell had a top layer thickness of ~140 m (compensating for a missing bottom layer), which led to other problems (e.g. a huge spike in temperature in that cell).

Comment on lines 1499 to 1505
if (maxLevelCell(iCell) > maxLevelNeighbors) then
maxLevelCell(iCell) = maxLevelNeighbors
if (smoothingMask(iCell) == 0) then
bottomDepth(iCell) = refBottomDepth(maxLevelNeighbors)
end if
end if
end do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fix makes this treatment within the haney-number coordinate (but outside the "smoothing mask" that defines the region where the haney-number coordinate is actually applied) consistent with the same code in the z-star coordinate used without ice shelf cavities, see:

if (maxLevelCell(iCell) > maxLevelNeighbors) then
maxLevelCell(iCell) = maxLevelNeighbors
bottomDepth(iCell) = refBottomDepth(maxLevelNeighbors)
end if

@xylar
Copy link
Contributor Author

xylar commented Aug 1, 2024

@cbegeman, could you have a quick look at this? I need this fix to make progress on the SORRM mesh. I don't think you need to run any tests, I just think it would be useful to have someone take a quick look at the code and you came to mind.

Copy link
Contributor

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I think this change looks good. Glad you were able to track it down!

It's interesting to me that it is necessary to move bottomDepth up; that it isn't sufficient just to make sure that maxLevelCell ensures continuity with its neighbors to avoid tracer issues. But we can chat about it when time isn't critical. I'm out the rest of the week.

@xylar
Copy link
Contributor Author

xylar commented Aug 1, 2024

It's interesting to me that it is necessary to move bottomDepth up; that it isn't sufficient just to make sure that maxLevelCell ensures continuity with its neighbors to avoid tracer issues. But we can chat about it when time isn't critical. I'm out the rest of the week.

I haven't looked in detail but the issue was that the sum of the layer thicknesses was not equal to ssh + bottomDepth at this particular cell. Oddly, the result (at least after 2 days of integration) was that the top layer became 140 m thick to make up the difference. This evolution presumably had other bad side effects.

Obviously, there should be a sanity check that the sum of layer thicknesses is equal to ssh + bottomDepth to avoid such an issue. I don't particularly want to add that to init mode but it could be a good addition to Compass.

@xylar
Copy link
Contributor Author

xylar commented Aug 1, 2024

@cbegeman, thanks for having a look, by the way!

@xylar
Copy link
Contributor Author

xylar commented Aug 2, 2024

@jonbob, I believe this is ready to go in whenever you have a slot for it. It should be trivial from E3SM's perspective since only MPAS-Ocean init mode is involved.

jonbob added a commit that referenced this pull request Aug 7, 2024
…ext (PR #6535)

Fix bottom depth when filling bathymetry holes

In MPAS-Ocean init mode, when filling holes in the bathymetry (cells
with deepest layers that are deeper than any of their neighbors) as part
of the Haney-number-constrained vertical coordinate used for ice
shelves, the bottom depth was not also being updated correctly for some
cells. This merge fixes the issue for cells in the z-level region of the
ocean (far from ice-shelf cavities) by setting the bottom depth to the
deepest depth within the same z level as the deepest neighbor.

[BFB] MPAS-Ocean standalone only
@jonbob
Copy link
Contributor

jonbob commented Aug 7, 2024

Passes:

  • ERP_Ld3.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-pioroot1
  • ERS_Ld5.T62_oQU120.CMPASO-NYF.chrysalis_intel

merged to next

@jonbob jonbob merged commit 727ad81 into E3SM-Project:master Aug 8, 2024
13 checks passed
@jonbob
Copy link
Contributor

jonbob commented Aug 8, 2024

merged to master

@xylar xylar deleted the ocn/fix-bottom-depth-for-haney-coordinate branch August 8, 2024 14:15
@xylar
Copy link
Contributor Author

xylar commented Aug 8, 2024

Thanks @jonbob!

xylar added a commit to xylar/compass that referenced this pull request Aug 8, 2024
This merge updates the E3SM-Project submodule from [c7d7998](https://github.com/E3SM-Project/E3SM/tree/c7d7998) to [727ad81](https://github.com/E3SM-Project/E3SM/tree/727ad81).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6456
- [ ]  (ocn) E3SM-Project/E3SM#6510
- [ ]  (ocn) E3SM-Project/E3SM#6535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB MPAS-Ocean standalone Issues and features for standalone MPAS-Ocean code that dont impact E3SM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants