-
Notifications
You must be signed in to change notification settings - Fork 371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
limit Visbeck eddy length to dcEdge (AMOC PR 4/6) #4868
limit Visbeck eddy length to dcEdge (AMOC PR 4/6) #4868
Conversation
@@ -611,8 +611,8 @@ subroutine ocn_GM_compute_Bolus_velocity(statePool, & | |||
c_Visbeck = sqrt(sumN2*ltsum) | |||
sumRi = sumRi / (ltsum + 1.0E-11_RKIND) | |||
|
|||
eddyLength(iEdge) = min(c_Visbeck/(1.0E-15_RKIND + abs(fEdge(iEdge))), & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vanroekel this array was created in the original Visbeck addition. Both eddyLength
and eddyTime
are used only in these three lines within the iEdge
loop, so these arrays could both easily be replaced by scalars. The only reason to keep them is for writing them out. Do you have a preference if these are arrays or scalars?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mark-petersen no real preference on scalar v array. I had them as arrays for diagnostics but I don't think we need them for the duration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving based on visual inspection but think the change of eddyLength and eddyTime to scalars would be a good idea too
@@ -611,8 +611,8 @@ subroutine ocn_GM_compute_Bolus_velocity(statePool, & | |||
c_Visbeck = sqrt(sumN2*ltsum) | |||
sumRi = sumRi / (ltsum + 1.0E-11_RKIND) | |||
|
|||
eddyLength(iEdge) = min(c_Visbeck/(1.0E-15_RKIND + abs(fEdge(iEdge))), & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mark-petersen no real preference on scalar v array. I had them as arrays for diagnostics but I don't think we need them for the duration.
Ran a 5 year |
Ran a 5-year test with this branch for the SORRM: |
Visbeck baseline test (ran with hash tag b404a46) with SORRM was completed. Complete analysis here: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.milena/20220411.CRYO1850.ne30pg2_SOwISC12to60E2r4.chrysalis.CryoBranchBaselineGMVisbeck/Years1-5/ |
Clearly, the Visbeck eddy length limiter makes a small difference for SORRM. |
I'll post the analogous plots for the low-res comparison, adding global MOC: Visbeck baseline: Visbeck limiter (this PR): |
…ext (PR #4868) Limit Visbeck eddy length to dcEdge When using config_GM_closure='Visbeck' (Visbeck et al 1997), limit the eddy length scale to be no less than dcEdge, the distance between cells. This ensures that gmBolusKappa does not have very small values due to the eddyLength factor. When using config_GM_closure='Visbeck' it is not BFB. However, all E3SM runs use config_GM_closure='constant' or 'EdenGreatbatch', but never Visbeck, so this PR is BFB for all current tests. [BFB]
Passed:
merged to next |
merged to master |
When using
config_GM_closure='Visbeck'
(Visbeck et al 1997), limit the eddy length scale to be no less than dcEdge, the distance between cells. This ensures thatgmBolusKappa
does not have very small values due to theeddyLength
factor.All E3SM runs use
config_GM_closure='constant'
orconfig_GM_closure='EdenGreatbatch'
, but never Visbeck, so this PR is BFB for all tests. When usingconfig_GM_closure='Visbeck'
it is not BFB.[BFB]