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

For initialization with ice-shelf cavities, adjust SSH not land ice pressure #813

Merged
merged 18 commits into from
Jun 30, 2024

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Apr 9, 2024

Checklist

  • User's Guide has been updated
  • Developer's Guide has been updated
  • API documentation in the Developer's Guide (api.rst) has any new or modified class, method and/or functions listed
  • Documentation has been built locally and changes look as expected
  • The E3SM-Project submodule has been updated with relevant E3SM changes
  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes
  • Update cached files needed for the pr suite to run

@xylar xylar added enhancement New feature or request ocean in progress This PR is not ready for review or merging E3SM PR required labels Apr 9, 2024
@xylar xylar requested a review from cbegeman April 9, 2024 13:21
@xylar xylar self-assigned this Apr 9, 2024
@xylar
Copy link
Collaborator Author

xylar commented Apr 9, 2024

This merge requires E3SM-Ocean-Discussion/E3SM#90

@xylar xylar force-pushed the adjust-ssh-not-land-ice-pressure branch from 0a85620 to b7b16ee Compare April 10, 2024 10:20
@xylar xylar mentioned this pull request Apr 10, 2024
1 task
@xylar
Copy link
Collaborator Author

xylar commented Apr 10, 2024

This worked for both the Icos30 and SORRM meshes.

compass/ocean/iceshelf.py Outdated Show resolved Hide resolved
compass/ocean/mesh/remap_topography.py Show resolved Hide resolved
compass/ocean/mesh/remap_topography.py Outdated Show resolved Hide resolved
@xylar xylar mentioned this pull request Apr 14, 2024
1 task
@xylar xylar force-pushed the adjust-ssh-not-land-ice-pressure branch 3 times, most recently from 753f935 to cfb8a31 Compare April 14, 2024 14:37
Copy link
Collaborator

@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.

@xylar This looks good to me! Let me know if you'd like me to look it over again once you've finished the documentation.

@xylar xylar force-pushed the adjust-ssh-not-land-ice-pressure branch 3 times, most recently from 81ffc62 to 2456db9 Compare May 8, 2024 22:33
@xylar
Copy link
Collaborator Author

xylar commented May 9, 2024

Testing

I have successfully run the mesh, init and performance_test tests on the IcoswISC30 and IcoswISC240 meshes. I also created new cache files and ran the pr suite. All tests were on Chrysalis with Intel and OpenMPI.

@xylar xylar removed the in progress This PR is not ready for review or merging label May 9, 2024
@xylar
Copy link
Collaborator Author

xylar commented May 9, 2024

Waiting on E3SM-Project/E3SM#6375

@xylar
Copy link
Collaborator Author

xylar commented Jun 10, 2024

I rebased this and successfully ran the pr suite on Chrysalis (Intel, OpenMPI) so I think this is ready to go in once the E3SM-Project submodule has been updated with the changes from E3SM-Project/E3SM#6375.

@xylar xylar force-pushed the adjust-ssh-not-land-ice-pressure branch 2 times, most recently from bb9dc26 to f9843fe Compare June 12, 2024 11:50
@mark-petersen
Copy link
Collaborator

Testing. See notes at E3SM-Project/E3SM#6375 (comment)

@xylar
Copy link
Collaborator Author

xylar commented Jun 29, 2024

Would you advise me to use the initial conditions from the new process, with this PR? I assume that we catch the important updates for the new HR IC that way.

No, I would advise you to use the current compass. This process is important for meshes that will be used (eventually) for Antarctic Ice Sheet coupling, which I do not believe you are planning to do.

@mark-petersen
Copy link
Collaborator

Thanks, that is all very helpful information.

@xylar
Copy link
Collaborator Author

xylar commented Jun 29, 2024

In the meantime, tough, I will attempt to get RRSwISC6to18 working with these changes.

xylar added 18 commits June 30, 2024 05:52
Use the density of ice from BedMachine Antarctica v3
We want the ice draft to be more consistent with land ice pressure
in MPAS-Ocean so we use land ice thickness to compute both, and
use the density of seawater from E3SM rather than the one used
in BedMachine Antarctica v3.
They need landIcePressureObserved and landIceFloatingFracObserved,
not landIceGroundedFracObserved
This merge reorganizes SSH adjustment substantially. It is now
a sequence of alternating init and forward runs, with decreasing
minimum water column thickness.

The sea surface height is adjusted instead of `landIcePressure`
for better consistency with planned coupling to MALI.
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.
This will make results more consistent between BedMachine and
MALI (for future coupled simulations).
@xylar xylar force-pushed the adjust-ssh-not-land-ice-pressure branch from f9843fe to 1af70f8 Compare June 30, 2024 10:58
@xylar xylar marked this pull request as ready for review June 30, 2024 10:58
@xylar
Copy link
Collaborator Author

xylar commented Jun 30, 2024

Testing

I ran the pr suite on Chrysalis with Intel and OpenMPI one more time after the rebase and submodule update. I used the final test from #834 as a baseline. As expected, Icos240, IcoswISC240 and IcoswISC test cases pass pass execution and test comparison but fail baseline comparison.

@xylar xylar merged commit 463dc48 into MPAS-Dev:main Jun 30, 2024
4 checks passed
@xylar xylar deleted the adjust-ssh-not-land-ice-pressure branch June 30, 2024 15:02
xylar added a commit to xylar/compass that referenced this pull request Jul 1, 2024
Following MPAS-Dev#813, the
initial state is the same whether ice shelf cavities are present
or not.  It is the result of the `init/initial_state` step
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants