Fix biharmonic velocity mixing at boundary vertices#456
Open
xylar wants to merge 1 commit into
Open
Conversation
Compute the Laplacian of relative vorticity (Del2RelVortVertex) over the full vertex valid range [MinLayerVertexTop, MaxLayerVertexBot] and clamp each edge contribution to that edge's valid range [MinLayerEdgeTop, MaxLayerEdgeBot], matching VorticityAuxVars::computeVarsOnVertex. Previously Del2RelVortVertex was computed only over the narrower [MinLayerVertexBot, MaxLayerVertexTop] range (layers where every surrounding cell is active) and summed Del2Edge with no per-edge clamp. The biharmonic velocity tendency (VelocityHyperDiffOnEdge) reads Del2RelVortVertex over the edge range up to MaxLayerEdgeTop, which for a deep edge sharing a vertex with a shallower cell exceeds MaxLayerVertexTop. Those boundary-vertex layers were never computed, so on partial-bottom meshes the biharmonic term was under-computed there. This is a latent correctness bug on its own; on the fill-values branch the uncomputed layers hold FillValueReal, so the term instead blew up to ~1e45, corrupting NormalVelocity and, through the flux divergence and vertical advection, PseudoThickness and the tracers (surfaced by the new state validation in DRIVER_TEST). The wider range gives boundary vertices a valid, generally non-zero value from their active edges; inactive edges contribute zero via the per-edge clamp and Del2Edge's zeroed boundary band, so no fill value is read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
TestingI tested this fix as part of #428 CTest unit tests:
There are no |
This was referenced Jul 2, 2026
Author
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.
Compute the Laplacian of relative vorticity (
Del2RelVortVertex) over the full vertex valid range[MinLayerVertexTop, MaxLayerVertexBot]and clamp each edge contribution to that edge's valid range[MinLayerEdgeTop, MaxLayerEdgeBot], matchingVorticityAuxVars::computeVarsOnVertex.Previously
Del2RelVortVertexwas computed only over the narrower[MinLayerVertexBot, MaxLayerVertexTop]range (layers where every surrounding cell is active) and summedDel2Edgewith no per-edge clamp. The biharmonic velocity tendency (VelocityHyperDiffOnEdge) readsDel2RelVortVertexover the edge range up toMaxLayerEdgeTop, which for a deep edge sharing a vertex with a shallower cell exceedsMaxLayerVertexTop. Those boundary-vertex layers were never computed, so on partial-bottom meshes the biharmonic term was under-computed there. This is a latent correctness bug on its own; on the fill-values branch (#428) the uncomputed layers holdFillValueReal, so the term instead blew up to ~1e45, corruptingNormalVelocityand, through the flux divergence and vertical advection,PseudoThicknessand the tracers (surfaced by the new state validation inDRIVER_TEST).The wider range gives boundary vertices a valid, generally non-zero value from their active edges; inactive edges contribute zero via the per-edge clamp and
Del2Edge's zeroed boundary band, so no fill value is read.Checklist
Testingwith the following:have been run on and indicate that are all passing.