+(*)MOM_hor_visc bug fix and loop size cleanup #84
Conversation
Fixed the bug noted in issue mom-ocean#72 and excessive loop sizes noted in the unresolved comments in a recent commit, and cleaned up other aspects of MOM_hor_visc, mostly related to the code that is exercised when USE_GME=True. - Fixed the bug with the wrong arrays being used when ADD_LES_VISCOSITY=True that was noted in MOM6 issue mom-ocean#72. - Corrected some of the overly large loop extents that were noted in unresolved comments about MOM6 PR mom-ocean#65. - Only log USE_KH_BG_2D if a Laplacian viscosity is used. - Use extra calculations in the halos and marching in to avoid unnecessary halo updates in smooth_GME if there multiple smoothing passes. - Corrected the capitalization of some horizontal indices to follow the MOM6 convention for indicating the horizontal staggering position. - Collected the post_data calls for diagnostics with use_GME with the other post data calls to collocate some of the potential inter-PE synchronization points. - Fixed the indentation of some expressions that were using less than 4 points for some continuation lines. - Eliminated several unused variables, and fused some loops to allow 2-d variables to be replaced with scalars. With this PR, answers can change when ADD_LES_VISCOSITY=True and there is a Smagorinsky or Leith Laplacian viscosity and there is a nonzero background. One run-time parameter is no longer logged if LAPLACIAN=false, so there are changes to the MOM_parameter_doc files. All answers in the MOM6-examples test suite are bitwise identical.
Hallberg-NOAA
added
answer-changing
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #84 +/- ##
=========================================
Coverage 28.98% 28.98%
=========================================
Files 245 245
Lines 72185 72188 +3
=========================================
+ Hits 20922 20926 +4
+ Misses 51263 51262 -1
Continue to review full report at Codecov.
|
|
Thanks for addressing the issue regarding While we are on this, I think there is another issue (mom-ocean#1544) with modified_leith (I posted in the main repo during the migration and I didn't know the right place to post). It looks like the code is slightly different from the description in the comments (and the literatures I think). |
There was a problem hiding this comment.
@Hallberg-NOAA: thank you very much for addressing these issues and improving this module. I have verified that this PR does not change answers when USE_GME = True, including when using different PE layouts.
Minor code cleanup in response to the code review from Gustavo Marques. In particular, this introduces a roundoff-level answer changing code simplification for code that is only exercised if USE_GME=True. In all other cases the answers are bitwise identical.
|
I believe that the latest commit should satisfactorily address the suggestion by @gustavo-marques, and that this PR is now ready for pipeline testing. |
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/15005 ✔️ 🟡 Removed parameter:
|
|
I can't see any reason what status GitHub is waiting on here, and I've tried the "resubmit" trick to no avail, so I am going to admin-override this PR. |
Fixed the bug noted in issue #72 and excessive loop sizes noted in the
unresolved comments in a recent commit, and cleaned up other aspects of
MOM_hor_visc, mostly related to the code that is exercised when USE_GME=True.
Fixed the bug with the wrong arrays being used when ADD_LES_VISCOSITY=True
that was noted in MOM6 issue Potential bug related the add_LES_viscosity option #72.
Corrected some of the overly large loop extents that were noted in unresolved
comments about MOM6 PR Fixes issues with the GME code and get_param calls for Leith options #65.
Only log USE_KH_BG_2D if a Laplacian viscosity is used.
Use extra calculations in the halos and marching in to avoid unnecessary halo
updates in smooth_GME if there multiple smoothing passes.
Corrected the capitalization of some horizontal indices to follow the MOM6
convention for indicating the horizontal staggering position.
Collected the post_data calls for diagnostics with use_GME with the other
post data calls to collocate some of the potential inter-PE synchronization
points.
Fixed the indentation of some expressions that were using less than 4 points
for some continuation lines.
Eliminated several unused variables, and fused some loops to allow 2-d
variables to be replaced with scalars.
With this PR, answers can change when ADD_LES_VISCOSITY=True and there is a
Smagorinsky or Leith Laplacian viscosity and there is a nonzero background.
One run-time parameter is no longer logged if LAPLACIAN=false, so there are
changes to the MOM_parameter_doc files. All answers in the MOM6-examples test
suite are bitwise identical.