+Remove in-loop clocks and obsolete params #28
Conversation
Removed clocks that were being called from inside of j-loops in two modules. These are inefficient and can cause the model to hang in some cases if used, and there are better ways to get timing information at this level. If there is interest in the timing breakdown at this level, the code should be restructured to move the key blocks outside of the j-loops. The run-time parameter ALLOW_CLOCKS_IN_OMP_LOOPS is no longer being used so it is now obsoleted. All answers are bitwise identical, but there are changes to some MOM_parameter_doc files.
Added a deallocate call for eta_PF_start in step_MOM_dyn_split_RK2() to avoid a possible memory leak. All answers are bitwise identical.
Set find_salt_root even if SHELF_THREE_EQN = .False. to avoid using an uninitialized logical to determine which parameters are logged. Without this the contents of some MOM_parameter_doc.all files could depend on the state of uninitialized memory and was compiler dependent in some cases. All answers are bitwise identical, but in some cases the contents of MOM_parameter_doc files could be corrected.
The runtime parameter ETA_TOLERANCE_AUX was being read but was never used, so it is being obsoleted. However, because some experiments were using this and there are effectively no changes in behavior, a warning will be issued instead of a fatal error if this parameter is set. All answers are bitwise identical, but there are changes to some MOM_parameter_doc files.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #28 +/- ##
============================================
- Coverage 29.06% 29.05% -0.02%
============================================
Files 240 240
Lines 71319 71293 -26
============================================
- Hits 20731 20712 -19
+ Misses 50588 50581 -7
Continue to review full report at Codecov.
|
|
Can you first test if #27 fixes the problem you noticed? |
|
The problem that this addresses appears to be intermittent, and it may be dependent on the state of the computer. If I encounter the problem again before this PR is merged in, I will take that opportunity to test #27. However, the issue has not re-occurred in the past few days of running on Gaea. Regardless of the specific issue with Gaea that prompted these changes, I think that these changes are a good idea. |
|
I agree, in-loop clocks have potential to cause a lot of trouble. |
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14308 ✔️ 🟡 This will require a parameter update due to removal of various clock params. |
This PR fixes some problems with the model potentially hanging, having a
memory leak, or having output that depends on the contents of an uninitialized
logical. As a part of this change, a run time parameter that is no longer being
used was obsoleted, and because the MOM_parameter_doc files were changing anyway
an additional run-time parameter that does nothing was also obsoleted. All
answers are bitwise identical, but the contents in some MOM_parameter_doc files
change.
The commits in this PR include: