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

Fix support for maxLevelCell = 0 in MPAS-Ocean #83

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Mar 7, 2024

There are various locations where maxLevelCell was previously assumed to be non-zero that this PR fixes.

@xylar xylar requested a review from cbegeman March 7, 2024 11:21
@xylar xylar changed the title Fix support for maxLevelCell = 0 in MPAS-=Ocean Fix support for maxLevelCell = 0 in MPAS-Ocean Mar 7, 2024
@cbegeman
Copy link
Collaborator

@xylar I haven't had a chance to look at the output, but these changes resulted in QUwISC240's ssh_adjustment completing when ISC cells have maxLevelCell = 0.

@xylar xylar force-pushed the ocn/fix-max-level-cell-zero branch from 47391d1 to 3db25e4 Compare March 11, 2024 12:03
@xylar
Copy link
Collaborator Author

xylar commented Mar 11, 2024

@cbegeman, I noticed that I started from a really old base here (6 months old). Not sure quite why but I was working from Polaris so maybe that was the current state of the E3SM-Project submodule where I was working. Anyway, I rebased and removed the one change that wasn't necessary anymore (and had a conflict).

@xylar
Copy link
Collaborator Author

xylar commented Mar 11, 2024

I'll do some more testing as soon as I have time. I would like to make a version of the IcoswISC30E3r5 mesh with maxLevelCell = 0 as a starting point, assuming no one has done that already.

@cbegeman
Copy link
Collaborator

@xylar Yes, I noticed that it was old but didn't want to change your branch too much. Thanks for rebasing!

@cbegeman
Copy link
Collaborator

@xylar Have you checked if this is BFB lately? I could do this. Since we are moving on from this approach, it could be nice to get these fixes in so that it's possible for folks to build and run with maxLevelCell=0 in the future.

@xylar
Copy link
Collaborator Author

xylar commented Mar 14, 2024

@cbegeman, no, I haven't done any BFB testing of this but I would be very surprised if it isn't BFB on meshes that don't have maxLevelCell = 0.

I would still like to test it more on the ISOMIP+ test cases in Polaris, though, before I try to get it merged into E3SM just so I don't end up needing to do a second PR for things I missed. I think I might be able to get to that tomorrow or before early next week at the latest.

@cbegeman
Copy link
Collaborator

@xylar That makes sense. Thanks! I wasn't sure if you were still going this route for ISOMIP+ and I wanted to make sure it didn't get stranded.

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.

2 participants