(*) N2_floor init fix when FGNV streamfn disabled #1505
Conversation
The `N2_floor` buoyancy frequency was left unset when `KHTH_USE_FGNV_STREAMFUNCTION` was disabled. This could potentially cause errors, such as floating point exceptions. Ideally we would restrict the calculations of `hN2_[uv]` to when the streamfunction is enabled. But due to propagation to these values to `hN2_[xy]_PE` fields, which may be used outside of the streamfunction, it is perhaps best to just initialize `N2_floor` to zero. Although this would mostly likely be initialized to zero in production, there is a chance that this could modify answers derived from random initialization. Thanks to @wfcooke for reporting this. It was also independently (and inexplicably) detected during removal of MEKE pointers, suggesting some memory volatility.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1505 +/- ##
=========================================
Coverage 29.06% 29.06%
=========================================
Files 237 237
Lines 71642 71643 +1
=========================================
+ Hits 20822 20823 +1
Misses 50820 50820
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Although this could potentially change answers in some (hopefully unused) cases, I agree that initializing this array to 0 is unambiguously the right thing to do, rather than leaving it uninitialized. I approve this PR. It should be merged in as soon as it has been involved in a successful pipeline test (the TC testing having already passed).
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/13704 ✔️ |
The
N2_floorbuoyancy frequency was left unset whenKHTH_USE_FGNV_STREAMFUNCTIONwas disabled. This could potentiallycause errors, such as floating point exceptions.
Ideally we would restrict the calculations of
hN2_[uv]to when thestreamfunction is enabled. But due to propagation to these values to
hN2_[xy]_PEfields, which may be used outside of the streamfunction,it is perhaps best to just initialize
N2_floorto zero.Although this would mostly likely be initialized to zero in production,
there is a chance that this could modify answers derived from random
initialization.
Thanks to @wfcooke for reporting this. It was also independently (and
inexplicably) detected during removal of MEKE pointers, suggesting some
memory volatility.