Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ using Oceananigans.ImmersedBoundaries: ImmersedBoundaryGrid
using Oceananigans.Models: AbstractModel, validate_model_halo, validate_tracer_advection, extract_boundary_conditions, initialization_update_state!
using Oceananigans.TimeSteppers: Clock, TimeStepper, update_state!, AbstractLagrangianParticles, SplitRungeKutta3TimeStepper
using Oceananigans.TurbulenceClosures: validate_closure, with_tracers, build_diffusivity_fields, add_closure_specific_boundary_conditions
using Oceananigans.TurbulenceClosures: time_discretization, implicit_diffusion_solver
using Oceananigans.TurbulenceClosures: time_discretization, implicit_diffusion_solver, closure_required_tracers
using Oceananigans.Utils: tupleit

import Oceananigans: initialize!
Expand Down Expand Up @@ -141,14 +141,35 @@ function HydrostaticFreeSurfaceModel(; grid,
@apply_regionally validate_model_halo(grid, momentum_advection, tracer_advection, closure)

if !(grid isa MutableGridOfSomeKind) && (vertical_coordinate isa ZStarCoordinate)
error("The grid does not support ZStarCoordinate vertical coordinates. Use a `MutableVerticalDiscretization` to allow the use of ZStarCoordinate (see `MutableVerticalDiscretization`).")
msg = string("The grid ", summary(grid), " does not support ZStarCoordinate.", '\n',
"z must be a MutableVerticalDiscretization to allow the use of ZStarCoordinate.")
throw(ArgumentError(msg))
end

# Validate biogeochemistry (add biogeochemical tracers automagically)
tracers = tupleit(tracers) # supports tracers=:c keyword argument (for example)
biogeochemical_fields = biogeochemical_auxiliary_fields(biogeochemistry)
tracers, biogeochemical_fields = validate_biogeochemistry(tracers, biogeochemical_fields, biogeochemistry, grid, clock)

# Automatically append closure-required tracers and disallow users from specifying them explicitly
user_tracer_names = tracernames(tracers)
closure_tracer_names = closure_required_tracers(closure)

# Throw an error in case of a conflict between user-specified tracers and any other tracers.
if any(name ∈ user_tracer_names for name in closure_tracer_names)
msg = string("The tracer names $(user_tracer_names) overlap with the closure auxiliary", '\n',
"tracer names $(closure_tracer_names) associated with $(summary(closure)).", '\n',
"The names $(closure_tracer_names) cannot be specified explicitly", '\n',
"or be the names of biogeochemical tracers.")
throw(ArgumentError(msg))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a warning, because the user should be able to manually add tracers even if using a closure that needs that tracer. This is in the spirit of making the code understandable and making sure users know what they are doing. We had this conversation some time ago in CliMA/ClimaOcean.jl#125, where we decided that it is propedeutic to require the users to manually add tracers that are needed and not automatically add them. I think that conversation still holds here. I like the idea of automatically adding tracers (like I was stating in that conversation), but I wouldn't preclude the possibility of explicitly adding tracers that are needed for parameterizations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason we need to throw an error is because users may want to add passive tracers whose names conflict with the active closure tracers. I think this is a significant enough issue to warrant an error.

I don't think there is a benefit to allowing users to write tracers = :e and have the same effect as not adding tracers at all. Rather, I think this behavior is confusing and surprising.

Regarding the conversation in ClimaOcean:

We had this conversation some time ago in CliMA/ClimaOcean.jl#125, where we decided that it is propedeutic to require the users to manually add tracers that are needed and not automatically add them.

This PR overturns that decision: after this PR, users will not manually add closure tracers.

elseif tracers isa NamedTuple
closure_tracer_fields = Tuple(CenterField(grid) for _ in closure_tracer_names)
closure_tracers = NamedTuple{closure_tracer_names}(closure_tracer_fields)
tracers = merge(tracers, closure_tracers)
else
tracers = tuple(user_tracer_names..., closure_tracer_names...)
end

# Reduce the advection order in directions that do not have enough grid points
@apply_regionally momentum_advection = validate_momentum_advection(momentum_advection, grid)
default_tracer_advection, tracer_advection = validate_tracer_advection(tracer_advection, grid)
Expand Down Expand Up @@ -259,4 +280,3 @@ initialize!(model::HydrostaticFreeSurfaceModel) = initialize_free_surface!(model

# return the total advective velocities
@inline total_velocities(model::HydrostaticFreeSurfaceModel) = model.velocities

21 changes: 19 additions & 2 deletions src/TurbulenceClosures/TurbulenceClosures.jl
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export
∂ⱼ_τ₂ⱼ,
∂ⱼ_τ₃ⱼ,

cell_diffusion_timescale
cell_diffusion_timescale,
closure_required_tracers

using KernelAbstractions
using Adapt
Expand Down Expand Up @@ -79,6 +80,23 @@ closure_summary(closure) = summary(closure)
with_tracers(tracers, closure::AbstractTurbulenceClosure) = closure
compute_diffusivities!(K, closure::AbstractTurbulenceClosure, args...; kwargs...) = nothing

# Tracer names that a closure requires (eg TKE-based closures)
# Fallbacks: by default closures do not require extra tracers.
closure_required_tracers(::Nothing) = ()
closure_required_tracers(::AbstractTurbulenceClosure) = ()
closure_required_tracers(::AbstractArray{<:AbstractTurbulenceClosure}) = ()
closure_required_tracers(closures::Tuple) = begin
names = Symbol[]
for c in closures
for n in closure_required_tracers(c)
if !(n in names)
push!(names, n)
end
end
end
return Tuple(names)
end

# The required halo size to calculate diffusivities. Take care that if the diffusivity can
# be calculated from local information, still `B = 1`, because we need at least one additional
# point at each side to calculate viscous fluxes at the edge of the domain.
Expand Down Expand Up @@ -210,4 +228,3 @@ include("turbulence_closure_diagnostics.jl")
#####

end # module

Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ function with_tracers(tracer_names, closure::FlavorOfCATKE)
return closure
end

# Required tracer names for CATKE
closure_required_tracers(::FlavorOfCATKE) = tuple(:e)

# For tuples of closures, we need to know _which_ closure is CATKE.
# Here we take a "simple" approach that sorts the tuple so CATKE is first.
# This is not sustainable though if multiple closures require this.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ function with_tracers(tracer_names, closure::FlavorOfTD)
return closure
end

# Required tracer names for TKEDissipation
closure_required_tracers(::FlavorOfTD) = (:e, :ϵ)

#####
##### Stratified displacement length scale limiter
#####
Expand Down
2 changes: 1 addition & 1 deletion test/benchmark_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function ocean_benchmark(arch, Nx, Ny, Nz, topology, immersed, tracer_advection=
buoyancy,
closure,
free_surface,
tracers = (:T, :S, :e))
tracers = (:T, :S))

@info "Model is built"

Expand Down
3 changes: 1 addition & 2 deletions test/test_hydrostatic_free_surface_models.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ function time_step_hydrostatic_model_works(grid;
velocities = nothing)

buoyancy = BuoyancyTracer()
closure isa CATKEVerticalDiffusivity && push!(tracers, :e)

model = HydrostaticFreeSurfaceModel(; grid, coriolis, tracers, velocities, buoyancy,
momentum_advection, tracer_advection, free_surface, closure)
Expand Down Expand Up @@ -68,7 +67,7 @@ function time_step_hydrostatic_model_with_catke_works(arch, FT)
model = HydrostaticFreeSurfaceModel(;
grid,
buoyancy = BuoyancyTracer(),
tracers = (:b, :e),
tracers = (:b,),
closure = CATKEVerticalDiffusivity(FT)
)

Expand Down
13 changes: 8 additions & 5 deletions test/test_turbulence_closures.jl
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ function run_catke_tke_substepping_tests(arch, closure)
grid = RectilinearGrid(arch, size=(2, 2, 2), extent=(100, 200, 300))

model = HydrostaticFreeSurfaceModel(; grid, momentum_advection = nothing, tracer_advection = nothing,
closure, buoyancy=BuoyancyTracer(), tracers=(:b, :e))
closure, buoyancy=BuoyancyTracer(), tracers=(:b))

