ESCOMP / CISM

Community Ice Sheet Model
GNU Lesser General Public License v3.0
6 stars 11 forks source link

Changes needed for multiple ice sheet instances #45

Closed billsacks closed 2 years ago

billsacks commented 2 years ago

This is a work in progress, but I'm opening it so @whlipscomb can look at it, if that will be useful.

whlipscomb commented 2 years ago

@billsacks – These changes look good to me. One thing I'm not clear about is how gcm_restart_file is initialized for each instance. The top-down approach is to pass in a list of restart filenames from glc_InitMod, and the bottom-up approach is to get that info by reading the instance-specific config files. In the latter case, you wouldn't need to pass gcm_restart_file into glad_i_initialize_gcm. I'm not sure which approach is easier.

billsacks commented 2 years ago

These changes look good to me. One thing I'm not clear about is how gcm_restart_file is initialized for each instance. The top-down approach is to pass in a list of restart filenames from glc_InitMod, and the bottom-up approach is to get that info by reading the instance-specific config files. In the latter case, you wouldn't need to pass gcm_restart_file into glad_i_initialize_gcm. I'm not sure which approach is easier.

Thanks, @whlipscomb . We're using the top-down approach. This approach works better in the CESM context because of CESM's convention of using an rpointer file for each component; these rpointer files are typically one-line files that contain the name of the most recent restart file that should be used by the component when starting up in a restart run. So the CISM-wrapper layer reads the rpointer file to determine the name of the restart file that CISM should use.

I have now made changes in CISM-wrapper so that there is a separate rpointer file for each ice sheet, but the overall flow is the same as before.

whlipscomb commented 2 years ago

Sounds good, @billsacks. Thanks for clarifying.

billsacks commented 2 years ago

@whlipscomb as I mentioned in my email last night, my two most recent commits (61aed41 and 8b928c6) bring in some significant changes, and affect standalone CISM runs as well as CESM-CISM runs. I hope I have done this in a way that doesn't change behavior for standalone CISM runs, but I haven't tested that. So I would welcome your review of these changes.

Copying my email message here:

I built off of some existing infrastructure for giving each model instance a unique id number (from 1 to N, where N is the number of ice sheets) (model%model_id), and I built this infrastructure out a bit. I have added some flexibility in how this is handled: In a CESM case (run through glad), we set max_models to be the actual number of models we're using. But, not knowing what I should do elsewhere (let alone how to test it), I have kept this flexible so that it should work correctly with other drivers that do not call set_max_models: in this case, we use a hard-coded max (which should just mean that some arrays are bigger than they need to be). (See the calls to set_max_models and check_num_models in glad_main.F90 for the optional things I do there that currently aren't done in other frontends.) The key required piece for any driver is that it needs to be sure to call register_model early in initialization, so that model%model_id is set by the time it's needed.

I haven't done any testing of cism_front_end or glint, despite having changed those a bit. Can you please let me know if you'd like me to test those (and if so, how), if you'd like to test them yourself, or if you're comfortable with my changes coming to main without testing those? I'm not imagining testing those with multiple ice sheet instances, unless you feel that's important: I'm just imagining possibly testing them with a typical single instance to make sure they still compile & run with my changes; testing the writing of a restart file with those other frontends would be good, too, since if I broke anything, it would probably be that.

whlipscomb commented 2 years ago

@billsacks – I think it will be easiest if I test the glint changes. A few months ago, I had glint-example working with multiple ice sheets, and I can check that this capability still works. I don't feel strongly about whether this is done before bringing your changes to the main branch, especially since I might not be able to work on it until the week after next. (I'm taking some time off next week.)

billsacks commented 2 years ago

Thanks, @whlipscomb . I should clarify that I'm slightly worried about having broken even single-instance runs. Before 8b928c6fa612aeac5d7b886acee326696b0d213c, even single-instance runs would crash in initializing the restart variable lists, because model_id was being accessed before it was defined. I think I have fixed that by moving the calls to register_model to early in initialization (specifically, before glide_config, which is where model_id is needed for defining restart variable lists), but running into that issue made me a little concerned that there may be others that I am overlooking.

whlipscomb commented 2 years ago

@billsacks – Thanks for the heads-up. If single-instance runs have broken, I expect it won't take long to find out :)

billsacks commented 2 years ago

I have done more testing of this in the CESM context. I still haven't tested standalone CISM, but based on input from @whlipscomb it sounds okay to merge it without having done that yet. I'm going to merge so that I can point to a tag of CISM in my upcoming CISM-wrapper tag.