Add Antarctic Ice Sheet meshes for MALI #6440
Conversation
|
This PR previously discussed at E3SM-Ocean-Discussion#97 |
|
|
@matthewhoffman and @jonbob, I rebased onto and am seeing: I believe the issue is that the file is called |
|
The same problem exists with the 4to20km mesh: This should be |
I think we need to rename the files because I don't think there's support for having the prefix and datestamp be separated by an underscore. |
|
@matthewhoffman, I think you need to make all the files in You also need to make everything world readable so it can be downloaded form the inputdata server. Please run: |
|
@xylar -- thanks for taking on the initial testing. There are a bunch of mapping files I need to make before any of the new resolutions can work, but also maybe there will be issues with file permissions |
|
@jonbob, I think everything is readable so go ahead with mapping files. I don't think you need to add anything to the read-only directory for that purpose, so you should be good until @matthewhoffman is able to change permissions. |
|
@matthewhoffman, the Slack bot is bugging me about this one. Have you had a chance to change permissions and fix the issues I pointed out above? |
|
@matthewhoffman, it doesn't look like the filenames have been fixed, see #6440 (comment) and #6440 (comment) above. Could you take care of that, too? |
|
@matthewhoffman, also, could you rebase onto |
|
@matthewhoffman, I think something you did to update this branch (probably a rebase) also took out an earlier commit that added the Can you make sure you can run the following on Chrysalis and ping me to re-review after that works for you? |
matthewhoffman
force-pushed
the
matthewhoffman/mali/ais-meshes
branch
from
88e0d16 to
ef6171c
Compare
|
@xylar , sorry about all the little issues in this PR and for letting it sit for so long. I've looked through it all and made the following changes:
With these changes, I ran Do salinity restoring files need to be created for the In the meantime, @jonbob , you could proceed to generate the mapping files that will be needed for this PR. |
|
@matthewhoffman, I think there must be other errors besides salinity restoring. You will get complaints but the test should run. |
|
@xylar , you are right, sorry about that. The error seems to be related to directives in building PIO-related code, which is not something we are touching in this PR. I'll experiment with a few other test definitions to see if this is a pervasive error. |
|
@matthewhoffman, any chance this is a submodule issue? Maybe try a fresh clone or worktree? |
|
I was able to build successfully but I'm getting a segfault at runtime: See I'll try rerunning in debug mode to see if that provides any helpful info. |
|
In debug mode: I'm seeing: Presumably, this error is getting caught before the other error so it will need to be handled before we get to the one in RBF reconstruction. It seems like |
|
@xylar , yes, that makes sense - I had forgotten to update the submodules after rebasing. I can look at these MALI errors. I'm confused why they are showing up in this test and haven't shown up in other tests given this is not introducing any new functionality in MALI. |
|
@matthewhoffman, I can't speak to the vector reconstruction error but regarding the out-of-bounds indexing, do you maybe not have any tests that are compiled in debug mode? If not, maybe that one has just gone uncaught? |
|
I added the mapping files to this branch and have staged them in their corresponding locations on the lcrc local inputdata location for testing |
|
@xylar -- your test may have failed because there were no mapping files specified in config_grids. I'm running something similar right now |
|
Aaaah! It would be nice if E3SM gave an error that hinted a bit more in that direction but that certainly does sound like a good reason I was having problems! |
|
@xylar -- ah, I think E3SM would throw an error if a mapping file had been defined, but those mapping file entries were defined but blank so no file to even look for... But I was incorrect, that's not why your test failed. I ran something similar and saw the same error in the e3sm log. But MALI was throwing errors about not having xtime=0.0 in the file, and somehow that's the error e3sm ended up with? Anyway, @matthewhoffman -- I think in general we remove xtime from these files. I tested with that and got a new error about nEdgesOnCell being greater than maxEdges. I checked values in the initial file and saw this: so something is very wrong with ais_8to30km.20231222.nc |
|
I’ve re-evaluated this PR carefully, and I ended up finding issues with both the 8km and 4km mesh files. For the 8km mesh, I realized I had introduced a mesh that had not completed QAQC testing, so I rolled back to the previous version of the mesh that we had used extensively for ISMIP6-2300. The ice thickness initial condition makes it overly prone to Thwaites Glacier retreat, but given the purpose of this mesh is testing, that’s not really a concern (and maybe it’s actually useful). For the 4km mesh, two issues have been fixed:
With these updates, I ran tests for all the new meshes: The failure on the last item in the list is because it is attempting to use the SIA solver at relatively high resolution, including ice shelves, which is not a good idea and is likely to give unrealistic velocities that would trigger a CFL error. It got far enough (into the first GLC coupling interval) that I think everything about the mesh is working fine and I just need to come up with a better test. I also need to come up with tests for the two mesh_grids being added for B-cases. I will coordinate with @jonbob on both of these two issues. |
|
With the addition of a The remaining task is to demonstrate successful tests of the two remaining grids added in this PR, which will require B-cases. |
|
I've successfully tested the two fully-coupled grid specifications: with a couple caveats:
But this confirms that these new AIS meshes work in a B-case and the mapping files are correct. (thanks, @jonbob , for helping me sort through all these details!) @xylar , do you think we should deal with enabling ocn_glcshelf coupling for B-cases in this PR? My preference is to leave it out, as there will likely be other adjustments we need to make to support B-cases with iceshelf coupling, but I'm open to addressing it here. If the answer is no, then I have completed all necessary updates and testing and this PR is ready for re-review from @jonbob and @xylar . |
@matthewhoffman, no, I agree that we need to leave that for another PR at a later time. Update: I think the existing BG test cases would already test melt fluxes in coupled mode if you were to run with PISMF, see below. That's the case for CRYO configurations other than with |
@matthewhoffman I think all cases with So I don't think ocn_glcshelf is needed or useful in any cases but JRA.
|
There was a problem hiding this comment.
I did a test merge of this branch with master. Then, I ran a test with a baseline from master to make sure things are BFB:
SMS_D_P480_Ld1.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel
and, as expected, they are.
Then, I successfully ran:
SMS_D_Ld5.TL319_oQU240wLI_ais8to30.MPAS_FOLISIO_JRA1p5.chrysalis_gnu.mpaso-ocn_glcshelf
SMS_D_Ld5.TL319_oQU240wLI_ais8to30.MPAS_LISIO_JRA1p5.chrysalis_intel.mpaso-ocn_glcshelf
SMS_D_Ld5.TL319_IcoswISC30E3r5_ais4to20.MPAS_FOLISIO_JRA1p5.chrysalis_gnu.mpaso-ocn_glcshelf
(I tried the intel versions of the FOLISIO tests but Trilinos isn't available for that compiler on Chrysalis.)
I didn't test any BG cases.
I looked through the code changes and they look good to me. I have a couple of questions about the existing LISIO test but I'm approving regardless of the answers there.
Oh, I see, thanks for pointing that out. I had missed that. As far as I can tell, we do not yet have any compsets with PISMF and MALI, but that is the goal you and I are working toward, and there likely are other issues needing attention before we introduce that. |
|
@matthewhoffman -- I ran the new test that this PR brings in, and it failed on chrysalis with: Can you think of any reason this resolution wouldn't restart exactly? I tested the one it replaces and that does pass, though I didn't see anything in mali_in to explain the difference |
This commit adds the mpas.ais4to20km and mpas.ais8to30km meshes for MALI. Mapping file entries are added but the mapping files themselves don't exist yet.
This will provide a low res testing configuration. Mapping files are listed but the filenames not yet added.
The updated mpas.ais4to20km mesh correctly handles merging bedmap2 for Amundsen Sea Sector into the BedMachine geometry elsewhere. The previous version had an error in how that was done. The updated mpas.ais8to30km mesh rolls back from an inadequately tested 8km mesh to the version that was used in ISMIP6-AIS-2300.
This commit adds a FOLISIO compset (First-Order Land Ice, Sea Ice, Ocean) that uses the Albany First-Order velocity solver in MALI instead of the standard shallow-ice approximation solver. The SIA solver is inappropriate for Antarctica and was causing CFL violations when used with the new mesh, making it not practical to use even for smoketesting.
matthewhoffman
force-pushed
the
matthewhoffman/mali/ais-meshes
branch
from
2696b1d to
ed1992a
Compare
|
@jonbob , thanks for catching that. I've finally had a chance to look into it myself, and I found that the same test with the existing mesh is also failing when I run it manually: Contents of When I look at Looking at the code, that message should be printing out I see that However it looks like the case name is set correctly in However, if I rebase onto master the error with Unfortunately, an error using the new mesh persists: with this in It's not obvious to me where the problem is coming from, but I'll look into it further. My suspicion is it's an issue with the ocn-glcshelf coupling that the old mesh didn't expose, but I'm not sure yet. If so, the best strategy might be to keep the test using the old mesh for now. I'll follow up. |
|
I think I understand the test error. The previous I ran a test where I overwrote the temperature field in the |
matthewhoffman
force-pushed
the
matthewhoffman/mali/ais-meshes
branch
from
ed1992a to
0a92e25
Compare
|
If this is simply a request / need to update the default namelist so that the thermal solver is active by default I think we should just do that. Most realistic configs will need this anyway. I am happy to help out w/ re-running any tests once there's a PR in in place for this change. |
Add Antarctic Ice Sheet meshes for MALI This PR introduces two new meshes of the Antarctic Ice Sheet for MALI: * high res production mesh: mpas.ais4to20km * low-res testing mesh: mpas.ais8to30km Associated with these MALI meshes are 5 new E3SM model_grids: * ultra low res mesh for testing GG cases with JRA forcing: TL319_oQU240wLI_ais8to30 * v3 low res mesh for BG and IG cases with low res Antarctica: ne30pg2_r05_IcoswISC30E3r5_ais8to30 * v3 low res mesh for GG cases with JRA forcing with low res Antarctica: TL319_IcoswISC30E3r5_ais8to30 * v3 low res mesh for BG and IG cases with high res Antarctica: ne30pg2_r05_IcoswISC30E3r5_ais4to20 * v3 low res mesh for GG with JRA forcing cases with high res Antarctica TL319_IcoswISC30E3r5_ais4to20 [BFB] for all currently tested configurations
|
Passes:
merged to next |
|
merged to master |
This PR introduces two new meshes of the Antarctic Ice Sheet for MALI:
Associated with these MALI meshes are 5 new E3SM model_grids:
[BFB] for all currently tested configurations