# set random velocities
Random.seed!(1234)
Expand Down Expand Up @@ -209,14 +209,17 @@ function run_time_step_with_catke_tests(arch, closure)
grid = RectilinearGrid(arch, size=(2, 2, 2), extent=(1, 2, 3))
buoyancy = BuoyancyTracer()

# These shouldn't work (need :e in tracers)
@test_throws ArgumentError HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers=:b)
@test_throws ArgumentError HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers=(:b, :E))
# These should work now (:e is auto-appended)
@test HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers=:b) isa HydrostaticFreeSurfaceModel
@test HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers=(:b, :E)) isa HydrostaticFreeSurfaceModel

# CATKE isn't supported with NonhydrostaticModel (we don't diffuse vertical velocity)
@test_throws ErrorException NonhydrostaticModel(; grid, closure, buoyancy, tracers=(:b, :c, :e))

model = HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers = (:b, :c, :e))
# Supplying closure tracers explicitly should error
@test_throws ArgumentError HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers = (:b, :c, :e))

model = HydrostaticFreeSurfaceModel(; grid, closure, buoyancy, tracers = (:b, :c))

# Default boundary condition is Flux, Nothing... with CATKE this has to change.
@test !(model.tracers.e.boundary_conditions.top.condition isa BoundaryCondition{Flux, Nothing})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function run_hydrostatic_simulation!(grid_size, ranks, FT::DataType = Float64;
coriolis,
closure,
free_surface,
tracers = (:T, :S, :e),
tracers = (:T, :S),
buoyancy,
timestepper)

