ESCOMP / CMEPS

NUOPC Community Mediator for Earth Prediction Systems
https://escomp.github.io/CMEPS/
24 stars 79 forks source link

Fix issues with multiple ice sheets #242

Closed billsacks closed 3 years ago

billsacks commented 3 years ago

Description of changes

Fixes a few issues with running multiple ice sheets (i.e., multiple glc components):

(1) Fixes accumulation of topography with multiple ice sheets (fixes #229)

(2) Removes the need to set num_icesheets explicitly in the namelist; instead, determines this based on the number of elements in the mesh_glc namelist variable

(3) Removes a buggy double loop over ice sheet instances for smb renormalization

Specific notes

Contributors other than yourself, if any: @mvertens was the main contributor; I just made some final changes & testing

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers?

Any User Interface Changes (namelist or namelist defaults changes)?

Testing performed if application target is CESM:(either UFS-S2S or CESM testing is required):

Testing performed if application target is UFS-coupled:

Testing performed if application target is UFS-HAFS:

Hashes used for testing:

billsacks commented 3 years ago

@mvertens or others reviewing this: A few notes:

(1) I recommend viewing diffs with whitespace diffs hidden: one commit in particular introduced a lot of whitespace changes because of the removal of a level of looping, so it's easier to see what changed if you hide the whitespace diffs

(2) I have run CISM's aux_glc testing on these changes; I'm not sure if you want some other testing run

(3) I see there are merge conflicts; let me know if you want me to help resolve them

billsacks commented 3 years ago

I'm working on the merge conflicts and will push soon....

billsacks commented 3 years ago

Okay, I fixed the merge conflicts and just pushed the result. That was one ugly set of merge conflicts. For one conflict, I swear that git just threw up its hands in despair: what it gave me even outside the conflict region was completely wrong and it took me a while to figure out what was going on. But I think I have things right now: I did a careful comparison of the diffs from master to this branch before and after the merge, and the diffs look effectively the same now. I'm going to run some tests overnight to double-check the merge and will post the results back here in the morning.

billsacks commented 3 years ago

I reran the aux_glc testing with the latest version of this branch, post-merge, and compared with my previous set of testing. Tests passed and were bit-for-bit – other than the ones I referenced in #243 , and other than one FIELDLIST diff that I assume is expected from the update from cmeps0.13.30 to latest (the new version has time_bnds in the cpl hist file, the old did not).

So it seems like the hairy merge probably went okay.