search

ESCOMP / CISM-wrapper

Community Ice Sheet Model wrapper for CESM
http://www.cesm.ucar.edu/models/cesm2.0/land-ice/
Other
3 stars 15 forks source link

Call ESMF_DistGridDestroy if/when it is needed #65

Closed billsacks closed 2 years ago

billsacks commented 2 years ago

In the following, I am referring to this block of code:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_comp_nuopc.F90#L381-L405

The last line of that calls:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_import_export.F90#L235

which in turn calls:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_import_export.F90#L541

Recently, Mariana suggested that I add a DistGridDestroy call here:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_comp_nuopc.F90#L400

after using the DistGrid to create a mesh a few lines earlier. However, when I do that, the code dies in the ESMF_FieldCreate using the mesh on this line:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_import_export.F90#L586

This is in a run built with debug flags, using a single ice sheet (so num_icesheets is 1 in the various loops in the above code references).

My guess is that the mesh stores a reference to the DistGrid, and that DistGrid reference needs to still be valid during the FieldCreate call.

Simply removing the DistGridDestroy call solves the problem for the single ice sheet case. But this raises two questions for me:

(1) When, if at all, should we call DistGridDestroy in this code?

(2) Given that it doesn't seem safe to call DistGridDestroy to clean the DistGrid object each time through this loop:

https://github.com/ESCOMP/CISM-wrapper/blob/fe02e90094471dd93576880615cbe4f8c43f0e5e/drivers/cpl/nuopc/glc_comp_nuopc.F90#L381-L405

do we actually need an array of DistGrid objects in a multiple ice sheet run so that the DistGrid for ice sheet 2 doesn't clobber the DistGrid for ice sheet 1 while it is still needed? It seemed like things worked in our multiple ice sheet test run with only one DistGrid object (without the DistGridDestroy call), but I'm wondering if that is not always guaranteed.

billsacks commented 2 years ago

For now, to be safe, I am introducing an array of DistGrids and not calling DistGridDestroy – see https://github.com/ESCOMP/CISM-wrapper/commit/e930f69bbcfc9390295f4b8261b3ec14ca435556. But I'm not sure if that's the right thing to do, so I'd like input from @mvertens on this when she gets a chance.

billsacks commented 2 years ago

Rocky Dunlap points out:

My short answer is that I think you do need separate DistGrids - one for each ice sheet - since the index space is going to be completely different for each ice sheet (e.g., the number of points will almost certainly be different). In the loop where you do NOT call DistGridDestroy, I think you are actually creating two separate DistGrids, so this is why it is working. I am not sure if this is a memory leak, since you are losing the DistGrid reference.

But I'm waiting to hear when (if at all) it's safe to destroy the DistGrid object.

billsacks commented 2 years ago

@mvertens suggests just closing this as a wontfix.