Expand Down
2 changes: 1 addition & 1 deletion validation/mesoscale_turbulence/eddying_channel.jl
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ model = HydrostaticFreeSurfaceModel(grid = grid,
buoyancy = BuoyancyTracer(),
coriolis = coriolis,
closure = (horizontal_closure, vertical_closure, catke),
tracers = (:b, :e, :c),
tracers = (:b, :c),
boundary_conditions = (b=b_bcs, u=u_bcs, v=v_bcs),
forcing = (; b=Fb,)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ model = HydrostaticFreeSurfaceModel(grid = grid,
coriolis = coriolis,
#closure = (horizontal_diffusivity, convective_adjustment, gent_mcwilliams_diffusivity),
closure = (catke, diffusive_closure..., gent_mcwilliams_diffusivity),
tracers = (:b, :e, :c),
tracers = (:b, :c),
boundary_conditions = (b=b_bcs, u=u_bcs, v=v_bcs),
forcing = (; b=Fb))

Expand Down
3 changes: 1 addition & 2 deletions validation/vertical_mixing_closures/bottom_boundary_layer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ u_bcs = FieldBoundaryConditions(bottom = u_bottom_bc)
v_bcs = FieldBoundaryConditions(bottom = v_bottom_bc)

model = HydrostaticFreeSurfaceModel(; grid, closure, coriolis,
tracers = (:b, :e),
tracers = (:b,),
buoyancy = BuoyancyTracer(),
boundary_conditions = (; u=u_bcs, v=v_bcs))

Expand Down Expand Up @@ -113,4 +113,3 @@ record(fig, "bottom_boundary_layer.mp4", 1:Nt, framerate=12) do nn
n[] = nn
end


Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ end
for closure in closures_to_run

model = HydrostaticFreeSurfaceModel(; grid, closure, coriolis,
tracers = (:b, :e, :ϵ),
tracers = (:b,),
buoyancy = BuoyancyTracer(),
boundary_conditions = (; b=b_bcs, u=u_bcs))

Expand Down Expand Up @@ -165,4 +165,3 @@ display(fig)
# record(fig, "windy_convection.mp4", 1:Nt, framerate=24) do nn
# n[] = nn
# end

Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ for closure in (k_epsilon, catke) #, k_epsilon_const_stability)
global model

model = HydrostaticFreeSurfaceModel(; grid, closure, coriolis,
tracers = (:b, :e, :ϵ),
tracers = (:b,),
buoyancy = BuoyancyTracer(),
boundary_conditions=(u=u_bcs, b=b_bcs))

Expand Down Expand Up @@ -111,4 +111,3 @@ lines!(axϵ, ϵn[3], z, color=colors[3])
Legend(fig[0, 1:4], axe, nbanks=3, framevisible=false, tellheight=true)

fig

2 changes: 1 addition & 1 deletion validation/vertical_mixing_closures/gpu_tkevd_ensemble.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ coriolis_ensemble = CuArray([FPlane(f=f_ij(i, j)) for i=1:Ex, j=1:Ey])

model = HydrostaticFreeSurfaceModel(architecture = GPU(),
grid = grid,
tracers = (:b, :e),
tracers = (:b,),
buoyancy = BuoyancyTracer(),
coriolis = coriolis_ensemble,
boundary_conditions = (b=b_bcs, u=u_bcs, v=v_bcs),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ model = HydrostaticFreeSurfaceModel(; grid, closure,
momentum_advection = WENO(),
tracer_advection = WENO(),
coriolis = FPlane(f=1e-4),
tracers = (:b, :e),
tracers = (:b,),
boundary_conditions = (; b=b_bcs, u=u_bcs),
buoyancy = BuoyancyTracer())

Expand Down Expand Up @@ -249,4 +249,3 @@ record(fig, filename[1:end-5] * ".mp4", 1:Nt, framerate=24) do nn
n[] = nn
end
=#

Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ closure = TKEDissipationVerticalDiffusivity()
#closure = CATKEVerticalDiffusivity()

model = HydrostaticFreeSurfaceModel(; grid, closure, coriolis,
tracers = (:b, :e, :ϵ),
tracers = (:b,),
buoyancy = BuoyancyTracer(),
boundary_conditions=(u=u_bcs, b=b_bcs))

Expand Down Expand Up @@ -120,4 +120,3 @@ lines!(axs, 𝕊ᶜn, zf, label="𝕊ᶜ")
axislegend(axs, position=:rb)

fig

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ boundary_conditions = (b = FieldBoundaryConditions(top = FluxBoundaryCondition(1
u = FieldBoundaryConditions(top = FluxBoundaryCondition(-2e-4)))

model = HydrostaticFreeSurfaceModel(; grid, closure, coriolis, boundary_conditions,
tracers = (:b, :e), buoyancy = BuoyancyTracer())
tracers = (:b,), buoyancy = BuoyancyTracer())

bᵢ(z) = 1e-6 * z
set!(model, b=bᵢ, e=1e-6)
Expand All @@ -22,4 +22,3 @@ time_step!(model, 600)
time_step!(model, 600)
end


Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ closure = (VerticalScalarDiffusivity(VerticallyImplicitTimeDiscretization(), κ=
CATKEVerticalDiffusivity())

model = HydrostaticFreeSurfaceModel(; grid, closure,
tracers = (:b, :e),
tracers = (:b,),
buoyancy = BuoyancyTracer())

bᵢ(z) = 1e-5 * z
set!(model, b = bᵢ)
simulation = Simulation(model, Δt=1minute, stop_iteration=10)

run!(simulation)

Loading