CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 131 forks source link

ridge_ice no longer public #478

Open kshedstrom opened 9 months ago

kshedstrom commented 9 months ago

Hi all, some of us in the SIS2 community have been calling Icepack's ridge_ice from SIS2. I saw that it became private as of Icepack 1.3.4 and started working on updating how we call Icepack's ridging routines. The public icepack_step_ridge calls both ridge_ice and cleanup_itd. I'm not opposed to the idea of cleaning up the itd in a mass and energy conserving fashion, but something you are doing doesn't play well with SIS2.

Can I ask that ridge_ice be made public again?

apcraig commented 9 months ago

We could make it public. We would rename it to something like icepack_ridge_ice if we do. We'll have to discuss that. But I'd like to understand better why icepack_step_ridge doesn't work. When you say "process mask_table", what do you mean.

Also, are you currently just using ridge_ice more-or-less without any other icepack code? If we make it public, maybe that would be a good time to update the public ridging interfaces to remove all the config/parameters arguments that are static during the run. These are set with the icepack_init_parameters interface prior to using ridge_ice. Would that be problematic?

kshedstrom commented 9 months ago

Both MOM6 and SIS2 have the option to read a mask_table such that the cores with all land can be masked out. For instance, I can have a layout of 20 by 25, with say 40 of them masked out. One would then request 460 cores to run it on instead of 500. When running SIS2 with icepack_step_ridge and a mask_table, it fails in a communication elsewhere in the code.

The SIS2 ridging code has an initialization routine which calls icepack_init_tracer_sizes, icepack_init_tracer_indices, and icepack_init_parameters(mu_rdg_in=CS%mu_rdg, conserv_check_in=.true.). I don't have a problem with more such calls.

I believe the plan is to make more calls to Icepack in the future, adding features currently lacking in SIS2, but not CICE. Melt ponds have been discussed, also the light penetration scheme and black carbon. Maybe eventually even the ice algae.

apcraig commented 9 months ago

Thanks @kshedstrom. The icepack calls are on a gridcell by gridcell basis, so however you mask and decompose has nothing to do with icepack. There is no dependency between gridcells in icepack, so it'd be interesting to understand better why changes to icepack are interacting with the mask/decomposition.

kshedstrom commented 9 months ago

According to icepack_itd.F90:

! Cleanup subroutine that rebins thickness categories if necessary,
!  eliminates very small ice areas while conserving mass and energy,
!  aggregates state variables, and does a boundary call.

I didn't immediately find the "boundary call", assuming that is some sort of MPI communication. I wouldn't know what to look for!

apcraig commented 9 months ago

@kshedstrom that comment must be old. There is no boundary call in Icepack. Icepack operates on one gridcell at a time with no communication between gridcells in icepack.

kshedstrom commented 9 months ago

@apcraig Interesting. I can't explain my troubles with it then.

eclare108213 commented 1 month ago

We need to fix the boundary call comment in the code, but otherwise it would be good to understand why icepack_step_ridge is causing a problem. It sounds like cleanup_itd is the source, since ridge_ice works on its own, but perhaps there is something else in icepack_step_ridge, like an array declaration or intent() or optional argument? What communication elsewhere fails? Do you get any additional information when running with debugging flags in a case that fails to see if something isn't initialized, NaNs etc? The significant slowdown could be indicative of NaNs.

eclare108213 commented 1 week ago

Hi @kshedstrom, did you ever figure out what was causing the boundary problem when calling icepack_step_ridge? Based on the discussion above, it shouldn't be coming from Icepack itself, and if it's no longer a problem for you, I'd like to close this issue.

kshedstrom commented 1 week ago

No, sorry, it's just easier for us to checkout version 1.3.3 and use that. If I tell Icepack not to call cleanup_itd, I get a lot of warnings from Icepack and some options cause SIS2 to blow up as well in its dynamics. I need a smaller test case!

eclare108213 commented 1 week ago

Does it work to use the current release but just make ridge_ice public? That's a simple fix on your end. There definitely seems to be a conflict of some kind between SIS2's setup and Icepack, whether it's memory or undefined array arguments or something else.

kshedstrom commented 1 day ago

No, it doesn't work. Something has changed numerically between 1.3.3 and 1.4.x.

One option if you don't allow the public ridge_ice is to add some optional arguments as in https://github.com/ESMG/Icepack/tree/optional_cleanup but that doesn't work for me either.

Sorry, recovering from Covid, moving a little slowly.