Skip to content

Commit c69b6eb

Browse files
glwagnerclaude
glwagner
and
claude
authored
Fix ContinuousBoundaryFunction dispatch ambiguity on Flat grids (#5472)
* Fix ContinuousBoundaryFunction dispatch ambiguity on Flat grids When a grid dimension is Flat, its location type is Nothing, which collides with the convention that the boundary-normal dimension also has location Nothing. This caused method ambiguity errors for ContinuousBoundaryFunction getbc on grids like (Flat, Flat, Bounded). Add grid-type-constrained disambiguation methods for all combinations of two and three Nothing location parameters, for both domain boundary (2-index) and immersed boundary (3-index) getbc methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use normal FieldBoundaryConditions constructor in test Use regularize_field_boundary_conditions to process BCs, matching how model constructors handle user-provided boundary conditions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Replace grid-type disambiguation with BoundaryNormal sentinel type Instead of adding grid-type-constrained methods (which created more ambiguities than it resolved), use a BoundaryNormal sentinel type for the boundary-normal direction in ContinuousBoundaryFunction. Previously, Nothing was used for both Flat dimensions and boundary- normal directions, causing dispatch ambiguity. BoundaryNormal is a distinct type that unambiguously marks the boundary-normal direction, eliminating all practical ambiguities without adding any new methods. Aqua recursive ambiguity count: 343 (baseline) → 325 (with fix). CBF-specific ambiguities: 6 theoretical (unchanged, unreachable). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update location tests to expect BoundaryNormal instead of Nothing The boundary-normal direction now uses BoundaryNormal instead of Nothing, so the location assertions in the integration tests need to be updated accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add field_dependencies test for ContinuousBoundaryFunction on Flat grid Exercises the interpolation_code(::BoundaryNormal, ...) methods to improve patch coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * better name for new location * Improve coverage: build and time-step model on Flat grid Replace explicit interpolation_code tests with a full model build and time_step! on a (Flat, Flat, Bounded) grid with field-dependent ContinuousBoundaryFunction. This exercises the complete regularization path including all BoundaryAdjacent interpolation_code methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 446b88e commit c69b6eb

File tree

3 files changed

+112
-34
lines changed

3 files changed

+112
-34
lines changed

‎src/BoundaryConditions/continuous_boundary_function.jl‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Oceananigans.Architectures: Architectures, on_architecture
22
using Oceananigans.Operators: index_and_interp_dependencies
3+
import Oceananigans.Operators: interpolation_code
34
using Oceananigans.Utils: Utils, tupleit, user_function_arguments, prettysummary
45
using Oceananigans.Grids: XFlatGrid, YFlatGrid, ZFlatGrid, YZFlatGrid, XZFlatGrid, XYFlatGrid
56
using Oceananigans.Grids: ξnode, ηnode, rnode
@@ -9,6 +10,25 @@ import Oceananigans: location
910
struct LeftBoundary end
1011
struct RightBoundary end
1112

13+
"""
14+
BoundaryAdjacent
15+
16+
Sentinel type used as the location in the boundary-normal direction for
17+
`ContinuousBoundaryFunction`. Unlike `Nothing` (which also represents `Flat`
18+
dimensions), `BoundaryAdjacent` unambiguously marks the boundary-normal direction,
19+
preventing dispatch ambiguities on grids with `Flat` topologies.
20+
"""
21+
struct BoundaryAdjacent end
22+
23+
# BoundaryAdjacent acts like Nothing for interpolation purposes
24+
interpolation_code(::Type{BoundaryAdjacent}) = :ᵃ
25+
interpolation_code(::BoundaryAdjacent) = :ᵃ
26+
interpolation_code(::BoundaryAdjacent, to) = :ᵃ
27+
interpolation_code(from, ::BoundaryAdjacent) = :ᵃ
28+
interpolation_code(::BoundaryAdjacent, ::BoundaryAdjacent) = :ᵃ
29+
interpolation_code(::BoundaryAdjacent, ::Nothing) = :ᵃ
30+
interpolation_code(::Nothing, ::BoundaryAdjacent) = :ᵃ
31+
1232
"""
1333
struct ContinuousBoundaryFunction{X, Y, Z, I, F, P, D, N, ℑ} <: Function
1434
@@ -61,7 +81,7 @@ returning `BoundaryCondition(C, regularized_condition)`.
6181
The regularization of `bc.condition::ContinuousBoundaryFunction` requries
6282
6383
1. Setting the boundary location to `LX, LY, LZ`.
64-
The location in the boundary-normal direction is `Nothing`.
84+
The location in the boundary-normal direction is `BoundaryAdjacent`.
6585
6686
2. Setting the boundary-normal index `I` for indexing into `field_dependencies`.
6787
`I` is either `1` (for left boundaries) or
@@ -76,8 +96,8 @@ The regularization of `bc.condition::ContinuousBoundaryFunction` requries
7696
function regularize_boundary_condition(boundary_func::ContinuousBoundaryFunction,
7797
grid, loc, dim, Side, field_names)
7898

79-
# Set boundary-normal location to Nothing:
80-
LX, LY, LZ = Tuple(i == dim ? Nothing : destantiate(loc[i]) for i = 1:3)
99+
# Set boundary-normal location to BoundaryAdjacent:
100+
LX, LY, LZ = Tuple(i == dim ? BoundaryAdjacent : destantiate(loc[i]) for i = 1:3)
81101

82102
indices, interps = index_and_interp_dependencies(LX, LY, LZ,
83103
boundary_func.field_dependencies,
@@ -116,9 +136,9 @@ end
116136
@inline z_boundary_node(i, j, k, grid::YFlatGrid, ℓx, ℓy) = tuple(ξnode(i, j, k, grid, ℓx, nothing, Face()))
117137
@inline z_boundary_node(i, j, k, grid::XYFlatGrid, ℓx, ℓy) = tuple()
118138

119-
const XBoundaryFunction{LY, LZ, S} = ContinuousBoundaryFunction{Nothing, LY, LZ, S} where {LY, LZ, S}
120-
const YBoundaryFunction{LX, LZ, S} = ContinuousBoundaryFunction{LX, Nothing, LZ, S} where {LX, LZ, S}
121-
const ZBoundaryFunction{LX, LY, S} = ContinuousBoundaryFunction{LX, LY, Nothing, S} where {LX, LY, S}
139+
const XBoundaryFunction{LY, LZ, S} = ContinuousBoundaryFunction{BoundaryAdjacent, LY, LZ, S} where {LY, LZ, S}
140+
const YBoundaryFunction{LX, LZ, S} = ContinuousBoundaryFunction{LX, BoundaryAdjacent, LZ, S} where {LX, LZ, S}
141+
const ZBoundaryFunction{LX, LY, S} = ContinuousBoundaryFunction{LX, LY, BoundaryAdjacent, S} where {LX, LY, S}
122142

123143
# Return ContinuousBoundaryFunction on east or west boundaries.
124144
@inline function getbc(cbf::XBoundaryFunction{LY, LZ, S}, j::Integer, k::Integer,

‎test/test_boundary_conditions.jl‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ include("dependencies_for_runtests.jl")
33
using Oceananigans.BoundaryConditions: PBC, ZFBC, VBC, OBC, Zipper
44
using Oceananigans.BoundaryConditions: Mixed, MixedBoundaryCondition
55
using Oceananigans.BoundaryConditions: Zipper, ContinuousBoundaryFunction, DiscreteBoundaryFunction, regularize_field_boundary_conditions
6+
using Oceananigans.BoundaryConditions: compute_x_bcs!, compute_y_bcs!, compute_z_bcs!
67
using Oceananigans.Fields: Face, Center
78

89
simple_bc(ξ, η, t) = exp(ξ) * cos(η) * sin(t)
@@ -344,6 +345,62 @@ end
344345
@test all(f.data[1:10, 0, 1:10] .== f.data[1:10, 1, 1:10])
345346
@test all(f.data[1:10, 11, 1:10] .== f.data[1:10, 10, 1:10])
346347

348+
# ContinuousBoundaryFunction on Flat topologies (dispatch disambiguation)
349+
@info " Testing ContinuousBoundaryFunction on Flat topologies..."
350+
for topo in ((Flat, Flat, Bounded), (Flat, Bounded, Flat), (Bounded, Flat, Flat))
351+
bounded_dim = findfirst(t -> t === Bounded, topo)
352+
353+
grid_kw = Dict{Symbol,Any}(:topology => topo)
354+
if topo[1] !== Flat; grid_kw[:x] = (0, 1); end
355+
if topo[2] !== Flat; grid_kw[:y] = (0, 1); end
356+
if topo[3] !== Flat; grid_kw[:z] = (0, 1); end
357+
grid_kw[:size] = Tuple(4 for i in 1:3 if topo[i] !== Flat)
358+
359+
grid = RectilinearGrid(; grid_kw...)
360+
loc = (Center(), Center(), Center())
361+
362+
bc_func(t) = 1.0
363+
left_bc = FluxBoundaryCondition(bc_func)
364+
right_bc = FluxBoundaryCondition(bc_func)
365+
366+
if bounded_dim == 1
367+
bcs = FieldBoundaryConditions(west=left_bc, east=right_bc)
368+
elseif bounded_dim == 2
369+
bcs = FieldBoundaryConditions(south=left_bc, north=right_bc)
370+
else
371+
bcs = FieldBoundaryConditions(bottom=left_bc, top=right_bc)
372+
end
373+
374+
# Regularize like model constructors do
375+
bcs = regularize_field_boundary_conditions(bcs, grid, loc)
376+
c = CenterField(grid; boundary_conditions=bcs)
377+
Gc = CenterField(grid)
378+
clock = (; time = 0.0)
379+
380+
if bounded_dim == 1
381+
compute_x_bcs!(Gc, c, CPU(), clock, tuple())
382+
elseif bounded_dim == 2
383+
compute_y_bcs!(Gc, c, CPU(), clock, tuple())
384+
else
385+
compute_z_bcs!(Gc, c, CPU(), clock, tuple())
386+
end
387+
388+
@test !all(Array(interior(Gc)) .== 0)
389+
end
390+
391+
# ContinuousBoundaryFunction with field_dependencies on a Flat topology
392+
# Building and time-stepping a model exercises the full regularization
393+
# path including interpolation_code for BoundaryAdjacent.
394+
@info " Testing ContinuousBoundaryFunction with field_dependencies on Flat topology..."
395+
grid = RectilinearGrid(size=4, z=(0, 1), topology=(Flat, Flat, Bounded))
396+
dep_bc(t, T) = T # no spatial args on (Flat, Flat, Bounded)
397+
bottom_bc = FluxBoundaryCondition(dep_bc, field_dependencies=:T)
398+
model = NonhydrostaticModel(grid; tracers=:T,
399+
boundary_conditions=(T=FieldBoundaryConditions(bottom=bottom_bc),))
400+
set!(model, T=1)
401+
time_step!(model, 1e-3)
402+
@test model.clock.iteration == 1
403+
347404
# Minimal test for PolarValueBoundaryCondition
348405
polar_grid = LatitudeLongitudeGrid(size=(10, 10, 10), latitude=(-90, 90), longitude=(0, 360), z = (0, 1))
349406
c = CenterField(polar_grid)

‎test/test_boundary_conditions_integration.jl‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
include("dependencies_for_runtests.jl")
22

3-
using Oceananigans.BoundaryConditions: ContinuousBoundaryFunction,
3+
using Oceananigans.BoundaryConditions: ContinuousBoundaryFunction, BoundaryAdjacent,
44
fill_halo_regions!
55

66
using Oceananigans: prognostic_fields
@@ -234,33 +234,34 @@ test_boundary_conditions(C, FT, ArrayType) = (integer_bc(C, FT, ArrayType),
234234

235235
model = NonhydrostaticModel(grid; boundary_conditions, buoyancy = SeawaterBuoyancy(), tracers = (:T, :S))
236236

237-
@test location(model.velocities.u.boundary_conditions.bottom.condition) == (Face, Center, Nothing)
238-
@test location(model.velocities.u.boundary_conditions.top.condition) == (Face, Center, Nothing)
239-
@test location(model.velocities.u.boundary_conditions.north.condition) == (Face, Nothing, Center)
240-
@test location(model.velocities.u.boundary_conditions.south.condition) == (Face, Nothing, Center)
241-
@test location(model.velocities.u.boundary_conditions.east.condition) == (Nothing, Center, Center)
242-
@test location(model.velocities.u.boundary_conditions.west.condition) == (Nothing, Center, Center)
243-
244-
@test location(model.velocities.v.boundary_conditions.bottom.condition) == (Center, Face, Nothing)
245-
@test location(model.velocities.v.boundary_conditions.top.condition) == (Center, Face, Nothing)
246-
@test location(model.velocities.v.boundary_conditions.north.condition) == (Center, Nothing, Center)
247-
@test location(model.velocities.v.boundary_conditions.south.condition) == (Center, Nothing, Center)
248-
@test location(model.velocities.v.boundary_conditions.east.condition) == (Nothing, Face, Center)
249-
@test location(model.velocities.v.boundary_conditions.west.condition) == (Nothing, Face, Center)
250-
251-
@test location(model.velocities.w.boundary_conditions.bottom.condition) == (Center, Center, Nothing)
252-
@test location(model.velocities.w.boundary_conditions.top.condition) == (Center, Center, Nothing)
253-
@test location(model.velocities.w.boundary_conditions.north.condition) == (Center, Nothing, Face)
254-
@test location(model.velocities.w.boundary_conditions.south.condition) == (Center, Nothing, Face)
255-
@test location(model.velocities.w.boundary_conditions.east.condition) == (Nothing, Center, Face)
256-
@test location(model.velocities.w.boundary_conditions.west.condition) == (Nothing, Center, Face)
257-
258-
@test location(model.tracers.T.boundary_conditions.bottom.condition) == (Center, Center, Nothing)
259-
@test location(model.tracers.T.boundary_conditions.top.condition) == (Center, Center, Nothing)
260-
@test location(model.tracers.T.boundary_conditions.north.condition) == (Center, Nothing, Center)
261-
@test location(model.tracers.T.boundary_conditions.south.condition) == (Center, Nothing, Center)
262-
@test location(model.tracers.T.boundary_conditions.east.condition) == (Nothing, Center, Center)
263-
@test location(model.tracers.T.boundary_conditions.west.condition) == (Nothing, Center, Center)
237+
BN = BoundaryAdjacent
238+
@test location(model.velocities.u.boundary_conditions.bottom.condition) == (Face, Center, BN)
239+
@test location(model.velocities.u.boundary_conditions.top.condition) == (Face, Center, BN)
240+
@test location(model.velocities.u.boundary_conditions.north.condition) == (Face, BN, Center)
241+
@test location(model.velocities.u.boundary_conditions.south.condition) == (Face, BN, Center)
242+
@test location(model.velocities.u.boundary_conditions.east.condition) == (BN, Center, Center)
243+
@test location(model.velocities.u.boundary_conditions.west.condition) == (BN, Center, Center)
244+
245+
@test location(model.velocities.v.boundary_conditions.bottom.condition) == (Center, Face, BN)
246+
@test location(model.velocities.v.boundary_conditions.top.condition) == (Center, Face, BN)
247+
@test location(model.velocities.v.boundary_conditions.north.condition) == (Center, BN, Center)
248+
@test location(model.velocities.v.boundary_conditions.south.condition) == (Center, BN, Center)
249+
@test location(model.velocities.v.boundary_conditions.east.condition) == (BN, Face, Center)
250+
@test location(model.velocities.v.boundary_conditions.west.condition) == (BN, Face, Center)
251+
252+
@test location(model.velocities.w.boundary_conditions.bottom.condition) == (Center, Center, BN)
253+
@test location(model.velocities.w.boundary_conditions.top.condition) == (Center, Center, BN)
254+
@test location(model.velocities.w.boundary_conditions.north.condition) == (Center, BN, Face)
255+
@test location(model.velocities.w.boundary_conditions.south.condition) == (Center, BN, Face)
256+
@test location(model.velocities.w.boundary_conditions.east.condition) == (BN, Center, Face)
257+
@test location(model.velocities.w.boundary_conditions.west.condition) == (BN, Center, Face)
258+
259+
@test location(model.tracers.T.boundary_conditions.bottom.condition) == (Center, Center, BN)
260+
@test location(model.tracers.T.boundary_conditions.top.condition) == (Center, Center, BN)
261+
@test location(model.tracers.T.boundary_conditions.north.condition) == (Center, BN, Center)
262+
@test location(model.tracers.T.boundary_conditions.south.condition) == (Center, BN, Center)
263+
@test location(model.tracers.T.boundary_conditions.east.condition) == (BN, Center, Center)
264+
@test location(model.tracers.T.boundary_conditions.west.condition) == (BN, Center, Center)
264265
end
265266

266267
@testset "Boundary condition time-stepping works" begin

0 commit comments

Comments
 (0)