Fix inverted interpolation weights for Redi K_33 at vertical interfaces#944
Open
patrickscholz wants to merge 2 commits into
Open
Fix inverted interpolation weights for Redi K_33 at vertical interfaces#944patrickscholz wants to merge 2 commits into
patrickscholz wants to merge 2 commits into
Conversation
Background
----------
The Redi (1982) isopycnal diffusion scheme replaces simple vertical diffusion
with a full diffusion tensor that acts along isopycnal surfaces. Its vertical
(33) component is the effective vertical diffusivity:
K_33 = Kv + Ki * slope²
where Kv is the background diapycnal diffusivity (defined at interfaces, nl
levels), Ki is the isopycnal diffusivity and slope² is the squared isopycnal
slope (both defined at cell centres, nl-1 levels).
The implicit vertical diffusion solver assembles flux coefficients at layer
interfaces. For the interface between layers nz and nz+1 this is:
c(nz) = -(Kv(nz+1) + Ty1) * zinv2 * ...
Kv is already at the interface. Ty1 = Ki*slope² must be linearly interpolated
from the two bracketing cell centres Z(nz) and Z(nz+1) to the interface at
zbar(nz+1).
The bug
-------
Standard linear interpolation of a quantity f from cell centres Z(nz) and
Z(nz+1) to the interface at zbar(nz+1) is:
f* = f(nz) * (zbar(nz+1) - Z(nz+1)) / (Z(nz) - Z(nz+1))
+ f(nz+1) * (Z(nz) - zbar(nz+1)) / (Z(nz) - Z(nz+1))
i.e. each cell gets a weight equal to the distance to the *other* cell
divided by the total distance between cell centres. The cell closer to the
interface receives the larger weight.
The code had the two distance factors swapped:
Ty1 = (Z(nz) - zbar(nz+1)) * zinv2 * slope²(nz) * Ki(nz) ! WRONG
+ (zbar(nz+1) - Z(nz+1) ) * zinv2 * slope²(nz+1)* Ki(nz+1) ! WRONG
giving Ki(nz) the weight of the *distance from the upper cell to the
interface* instead of the distance from the interface to the lower cell.
Concrete example (Z(nz)=-10m, zbar(nz+1)=-15m, Z(nz+1)=-30m, total=20m):
correct weight on Ki(nz) : (zbar-Z(nz+1))/total = 15/20 = 0.75
code weight on Ki(nz) : (Z(nz)-zbar )/total = 5/20 = 0.25 <- wrong
On a uniform vertical grid both distances are equal (hnode/2 each), so both
weights evaluate to 0.5 and the error cancels exactly. The bug is therefore
invisible in idealised test cases with uniform vertical spacing and only
manifests on stretched grids.
Impact
------
All production FESOM2 configurations use a stretched vertical grid (high
resolution near the surface, coarser below). At each interface the
interpolated K_33 is biased toward whichever adjacent cell is thicker,
i.e. toward the coarser, deeper cell. This systematically overestimates
K_33 at shallow interfaces and underestimates it at depth, effectively
rotating the Redi diffusion tensor away from the isopycnal surface and
introducing spurious diapycnal mixing — exactly the flux the Redi scheme
is designed to suppress.
The error is largest where the vertical grid changes most rapidly and where
isopycnal slopes are steep: shelf breaks, the Antarctic Circumpolar Current,
and the upper ocean seasonal thermocline.
Note: the Redi scheme is not active by default. Impact is limited to runs
with isopycnal (Redi/GM) mixing enabled.
Fix
---
Swap the two distance factors in all three instances of Ty/Ty1 in
diff_ver_part_impl_ale (surface layer nzmin, interior loop nzmin+1 to
nzmax-2, and bottom layer nzmax-1), so each cell centre is weighted by
its distance to the opposite cell rather than to the interface:
Ty1 = (zbar(nz+1) - Z(nz+1)) * zinv2 * slope²(nz) * Ki(nz)
+ (Z(nz) - zbar(nz+1)) * zinv2 * slope²(nz+1)* Ki(nz+1)
No other logic is changed. On a uniform grid the fix is a no-op.
suvarchal
approved these changes
Jun 25, 2026
suvarchal
reviewed
Jun 25, 2026
suvarchal
left a comment
Collaborator
There was a problem hiding this comment.
is it another bug hunt of yours?
Contributor
Author
|
its some older stuff from nikolay and sergey about some AI assisted code evaluation, that i remembered and work through now |
Contributor
Author
patrickscholz
marked this pull request as ready for review
June 26, 2026 10:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Background
The Redi (1982) isopycnal diffusion scheme replaces simple vertical diffusion with a full diffusion tensor that acts along isopycnal surfaces. Its vertical (33) component is the effective vertical diffusivity:
where Kv is the background diapycnal diffusivity (defined at interfaces, nl levels), Ki is the isopycnal diffusivity and slope² is the squared isopycnal slope (both defined at cell centres, nl-1 levels).
The implicit vertical diffusion solver assembles flux coefficients at layer interfaces. For the interface between layers nz and nz+1 this is:
Kv is already at the interface. Ty1 = Ki*slope² must be linearly interpolated from the two bracketing cell centres Z(nz) and Z(nz+1) to the interface at zbar(nz+1).
The bug
Standard linear interpolation of a quantity f from cell centres Z(nz) and Z(nz+1) to the interface at zbar(nz+1) is:
i.e. each cell gets a weight equal to the distance to the other cell divided by the total distance between cell centres. The cell closer to the interface receives the larger weight.
The code had the two distance factors swapped:
giving Ki(nz) the weight of the distance from the upper cell to the interface instead of the distance from the interface to the lower cell.
Concrete example (Z(nz)=-10m, zbar(nz+1)=-15m, Z(nz+1)=-30m, total=20m):
On a uniform vertical grid both distances are equal (hnode/2 each), so both weights evaluate to 0.5 and the error cancels exactly. The bug is therefore invisible in idealised test cases with uniform vertical spacing and only manifests on stretched grids.
Impact
All production FESOM2 configurations use a stretched vertical grid (high resolution near the surface, coarser below). At each interface the interpolated K_33 is biased toward whichever adjacent cell is thicker, i.e. toward the coarser, deeper cell. This systematically overestimates K_33 at shallow interfaces and underestimates it at depth, effectively rotating the Redi diffusion tensor away from the isopycnal surface and introducing spurious diapycnal mixing — exactly the flux the Redi scheme is designed to suppress.
The error is largest where the vertical grid changes most rapidly and where isopycnal slopes are steep: shelf breaks, the Antarctic Circumpolar Current, and the upper ocean seasonal thermocline.
Note: the Redi scheme is not active by default. Impact is limited to runs with isopycnal (Redi/GM) mixing enabled.
Fix
Swap the two distance factors in all three instances of Ty/Ty1 in diff_ver_part_impl_ale (surface layer nzmin, interior loop nzmin+1 to nzmax-2, and bottom layer nzmax-1), so each cell centre is weighted by its distance to the opposite cell rather than to the interface:
No other logic is changed. On a uniform grid the fix is a no-op.