Bugfix: FGNV streamfunction vertical bounds #1346
Conversation
This patch fixes an issue with the vertical array bounds of the Ferrari et al. streamfunction. The array is bounded across interfaces, from 1 to nz+1, but only the interior values need to be determined due to the arbitrary boundary value (set here to zero). In the current source, the streamfunction is rescaled before calling streamfn_solver, but need not be applied to the boundary values. This is unlikely to cause errors in production, since the values are later reset to zero, but the rescaling can raise errors in more aggressive debugging builds, such as when the arrays are initialized with NaN values.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1346 +/- ##
============================================
- Coverage 45.83% 45.82% -0.01%
============================================
Files 234 234
Lines 72663 72667 +4
============================================
Hits 33302 33302
- Misses 39361 39365 +4
Continue to review full report at Codecov.
|
| @@ -993,10 +993,14 @@ subroutine thickness_diffuse_full(h, e, Kh_u, Kh_v, tv, uhD, vhD, cg1, dt, G, GV | |||
| ! Solve an elliptic equation for the streamfunction following Ferrari et al., 2010. | |||
| do I=is-1,ie | |||
| if (G%mask2dCu(I,j)>0.) then | |||
| Sfn_unlim_u(I,:) = ( 1. + CS%FGNV_scale ) * Sfn_unlim_u(I,:) | |||
| do K=2,nz | |||
| Sfn_unlim_u(I,K) = (1. + CS%FGNV_scale) * Sfn_unlim_u(I,K) | |||
There was a problem hiding this comment.
This is awesome 👏 thank you @marshallward
Any reason why it can't be
vectorized? e.g.,
x(index1_range, index2_range) =0. (I understand the compiler may do it!)
- Do you anticipate different behavior for different optimizations?
- violates coding style?
- something else?
There was a problem hiding this comment.
Yes, I think Sfn_unlim_[uv](2:nz) = ... would have been OK here, other than that it's discouraged by the style guide (although your example might be permitted).
This next line is a very explicit break from the style guide:
call streamfn_solver(nz, c2_h_v(i,:), hN2_v(i,:), Sfn_unlim_v(i,:))
due to the sliced array inputs - and the implicit 1D arrays - but they are only 1D arrays so I figured let's leave it for another day.
I wouldn't expect vectorization to be affected in this case, but compilers always have a surprise or two.
There was a problem hiding this comment.
@marshallward thanks for the fix.
[Our debug build now works with this fix and optimized (i.e., repro) has no change in answer.]
This patch fixes an issue with the vertical array bounds of the Ferrari
et al. streamfunction. The array is bounded across interfaces, from 1
to nz+1, but only the interior values need to be determined due to the
arbitrary boundary value (set here to zero).
In the current source, the streamfunction is rescaled before calling
streamfn_solver, but need not be applied to the boundary values. This
is unlikely to cause errors in production, since the values are later
reset to zero, but the rescaling can raise errors in more aggressive
debugging builds, such as when the arrays are initialized with NaN
values.
Thanks to @sanAkel for reporting this bug.