static_column_depth interface
#3841
Conversation
simone-silvestri
changed the title
ImmersedBoundariesbottom_height interface
|
Correction: |
|
Is There's a naming issue. We had previously referred to "z" as "height". Usually this doesn't matter but when it does, eg in realistic contexts and using nonlinear equation of state, then we are trying to be consistent with the literature where "z" is the "geopotential height". We can change the user-facing naming and notation for sure but it has to be done carefully and with consensus. I think this PR makes some big changes to notation and language that could have wide-ranging impacts on how we communicate about numerical models. |
|
|
||
| struct GridFittedBottom{H, I} <: AbstractGridFittedBottom{H} | ||
| bottom_height :: H | ||
| z_bottom :: H |
There was a problem hiding this comment.
I though we want to refer to z as the coordinate and height as a positive value.
At least, this is the direction we are taking in ClimaOcean where we are distinguishing very carefully between z and h or height.
In this case, this is technically a coordinate, not a height, for example we have now
There was a problem hiding this comment.
I though we want to refer to z as the coordinate and height as a positive value.
Usually "z" is called "geopotential height". In the way it's usually used, height can be positive or negative; the point is that it increases as we go upwards away from the center of the Earth (as opposed to "depth" which increases downwards). What would you call "z" instead? Altitude?
There was a problem hiding this comment.
this is technically a coordinate, not a height
Another name for z is "height coordinate".
There was a problem hiding this comment.
As someone who hasn't used immersed boundaries that much, I always find it a bit confusing that it's called bottom height because it makes me think I'm meant to put in a positive "height" from the bottom of the domain, but I would know that "z" is meant to be from the top
Co-authored-by: Gregory L. Wagner <[email protected]>
validation/multi_region/multi_region_near_global_quarter_degree.jl
Outdated
Show resolved
Hide resolved
| @@ -55,6 +55,7 @@ using Oceananigans.ImmersedBoundaries: z_bottom | |||
| import Oceananigans.Grids: required_halo_size_x, required_halo_size_y, required_halo_size_z | |||
| import Oceananigans.Architectures: on_architecture | |||
|
|
|||
| import Oceananigans.ImmersedBoundaries: z_bottom | |||
There was a problem hiding this comment.
it's the new rearrangement of the modules.
There was a problem hiding this comment.
I have changed z_bottom to be only in TurbulenceClosures as it is the only module in which is needed / used and all similar methods (z_top, depth, height_above_bottom...) are defined only there
| full_Δz = Δzᶜᶜᶜ(i, j, k, ibg.underlying_grid) | ||
| partial_Δz = z - zb |
There was a problem hiding this comment.
This should go in a different PR.
There was a problem hiding this comment.
The new numerical bottom height is located at by convention at z⁻ + Δz * (1 - ϵ) so ϵ * full_Δz == z - h removing the necessity of the max. I thought this change can clean up the partial cell code
There was a problem hiding this comment.
actually, there is a bug here with how the immersed bottom height is computed. I have corrected it
|
The dependency on OrthogonalSphericalShellGrids here is blocking the progress (the |
Shall we do this? In case we want to do it I have prepared CliMA/OrthogonalSphericalShellGrids.jl#47. |
|
This PR should be ready to merge |
This PR introduces an interface for
column_heightthat returns the height of the water column.This is simply
grid.Lzin regular grids.For
AbstractGridFittedBottomimmersed grids, the column height is readily read from the immersed boundary, provided that the immersed boundary represents the numerical bottom rather than the physical bottom.Therefore, this PR changes the constructor of the
ImmersedBoundaryGridto store the z-coordinate of the last immersed cell. In this way, the bottom height is also uniquely defined forGridFittedBottom, removing the necessity of having aCenterImmersedConditionand aInterfaceImmersedCondition.An immediate application of this interface is in the
SplitExplicitFreeSurfacewhere the bottom_height was previously stored.