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

Update global ocean init mode for meshes with ice-shelf cavities #6375

Merged

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Apr 29, 2024

This merge updates MPAS-Ocean init mode for global_ocean in several ways that are critical for initialization that is compatible with planned coupling to MALI.

The fields landIcePressure, landIceDraft and ssh are now read from a topography file. This means that landIcePressure is no longer computed based on the density at the ocean surface in init mode. Similarly, ssh is not required to be the same as landIceDraft.

Support is removed for reading land-ice topography fields from a dataset on a lat-lon grid. We no longer use this capability and maintaining it is not worth the effort.

The land-ice topography data set is now required to have landIceFloatingFracObserved as a field, and this is used to set landIceFloatingFraction. Previously, landIceFracObserved was being used to compute both landIceFraction and landIceFloatingFraction (assuming no grounded ice in the domain).

We no longer set landIceFraction and landIceFloatingFraction to zero when landIceMask and landIceFloatingMask, respectively, are zero. This is in order to allow melting in cells that are less than 50% land ice in order to better support conservation between ice sheet and ocean.

In addition, we use sshAdjustmentMask instead of landIceMask as the starting point for smoothing mask used to produce the Haney-number vertical coordinate. This is needed because otherwise locations with nonzero ice draft get missed and the Haney number may be too high for model stability.

@xylar xylar added mpas-ocean BFB PR leaves answers BFB MPAS-Ocean standalone Issues and features for standalone MPAS-Ocean code that dont impact E3SM. labels Apr 29, 2024
@xylar xylar requested review from mark-petersen and cbegeman April 29, 2024 15:58
@xylar
Copy link
Contributor Author

xylar commented Apr 29, 2024

This has been moved from E3SM-Ocean-Discussion#90, which includes relevant discussion.

@xylar
Copy link
Contributor Author

xylar commented Apr 29, 2024

@mark-petersen, you are the best person I could think of to sanity check changes to init mode. Let me know if you can do that.

@xylar
Copy link
Contributor Author

xylar commented Apr 30, 2024

@mark-petersen, this is a high priority for the FAnSSIE project. Can you review when you can?

@xylar
Copy link
Contributor Author

xylar commented Apr 30, 2024

Sorry, I forgot I already pinged you...

@xylar xylar force-pushed the ocn/init-land-ice-pressure-from-topo branch 2 times, most recently from cb9ca7e to f17151a Compare May 8, 2024 22:10
@xylar xylar removed the request for review from mark-petersen May 14, 2024 15:00
@xylar
Copy link
Contributor Author

xylar commented May 14, 2024

@sbrus89, @mark-petersen is away and I need someone to review this that knows about MPAS-Ocean's global ocean init mode. You're the best candidate. Would you be up for reviewing this? If not, remove yourself as a reviewer and I'll find someone else.

@xylar xylar requested a review from sbrus89 May 14, 2024 15:01
@sbrus89
Copy link
Contributor

sbrus89 commented May 15, 2024

@xylar, sure I will take a look.

@mark-petersen mark-petersen self-requested a review May 28, 2024 14:36
@rljacob
Copy link
Member

rljacob commented May 30, 2024

@sbrus89 please review.

@xylar
Copy link
Contributor Author

xylar commented Jun 5, 2024

@mark-petersen, @cbegeman and @sbrus89, a reminder that I would appreciate a review of this PR. This would allow me to make important changes to how meshes with ice-shelf cavities are initialized in MPAS-Dev/compass#813. This, in turn, is needed for me to make progress on coupling to MALI.

xylar added 6 commits June 10, 2024 02:47
We do not use this mode anymore and it is too difficult to maintain.
This merge uses the `landIceDraftObserved`, `landIcePressureObserved`
and `landIceFloatingFracObserved` to initialize the corresponding
MPAS-Ocean variables.  Previously, the `landIcePressure` was being
computed in a subroutine and the `landIceFloatingFraction` was
required to be identical to `landIceFraction`.

