Divide a prescribed energy flux by the Exner function - #1020
Conversation
`getbc coverage for all boundary faces` asserts that one step with a prescribed `ρE` surface flux moves `ρθ`. At Δt = 1e-6 the increment 𝒬 Az Δt / (cᵖᵐ V) is about 9.9e-9 against ρθ ≈ 353, whose Float32 spacing is 3.05e-5 — three thousand times larger. Measured Δρθ is 9.85e-9 in Float64 and exactly 0.0 in Float32, so in single precision the assertion could only ever pass on rounding noise from the rest of the tendency. This has been latent because `test_float_types()` returns `(Float64,)` unless `BREEZE_TEST_FLOAT32=true`, which is set nowhere in the repo, so CI has never exercised it. Raising Δt to 1 s puts the increment at 9.9e-3, two orders above the spacing, and the test then measures what it claims to in both precisions. One step of an anelastic single-cell model has no stability constraint that 1 s approaches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
An energy input reached the `ρθ` budget through two different conversions depending on how it arrived. The interior `ρE` forcing and the radiative flux divergence divide by `cᵖᵐ Π`; a prescribed flux under the same key divided by `cᵖᵐ` alone. Equating the two tendencies fixes the conversion: Oceananigans adds `getbc * Az / V` to `Gρθ`, while a volumetric source of the same strength enters as `FρE / (cᵖᵐ Π)`, so a flux must enter as `𝒬 / (cᵖᵐ Π)`. Physically, with `𝒬 = ρ cᵖᵐ ⟨w'T'⟩` and `T = Π θ + (ℒˡᵣ qˡ + ℒⁱᵣ qⁱ) / cᵖᵐ`, holding moisture fixed gives `δT = Π δθ`, so the condensate term cancels and the relation holds saturated as well as dry. Closes #976. Π comes from `dynamics_pressure`, reached through the `dynamics_fields` argument `getbc` already carried but these methods ignored. That is bit-identical to what the anelastic `diagnose_thermodynamic_state` reads, so flux and forcing divide by the same number. The compressible core instead diagnoses a density state whose pressure is ρRᵐT — which is what `dynamics.pressure` holds, written by the same inversion during `update_state!`, so the two agree up to its staleness inside an RK stage. Both wrapper structs gain `standard_pressure`, converted to the grid's float type at materialization: boundary conditions materialize from the stub dynamics, before `materialize_dynamics` normalizes, so on a Float32 grid the constructor would otherwise see a Float64 pˢᵗ and throw. Omitting the field is left a MethodError rather than given a convenience constructor — unlike `density`, which resolves at runtime through the #777 `Nothing` fallback, there is nothing to fall back to. `Jᶿ_to_𝒬` takes the same factor so the inverse stays the inverse; a flux supplied under `ρE` is unwrapped rather than reconverted, and round-trips exactly. Measured: the flux is unchanged at 1000 hPa, 4.7% larger at 850 hPa and 10.7% larger at 700 hPa. Every coupled NumericalEarth run picks this up through the `Jᵉ` field its coupler writes, so surface heating over elevated terrain changes by that much and existing baselines no longer reproduce. The test now runs two base pressures. A single column would have caught this change — at the default base pressure Π = 1.003, a relative difference of 3.3e-3, about ten times isapprox's Float32 tolerance — but it cannot distinguish a Π that tracks the column's pressure from a constant that happens to be right there. `𝒬 / cᵖᵐ` is identical at both pressures, so the ratio between them tests that the conversion varies correctly, and being a ratio it depends on no tolerance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
#974 wrote the flux/forcing asymmetry into the docs as intended behavior: `total_energy_density_name` said an energy input is divided by `cᵖᵐ` for fluxes and `cᵖᵐ Π` for forcings, and the boundary-condition page justified the split by noting the forcing "is applied to a potential temperature rather than a temperature" — which is equally true of the flux. Both now state the single conversion. The page's derivation also went from a dynamic flux to a kinematic one, dropping the ρ that `Jᶿ` carries, and asserted `θ = T / Π`, which holds only without condensate. The differential form is both correct and stronger. `Jᶿ` joins the notation table. It had `Jᵀ` but not `Jᶿ`, and they differ by exactly the Exner factor, so the distinction is now load-bearing. A bare `𝒬` needs no row: every `𝒬` in that table carries a superscript naming its flux, and `𝒬 = cᵖᵐ Π Jᶿ = cᵖᵐ Jᵀ` is the `𝒬ᵀ` already there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
49964c5 to
059bcee
Compare
|
Another check to make sure this fix doesn't break anything else: what does the coupler actually put under I.e., if it supplied Traced through NumericalEarth ( # InterfaceComputations/interface_states.jl:308
# "Temperature increment including the ``lapse rate'' α = g / cᵖᵐ"
surface_atmosphere_temperature(Ψₐ, ℙₐ) = Tᵃᵗ + g * Δh / cᵃᵗ
# InterfaceComputations/compute_interface_state.jl:104
θᵃᵗ = surface_atmosphere_temperature(atmosphere_state, atmosphere_properties)
Δθ = θᵃᵗ - Tₛ
# InterfaceComputations/coefficient_based_turbulent_fluxes.jl:359
θ★ = Ch / sqrt(Cd) * Δθ
# InterfaceComputations/atmosphere_interface_kernels.jl:56
interface_fluxes.sensible_heat[i, j, 1] = - ρᵃᵗ * cᵖᵐ * u★ * θ★and that value reaches the
So Breeze's
Only the reference level separates those two cases. Therefore A naming question this raises. The function is 🤖 Generated with Claude Code |
`𝒬_to_Jᶿ` took a bare `𝒬`, which does not say which energy flux. Handing it a latent heat flux would be wrong — that energy is carried by the moisture flux, as the coupler that writes this field notes — so the valid input is specifically an enthalpy flux, `cᵖᵐ` times a temperature flux. That is `𝒬ᵀ` as the notation table defines it, and the table gives every `𝒬` a superscript naming its flux: `𝒬ᵀ = cᵖᵐ Jᵀ`, `𝒬ᵛ = ℒˡ Jᵛ`. What arrives under `ρE` from a coupled model is `𝒬ᵀ`. NumericalEarth's similarity theory forms `θᵃᵗ` by lifting the lowest-level air dry-adiabatically through the MOST reference height — the cell-centre elevation above ground, per column on a terrain-following grid — and differences it against the skin temperature at that same ground. A potential temperature referenced to the ground is numerically the temperature there, so `- ρ cᵖᵐ u★ θ★` is `ρ cᵖᵐ ⟨w'T'⟩`. Had it instead been referenced to `pˢᵗ`, the flux would already be `cᵖᵐ Jᶿ` and the preceding commit's division by `Π` would double-count. Same class of correction as that commit, applied to the other end of the signature: the function returned `Jᵀ` while claiming `Jᶿ`; it accepted `𝒬ᵀ` while claiming an unspecified `𝒬`. Mechanical and confined — `𝒬ᵀ_to_Jᶿ` and `Jᶿ_to_𝒬ᵀ` are internal to this file and not exported, so a maintainer preferring the bare spelling can revert it in one pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
The six `getbc` methods differ in exactly one thing: which boundary cell they hand to the conversion — `(i,j,1)` for bottom, `(i,j,Nz)` for top, `(1,j,k)` for west, and so on. Nothing asserted that. `getbc coverage for all boundary faces` covers bottom and west, and asserts only that `ρθ` moved, which a conversion reading the wrong cell also satisfies. Adding `Π` raised the cost of getting that index wrong. It previously reached only `qᵛ` and density, which vary weakly, so a misread cell was a percent. `Π` follows pressure: over the column used here it runs ≈0.97 at the lowest cell to ≈0.53 at the highest, so the same mistake is now most of a factor of two. So this checks the value rather than the motion: for each of the six faces, that the boundary condition returns `𝒬ᵀ / (cᵖᵐ Π)` with `Π` taken at that face's own cell, on a fully bounded 15 km column where the faces genuinely disagree. That premise is asserted first rather than last: if `Π` barely varied, the six would pass under any index, so it belongs before them, not after. No `θ` is set. Nothing the conversion reads depends on it — the density and pressure are the anelastic reference fields, built at construction, and `exner_function` ignores the state's `θ`, which is the property the whole conversion rests on. Setting it would imply otherwise. That holds only while the default microphysics is `nothing`: with one, `set!` would split `qᵗ` into condensate using temperature and the vapor-only expectation here would stop matching. What it does not catch: the anelastic reference pressure is a column, so `Π` has no horizontal structure and a pure i↔j or 1↔N mix-up between two horizontal faces is invisible. Errors in the vertical index, and errors confusing a horizontal index with it, do show. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
b7607ba to
a10cb79
Compare
| Jᶿ = Array(interior(Field(BoundaryConditionOperation(ρθ, side, model)))) | ||
| expected = [𝒬ᵀ / (cᵖᵐ * Π_cell(c...)) for c in getproperty(cells, side)] | ||
| @test vec(Jᶿ) ≈ vec(expected) |
There was a problem hiding this comment.
may be possible to use more of the built in Field infra here, eg avoiding Array(interior()) chain. Typical claudism.
| p = Array(interior(dynamics_pressure(model.dynamics))) | ||
| Nx, Ny, Nz = size(grid) | ||
|
|
||
| # The anelastic reference pressure is a column, so `Π` varies in `k` only. That is enough to | ||
| # catch any error in the vertical index, and any error that confuses a horizontal index with | ||
| # it; a pure i↔j or 1↔N mix-up between two horizontal faces would not show here. | ||
| p_cell(i, j, k) = FT(p[min(i, size(p, 1)), min(j, size(p, 2)), k]) | ||
| Π_cell(i, j, k) = exner_function(LiquidIcePotentialTemperatureState(zero(FT), q, pˢᵗ, p_cell(i, j, k)), | ||
| constants) | ||
|
|
||
| # Premise for everything below: `Π` must vary enough across the column that reading the wrong | ||
| # cell is visible. Without it the six assertions would pass under any index. | ||
| @test Π_cell(1, 1, 1) / Π_cell(1, 1, Nz) > 1.5 |
There was a problem hiding this comment.
it may be possible to redesign this stuff to use more built-in Field infra
however it doesn't matter. we can probably do a sweep later to clean this (and many other) up.
| q = grid_moisture_fractions(i, j, k, grid, ef.microphysics, ρ, qᵛ, fields) | ||
| cᵖᵐ = mixture_heat_capacity(q, ef.thermodynamic_constants) | ||
| return 𝒬 / cᵖᵐ | ||
| Π = near_wall_exner_function(i, j, k, ef, q, dynamics_fields) |
There was a problem hiding this comment.
might it be possible to use a more generic exner_function(i, j, k, grid, ef, q, dynamics_field) ?
also note to include the grid argument so this is compatible with KernelFunctionOperation
| end | ||
|
|
||
| # Convert energy flux to potential temperature flux: Jᶿ = 𝒬ᵀ / (cᵖᵐ Π) | ||
| @inline function 𝒬ᵀ_to_Jᶿ(i, j, k, grid, ef, 𝒬ᵀ, fields, dynamics_fields) |
There was a problem hiding this comment.
could be possible to share a utility with the energy forcing conversion for tendencies. but maybe not.
Review points from @glwagner: the helper was called `near_wall_exner_function` but is valid at any `i, j, k`, and a grid-point Exner evaluation should carry `grid` so it is usable from a `KernelFunctionOperation`. Both resolve the same way. It is not a boundary-conditions helper that happens to compute Π — it is Π at a grid point, so it becomes a method of `exner_function` itself with the signature `exner_function(i, j, k, grid, ef, q, dynamics_fields)`. Both call sites already had `grid` in scope. The `near_wall_` prefix was wrong for the reason given. Its neighbours earn it — `near_wall_velocity` reads `fields.u[i, j, 1]`, fixed to the first cell — while this takes an arbitrary index and the six `getbc` methods are what choose a boundary cell. The prefix described the callers, not the function. Extending a name imported through `using ... :` is not allowed, so `exner_function` moves to its own `import` line. Not piracy: `ef` is a type this module owns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ
Closes #976.
The conversion of a prescribed energy flux stopped one step short.
𝒬 / cᵖᵐis a temperature fluxJᵀ, and it was written into theρθbudget as though it were a potential-temperature flux. Themissing step is
Jᶿ = Jᵀ / Π. The function is named𝒬_to_Jᶿ; it returnedJᵀ.The interior
ρEforcing and the radiative flux divergence already take both steps, dividing bycᵖᵐ Π. This makes the flux path agree with them.Effect
Potential temperature tendency unchanged at 1000 hPa, +4.7% at 850 hPa, +10.7% at 700 hPa.
Any coupled
EarthSystemModelrun picks this up through the energy flux field its coupler writes,so surface heating over elevated terrain changes by that much and existing coupled baselines will
not reproduce.
Breaking
Both wrapper structs gain a
standard_pressurefield and a type parameter. They are exported fromBreeze.BoundaryConditions, so positional construction at the old arity now throws. Ordinary usageis unaffected — the public one-argument
EnergyFluxBoundaryCondition(flux)is unchanged, andThetaFluxBoundaryConditionFunction's shorter convenience constructor still works.I deliberately did not add a convenience constructor preserving the old
EnergyFluxarity: unlikedensity, which resolves at runtime through the #777Nothingfallback, there is nothing forstandard_pressureto fall back to, so omitting it should fail loudly rather than silently producea boundary condition carrying
nothing.standard_pressureconverts to the grid's float type at materialization. This is load-bearing, notdefensive: boundary conditions materialize from the stub dynamics, before
materialize_dynamicsnormalizes, so on a
Float32grid the constructor would otherwise see aFloat64value and throw.Version: strictly this is 0.12.0, since an exported struct's shape changed. Worth weighing
against the cost — NumericalEarth pins
Breeze = "0.11", so a 0.12.0 release strands this fixbehind a separate NumericalEarth compat PR, and the fix is what makes coupled surface-energy results
interpretable. 0.11.3 with the behavior change called out in the release notes is the pragmatic
alternative. Maintainers' call.
Tests
The existing conversion test read the model's own boundary condition, and at the default base
pressure (
Π = 1.003) it would have caught this change — the conventions differ by 3.3e-3, about tentimes
isapprox'sFloat32tolerance. It now runs two base pressures anyway, because a singlecolumn cannot distinguish a Π that tracks the column's pressure from a constant that happens to be
right there.
𝒬 / cᵖᵐis identical at both, so the ratio between them tests that the conversionvaries correctly, and depends on no tolerance.
The first commit is unrelated to the fix and lands first so history never goes red.
getbc coverage for all boundary facesassertsΔρθ != 0after one step atΔt = 1e-6, where the increment isabout
9.9e-9againstρθ ≈ 353, whoseFloat32spacing is3.05e-5— three thousand timeslarger. Measured
Δρθis9.85e-9inFloat64and exactly0.0inFloat32. That assertion couldonly ever pass in single precision on rounding noise, and this change perturbs the flux enough to
flip it. Raising
Δtto 1 s makes it measure what it claims. This has been latent becausetest_float_types()returns(Float64,)unlessBREEZE_TEST_FLOAT32=true, which is set nowhere inthe repo.
Verified
forcing_and_boundary_conditions.jlandbalance_adiabatically.jlgreen in both precisions;quality_assurance.jl(Aqua, ExplicitImports) clean. An independent review run coveredboundary_conditions,wall_fluxes,turbulence_closures,atmosphere_model_constructionanddiagnostics— 352/352 — and confirmed zero allocations and concrete return types, including amixed
Float32-grid /Float64-constants case.GPU: not run, but the path is precedented.
compute_potential_temperature_tendency!alreadybuilds a
LiquidIcePotentialTemperatureStateand callsexner_functionon it inside a kernellaunched on
arch(potential_temperature_tendency.jl:93-95), and the bulk-flux boundaryconditions already read
dynamics_fields.pon-device. This adds no construct that is not alreadyexercised there — the added field is an isbits scalar, and
Adapt.adapt_structureis updated forboth structs. What has not been run is this specific call site, since GPU jobs only trigger on
pull_request.Coordination
na_init) #834 reconstructs both structs with five positional arguments inwithout_microphysicsand willbreak against this. That is intended: it needs to carry
standard_pressurethrough.using ..Thermodynamics:import block this touches.Docs:
total_energy_density_name's docstring and the boundary-conditions page both stated theflux/forcing asymmetry as intended behavior; both now state the single conversion.
Jᶿjoins thenotation table, since
Jᵀwas there butJᶿwas not and they differ by exactly this factor.The last commit renames
𝒬_to_Jᶿ→𝒬ᵀ_to_JᶿandJᶿ_to_𝒬→Jᶿ_to_𝒬ᵀ. A bare𝒬does not saywhich energy flux, and a latent heat flux would be wrong here — that energy travels with the
moisture flux. The valid input is an enthalpy flux, which is
𝒬ᵀas the table defines it, and thetable gives every
𝒬a superscript. That is also why no bare-𝒬row was added: with the rename,none remains in the source. Both functions are internal to the one file and not exported, so the
rename is revertible in a pass if you prefer the bare spelling.
🤖 Generated with Claude Code
https://claude.ai/code/session_01D2fuRfqFoMFjvYyx3idQtQ