Fix USE_CONT_THICKNESS bug #697
Conversation
Optionally correct a bug due to a missing halo exchange. This likely isn't needed when compiled for nonsymmetric memory, but the added halo exchange does no harm in such cases. The pass_vector call could probably be replaced with fill_symmetric_edges, except there is no subroutine fill_vector_symmetric_edges_3d. USE_CONT_THICKNESS is not yet widely used, so rather than preserving the old (incorrect) solutions by default, this bug is corrected by default. However, the previous answers can be recovered by setting the new runtime parameter USE_CONT_THICKNESS_BUG to true. This parameter is only used (and logged) when USE_CONT_THICKNESS set to true. By default, this commit does change answers in symmetric memory cases with USE_CONT_THICKNESS = True, and there is a new runtime parameter in such cases.
There was a problem hiding this comment.
I agree that these changes are necessary.
However, I think that it would be more efficient if the extra halo update were moved outside of horizontal_viscosity() and instead were a part of the group pass for the other variables that come out of the continuity solver calls in step_MOM_dyn_split_RK2(). This would also allow for the intent of the hu_cont and hv_cont arguments to be kept as intent(in), which is more consistent with how they are actually used.
For now, I am going to suggest that we accept this PR as it stands, but with a plan to explore whether moving the new halo update outside of horizontal_viscosity() would be cleaner and more efficient.
Hallberg-NOAA
added
bug
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/24544 ✔️ |
Optionally correct a bug due to a missing halo exchange. This likely isn't needed when compiled for nonsymmetric memory, but the added halo exchange does no harm in such cases. The pass_vector call could probably be replaced with fill_symmetric_edges, except there is no subroutine fill_vector_symmetric_edges_3d.
USE_CONT_THICKNESS is not yet widely used, so rather than preserving the old (incorrect) solutions by default, this bug is corrected by default. However, the previous answers can be recovered by setting the new runtime parameter USE_CONT_THICKNESS_BUG to true. This parameter is only used (and logged) when USE_CONT_THICKNESS set to true.
By default, this commit does change answers in symmetric memory cases with USE_CONT_THICKNESS = True, and there is a new runtime parameter in such cases.