This merge also removes the unused variable
`landIceGroundedFracObserved`
Instead, it will be read in from the topography file directly,
meaning it can be updated (e.g. from ssh adjustment).
this is a more accurate description for how it is uses, since it
can determine where we modify either ssh or landIcePressure,
depending on the approach.
@xylar xylar force-pushed the ocn/init-land-ice-pressure-from-topo branch from f17151a to 2389f6f Compare June 10, 2024 07:47
This could allow us to choose to have melting bleed into cells that
are partly open ocean (e.g. for conservation when coupling to MALI).
Comment on lines +1030 to +1033
! don't mask these because we may want to include melt that bleeds
! into the open ocean
landIceFraction(iCell) = landIceFracObserved(iCell)
landIceFloatingFraction(iCell) = landIceFloatingFracObserved(iCell)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case anyone has been reviewing this already, I just added the commit to change this. I think we don't want to hard-code this masking here because we want some flexibility about how to allow fluxes to bleed into cells that are partly open ocean and partly ice. This will be important for conservation when coupling with MALI (without requiring rerouting to cells the ocean decides are under land ice, which I think we want to avoid). I don't see an clear harm in at least supporting this possibility.

This is needed because otherwise locations with nonzero ice draft
get missed and the Haney number may be too high for model stability.
@cbegeman
Copy link
Contributor

In addition, we use sshAdjustmentMask instead of landIceMask as the starting point for smoothign mask used to produce the Haney-number vertical coordinate. This is needed because otherwise locations with nonzero ice draft get missed and the Haney number may be too high for model stability.

Can you explain when this arose? Were these locations where landIceDraft > 0 and landIceMask = 0 ones where landIceFraction was less than the threshold?

@cbegeman
Copy link
Contributor

@xylar I tested this PR@e0c0c0755b with MPAS-Dev/compass#813 against compass/main@1588d78416 and E3SM/master@470a42bdcb. I'm not sure whether this would change with your latest commit but I was seeing maxDeltaSSH of 0.3m with this PR and maxDeltaSSH of 0.02m with the baseline (landIcePressure is the adjusted variable) for the IcoswISC240 test at the last ssh adjustment iteration. Is this similar to what you've been seeing?

@xylar
Copy link
Contributor Author

xylar commented Jun 12, 2024

In addition, we use sshAdjustmentMask instead of landIceMask as the starting point for smoothing mask used to produce the Haney-number vertical coordinate. This is needed because otherwise locations with nonzero ice draft get missed and the Haney number may be too high for model stability.

Can you explain when this arose? Were these locations where landIceDraft > 0 and landIceMask = 0 ones where landIceFraction was less than the threshold?

Yes, there are many such locations around the peninsula. They may have been avoided or the topography was different when grounded ice being masked out of the interpolation process but now that I am including it (as we will be doing with MALI), I am seeing places where not only is the landIcePressure nonzero far from places where landIceFraction > 0.5 but also the Haney number in some of these locations is huge if we use a z-star coordinate so we need to include these regions in the haney-number coordinate as well.

For the new SORRM mesh (MPAS-Dev/compass#807):

Here is the landIceMask (where landIceFraction > 0.5):
land_ice_mask

Here is the sshAdjustmentMask (where landIcePressure > 0):
ssh_adj_mask

Here is the smoothing mask before (seeded from landIceMask):
smoothing_mask_before

Here is the resulting Haney Number:
rx1_max_before

Here is the smoothing mask after the change (seeded from sshAdjustmentMask):
smoothing_mask_after

And the resulting Haney Number (now capped at 20, as required):
rx1_max_after

@xylar
Copy link
Contributor Author

xylar commented Jun 12, 2024

I'm not sure whether this would change with your latest commit but I was seeing maxDeltaSSH of 0.3m with this PR and maxDeltaSSH of 0.02m with the baseline (landIcePressure is the adjusted variable) for the IcoswISC240 test at the last ssh adjustment iteration. Is this similar to what you've been seeing?

I haven't looked at that. My plan is to bring this in in conjunction with MPAS-Dev/compass#813 so I wasn't trying to compare with the current compass main branch. But I would not be surprised if this changes maxDeltaSSH (both with main and with MPAS-Dev/compass#813).

@xylar
Copy link
Contributor Author

xylar commented Jun 12, 2024

@cbegeman, thanks very much for looking at this and for your testing.

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.

Thanks for the additional explanation and figures in response to my comments. I think the changes here look good. Thanks for all your work on this!

@cbegeman
Copy link
Contributor

I'm not sure whether this would change with your latest commit but I was seeing maxDeltaSSH of 0.3m with this PR and maxDeltaSSH of 0.02m with the baseline (landIcePressure is the adjusted variable) for the IcoswISC240 test at the last ssh adjustment iteration. Is this similar to what you've been seeing?

I haven't looked at that. My plan is to bring this in in conjunction with MPAS-Dev/compass#813 so I wasn't trying to compare with the current compass main branch. But I would not be surprised if this changes maxDeltaSSH (both with main and with MPAS-Dev/compass#813).

Agreed. It's hard to tell what changes are related to this PR and the new topography file. Since it doesn't crash, it seems to have been adjusted enough.

@xylar
Copy link
Contributor Author

xylar commented Jun 12, 2024

@cbegeman, thanks so much for reviewing!

@rljacob
Copy link
Member

rljacob commented Jun 13, 2024

@sbrus89 and @mark-petersen need your reviews.

@mark-petersen
Copy link
Contributor

I compiled this with gnu (debug and optimized) on chicoma and intel on chrysalis and tested with the nightly test suite.

There are machine-level diffs on the QU240 init, which are negligible.

pwd
/lustre/scratch5/mpeterse/runs/n/ocean_model_240620_266c9d69_ch_gfortran_openmp_SSHinit

grep 'l1:' -B 1 case_outputs/*init*log
temperature          Time index: 0
0:  l1: 4.44089209850063e-16  l2: 4.44089209850063e-16  linf: 4.44089209850063e-16
--
layerThickness       Time index: 0
0:  l1: 5.11590769747272e-13  l2: 4.58286294150329e-13  linf: 4.54747350886464e-13

Tested QUwISC240 using this branch and MPAS-Dev/compass#813 together, with gnu debug, and it is successful.

Next I will test IcoswISC240 and rerun the Arctic 3km mesh initialization I set up for the Aerosi project.

@mark-petersen
Copy link
Contributor

Results using this branch and MPAS-Dev/compass#813 together, with gnu debug.

output:


ocean/global_ocean/QUwISC240/mesh
  * step: base_mesh
  * step: remap_topography
  * step: cull_mesh
  test execution:      SUCCESS
  test runtime:        01:33
ocean/global_ocean/QUwISC240/WOA23/init
  * step: 01_init_with_draft_from_constant_density
  * step: 02_ssh_from_surface_density
  * step: 03_init
  * step: 04_adjust_ssh
  * step: 05_init
  * step: 06_adjust_ssh
  * step: 07_init
  * step: 08_adjust_ssh
  * step: 09_init
  * step: 10_adjust_ssh
  * step: initial_state
  * step: remap_ice_shelf_melt
  test execution:      SUCCESS
  test runtime:        04:32
ocean/global_ocean/QUwISC240/WOA23/performance_test
  * step: prognostic_ice_shelf_melt
  * step: data_ice_shelf_melt
  test execution:      SUCCESS
  test runtime:        00:53
ocean/global_ocean/IcoswISC240/mesh
  * step: base_mesh
  * step: remap_topography
  * step: cull_mesh
  test execution:      SUCCESS
  test runtime:        01:31
ocean/global_ocean/IcoswISC240/WOA23/init
  * step: 01_init_with_draft_from_constant_density
  * step: 02_ssh_from_surface_density
  * step: 03_init
  * step: 04_adjust_ssh
  * step: 05_init
  * step: 06_adjust_ssh
  * step: 07_init
  * step: 08_adjust_ssh
  * step: 09_init
  * step: 10_adjust_ssh
  * step: initial_state
  * step: remap_ice_shelf_melt
  test execution:      SUCCESS
  test runtime:        04:21
ocean/global_ocean/IcoswISC240/WOA23/performance_test
  * step: prognostic_ice_shelf_melt
  * step: data_ice_shelf_melt
  test execution:      SUCCESS
  test runtime:        00:53
Test Runtimes:
01:33 PASS ocean_global_ocean_QUwISC240_mesh
04:32 PASS ocean_global_ocean_QUwISC240_WOA23_init
00:53 PASS ocean_global_ocean_QUwISC240_WOA23_performance_test
01:31 PASS ocean_global_ocean_IcoswISC240_mesh
04:21 PASS ocean_global_ocean_IcoswISC240_WOA23_init
00:53 PASS ocean_global_ocean_IcoswISC240_WOA23_performance_test
Total runtime 13:44

@xylar
Copy link
Contributor Author

xylar commented Jun 21, 2024

Thanks very much @mark-petersen for reviewing!

@xylar
Copy link
Contributor Author

xylar commented Jun 21, 2024

@mark-petersen, could you approve the PR if you're happy?

@xylar
Copy link
Contributor Author

xylar commented Jun 21, 2024

@sbrus89, will you have time to review this very soon? If not, I'll probably just go forward with the reviews I have.

Copy link
Contributor

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

I've looked this over and the changes look good to me. I'm approving based on testing from @mark-petersen and @cbegeman.

@xylar
Copy link
Contributor Author

xylar commented Jun 21, 2024

Thank you, @sbrus89!

@mark-petersen
Copy link
Contributor

I'm testing with the new Arctic mesh for the Aerosi project, which is the RRS6to18 with a 3km Arctic. I'm having trouble with the init/adjust_ssh step, as the 04_adjust_ssh is unstable. I'm sure I just need to adjust my flags for this resolution, so I am trying that.

If these steps already ran:

01_init_with_draft_from_constant_density  02_ssh_from_surface_density  03_init

is it possible to just run the 04_adjust_ssh step, so I don't need to wait again? Thanks.

Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

I was able to run the IcoswISC240 through mesh generation, init, and performance test on chicoma with gnu. I set up and started running RRS6to18 and my new Arctic 3km mesh. I had some problems with that last one, but it is just my flag settings, which I can adjust myself.

Thanks for your work on this! I approve.

jonbob added a commit that referenced this pull request Jun 24, 2024
#6375)

Update global ocean init mode for meshes with ice-shelf cavities

This merge updates MPAS-Ocean init mode for global_ocean in several ways
that are critical for initialization that is compatible with planned
coupling to MALI:
* The fields landIcePressure, landIceDraft and ssh are now read from a
  topography file. This means that landIcePressure is no longer computed
  based on the density at the ocean surface in init mode. Similarly, ssh
  is not required to be the same as landIceDraft.
* Support is removed for reading land-ice topography fields from a
  dataset on a lat-lon grid. We no longer use this capability and
  maintaining it is not worth the effort.
* The land-ice topography data set is now required to have
  landIceFloatingFracObserved as a field, and this is used to set
  landIceFloatingFraction. Previously, landIceFracObserved was being
  used to compute both landIceFraction and landIceFloatingFraction
  (assuming no grounded ice in the domain).
* We no longer set landIceFraction and landIceFloatingFraction to zero
  when landIceMask and landIceFloatingMask, respectively, are zero. This
  is in order to allow melting in cells that are less than 50% land ice
  in order to better support conservation between ice sheet and ocean.
* In addition, we use sshAdjustmentMask instead of landIceMask as the
  starting point for smoothing mask used to produce the Haney-number
  vertical coordinate. This is needed because otherwise locations with
  nonzero ice draft get missed and the Haney number may be too high for
  model stability.

[BFB] mpas-ocean standalone feature
@jonbob
Copy link
Contributor

jonbob commented Jun 24, 2024

Passes:

  • ERS_Ld5.T62_oQU120.CMPASO-NYF.chrysalis_intel
  • SMS_D_Ld1.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF.chrysalis_intel.mpaso-jra_1958

merged to next

@xylar
Copy link
Contributor Author

xylar commented Jun 24, 2024

is it possible to just run the 04_adjust_ssh step, so I don't need to wait again? Thanks.

@mark-petersen, yes, you can run that step on its own by going into its directory (adjust_ssh/04_adjust_ssh or whatever it's called) and submitting the job_script.sh. You can skip several steps by editing WOA23/init/init.cfg so that the steps you want to skip are not in steps_to_run and then submitting job_script.sh in init.

@jonbob jonbob merged commit 31f771c into E3SM-Project:master Jun 25, 2024
13 checks passed
@jonbob
Copy link
Contributor

jonbob commented Jun 25, 2024

merged to master

@xylar xylar deleted the ocn/init-land-ice-pressure-from-topo branch June 29, 2024 15:30
@xylar
Copy link
Contributor Author

xylar commented Jun 29, 2024

Thank you so much @jonbob! And also thank you @cbegeman, @sbrus89 and @mark-petersen for the reviews!

xylar added a commit to xylar/compass that referenced this pull request Jun 30, 2024
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. mpas-ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants