Intent out halos #847
Intent out halos #847
Conversation
Initialize the halos of 7 intent out arrays to 0 so that they will not have uninitialized data in their halo regions. The impacted arrays are u_cor and v_cor in continuity_PPM, h_MLD in convert_MLD_to_ML_thickness, MLD in energetic_PBL_get_MLD and LmixScale, bottomFac2 and barotrFac2 in MEKE_lengthScales. This does not change solutions in any of the MOM6-examples test cases, but it is conceivable that could, and it does change the values in the debugging checksum output for some cases to more nearly reproduce under dimensional rescaling and rotation.
Added the new interfaces reset_h_var_halo_vals and reset_vector_halo_vals to the MOM_debugging module to reset the halo regions of a 2-d or 3-d tracer-cell array or the components of a 2-d or 3-d C-grid vector to specified values. This can be useful when working with arrays that have had their halo values replaced with random values being used for intent(out) arrays, and for debugging bad values along side checksum calls. All answers are bitwise identical, but there are two new public interfaces.
Reset 5 state variable halo values after initialization to compensate for unintended values that may have been set via intent(out) arrays in the initialization routines, when the new runtime parameter RESET_STATE_HALOS_POST_INIT is set to true. A subsequent halo update will replace these values in many cases, but enabling this option does change answers in the Baltic_OM4_05 test case and perhaps others. By default all answers are bitwise identical, but there is a new runtime parameter.
Hallberg-NOAA
added
the
Parameter change
|
I'm a bit uncomfortable with using zero-initialization to fix possible errors or to restore symmetry of a solution. Are all of these initializations required? Is there a way to fix the underlying gap in the solver, or is there a physical reason for zero-initialization that I am missing? |
|
For the most part this PR is not about fixing the symmetry of the solution, but rather about retaining the ability to use checksums (including halos) to verify the correct symmetry of the solution, and avoiding false differences in checksums that include the halos of certain fields. For those cases, an alternative equivalent approach might be change As noted, there is at least one case where restoring the halo values does change answers and gives rotational symmetry, and this is the reason for the new runtime parameter ( |
|
If the checksums need to agree, and they depend on prior values (which may be junk), then it seems to me that the proper solution is to define arguments as If the checksums are including values should should not be included, then shouldn't the fix be on the checksum, rather than the solver? |
|
These checksum values actually do matter when they are not being masked out by a land mask (as they would be with closed boundaries), and they are usually filled with the correct values by halo update calls, but of course halo updates do not change values at the closed edges of a domain. |
|
As an update, when I tried this PR based on the latest version of dev/gfdl, the answers are no longer changing when the new runtime parameter |
|
After discussion with @adcroft and @Hallberg-NOAA, we agreed to consider a solution using |
+(*)Set initialized state halos & intent out halos
This PR consists of 3 commits that set the halo regions of several intent(out) variables that are subsequently used in halo update or checksum calls or could otherwise be used to set other variables, such as those related to open boundary conditions.
To achieve this, the new overloaded interfaces
reset_h_var_halo_vals()andreset_vector_halo_vals()have been added to the MOM_debugging module to reset the halo regions of a 2-d or 3-d tracer-cell array or the components of a 2-d or 3-d C-grid vector to specified values. When the new runtime parameter RESET_STATE_HALOS_POST_INIT is set to true, these are used to set the 5 state variable halos (h,u,v,TandS) to sensible values before a pass var is done on htem. Enabling this option does change answers with the Baltic_OM4_05 test case and perhaps others, but it is not enabled by default.The halos of 7 other intent out variables that are used in halo updates or checksums but do not appear to changes answers (probably because their previously unset halo points are masked out by landmasks when they would not be reset by a halo update) are also explicitly set. The impacted arrays are
u_corandv_corincontinuity_PPM(),h_MLDinconvert_MLD_to_ML_thickness(),MLDinenergetic_PBL_get_MLD()andLmixScale,bottomFac2andbarotrFac2inMEKE_lengthScales(). This commit does change the values in the debugging checksum output for some cases to more nearly reproduce under dimensional rescaling and rotation.By default, all answers in the MOM6-examples test suite are bitwise identical. However, it is possible that there are cases where the answers change, but if so the existing answers should probably be considered to be incorrect. There are two new public interfaces, and a new entry in the MOM_parameter_doc files. The commits in this PR include: