Begins the implementation of Oceananigans.HydrostaticFreeSurfaceModel for general circulation modeling #1349

glwagner · 2021-02-09T22:19:47Z

This PR adds a new AbstractModel called HydrostaticFreeSurfaceModel to IncompressibleModel and ShallowWaterModel in the Oceananigans model suite.

HydrostaticFreeSurfaceModel is intended to solve the hydrostatic Boussinesq equations beneath a free surface. This PR adds the basic infrastructure needed to time step models on a RegularCartesianGrid with explicit free surface dynamics.

I think we should save dynamics tests and an example for a future PR, and just work on getting some of the basic ingredients into this model. The time-stepping tests are currently in place, but most of time_step!(model::HydrostaticFreeSurface, dt) needs to be fleshed out.

Todo for this PR:

correct tendency calculation and time-stepping for an Adams-Bashforth timestepping method
calculation of vertical velocity from the continuity equation

src/Models/HydrostaticFreeSurfaceModels/HydrostaticFreeSurfaceModels.jl

src/Models/HydrostaticFreeSurfaceModels/compute_w_from_continuity.jl

ali-ramadhan · 2021-02-10T12:41:59Z

src/Models/HydrostaticFreeSurfaceModels/hydrostatic_free_surface_model.jl

+function HydrostaticFreeSurfaceModel(; grid,
+                architecture::AbstractArchitecture = CPU(),
+                                             clock = Clock{eltype(grid)}(0, 0, 1),
+                                         advection = (momentum=VectorInvariant(), tracers=CenteredSecondOrder()),


💪

Out of curiousity I thought VectorInvariant changes the form of the equations being solved, but can we think of it as a different advection scheme?

I'm generalizing the kwarg advection to mean something like "advection term" (both continuous equation aspects and numerical aspects).

We could try to distinguish between continuous aspects (whether we use a vector invariance form, an "interior" form, or a conservation form) and numerical aspects (numerical approximations / stencils used to implement that form).

We could also have a more hierarchical interface; something like

advection = (momentum=VectorInvariantForm(), tracers=ConservationForm(scheme=CenteredSecondOrder()))

I'm open to suggestion; I just did something lazy and simple here.

Oh yeah no I think what you did is great. Just wanted to understand. Makes sense that it encapsulates both the continuous and numerical aspects.

src/Models/HydrostaticFreeSurfaceModels/show_hydrostatic_free_surface_model.jl

ali-ramadhan · 2021-02-10T12:46:43Z

test/test_hydrostatic_free_surface_models.jl

+function hydrostatic_free_surface_model_tracers_and_forcings_work(arch)
+    grid = RegularCartesianGrid(size=(1, 1, 1), extent=(2π, 2π, 2π))
+    model = HydrostaticFreeSurfaceModel(grid=grid, architecture=arch, tracers=(:T, :S, :c, :d))
+    set!(model, η=1)


Ah I guess we'll have to do this for a lot of tests, similar to ShallowWaterModel, to avoid blowup when timestepping due to division by zero?

You mean setting the displacement to some constant value?

Since we're modeling displacement rather than the total thickness of the layer, I think we should get away with an initial value of 0. I can remove the call to set! since it's tested in another routine.

Ah right it's testing set!.

test/test_hydrostatic_free_surface_models.jl

…surface_model.jl Co-authored-by: Ali Ramadhan <[email protected]>

francispoulin

I have not tried to run it but things look good to me.

One thing that I noticed is that advection seemed to be commented out for tracers. Did I read that right?

Also, time stepping is done differently, and not exactly sure why that is but any time stepping that works is good time stepping!

glwagner · 2021-02-10T15:01:52Z

One thing that I noticed is that advection seemed to be commented out for tracers. Did I read that right?

Yes --- because we are still finalizing the user interface that controls the advection term + advection scheme for momentum and tracers.

I think ideally we want to preserve the ability to use high-order advection schemes for momentum when we are on a RegularCartesianGrid. However, we also have to use a "vector-invariant form" of the advection term when running on curvilinear grids. So we need a way to specify both continuous and numerical aspects of the advection term implementation.

glwagner · 2021-02-10T15:07:10Z

@ali-ramadhan @francispoulin I propose that for the time being we use an abstraction where advection has a field momentum and a field for every tracer.

The value of this field controls the implementation of the advection term, with continuous and numerical aspects "flattened" into a single spec. If one writes VectorInvariant, then we implement a second-order VectorInvariant form of the momentum equations. If one provides an advection scheme, when we implement a conservation form of the momentum / tracer equations with the associated stencil.

When we have curvilinear grids we will need a function validate_advection(advection, grid) to ensure that a conservation form scheme isn't used on a curvilinear grid (for example). There's a similar consideration for stretched grids and our current WENO implementation.

glwagner · 2021-02-10T15:15:07Z

I also wonder if we really need to have two kwargs (momentum_advection and tracer_advection). It's a little messy and complicated to parse user-input if we try to smash everything into one kwarg.

ali-ramadhan · 2021-02-10T15:24:40Z

I think a single advection kwarg would make for a cleaner UI, since there are already lots of model kwargs.

I agree it's a bit messier to parse and validate but the parsing and validation can be done by small functions in a separate file.

francispoulin · 2021-02-10T15:32:13Z

Hmm, as a user I think there are reasons why you might wnat to have the choice to use different advection schemes for momentum and tracers (and buoyancy). But could there be a default where if there is only momentum advection specified, then tracers are the same?

…cer advection

francispoulin

Changes look reasonable to me.

It seems that nonlinear advection was added into the tendencies, but were the Coriolis terms also removed?

glwagner · 2021-02-10T21:11:44Z

It seems that nonlinear advection was added into the tendencies, but were the Coriolis terms also removed?

No they are there.

glwagner · 2021-02-10T21:12:27Z

src/Models/HydrostaticFreeSurfaceModels/hydrostatic_free_surface_tendency_kernel_functions.jl

+                                                              clock)
+
+    return ( - U_dot_∇v(i, j, k, grid, advection, velocities)
+             - y_f_cross_U(i, j, k, grid, coriolis, velocities)


@francispoulin here's a Coriolis term

Sorry, I clearly misread something.

glwagner added 2 commits February 9, 2021 17:15

Adds HydrostaticFreeSurfaceModel

75f629d

Debugs HydrostaticFreeSurfaceModel time-stepping

f5bcc9f

glwagner requested a review from ali-ramadhan February 10, 2021 12:34

Use div_xyᶜᶜᵃ instead of hdiv

4a78232

ali-ramadhan approved these changes Feb 10, 2021

View reviewed changes

Update src/Models/HydrostaticFreeSurfaceModels/show_hydrostatic_free_…

f4c5102

…surface_model.jl Co-authored-by: Ali Ramadhan <[email protected]>

francispoulin reviewed Feb 10, 2021

View reviewed changes

glwagner added 2 commits February 10, 2021 10:09

Do actually run GPU tests for Bounded, Bounded, Bounded

e2dc55e

No need to set! a constant displacement for testing

b85b0d6

Cleans up UI for advection schemes and re-implements momentum and tra…

fe2671f

…cer advection

francispoulin reviewed Feb 10, 2021

View reviewed changes

glwagner commented Feb 10, 2021

View reviewed changes

glwagner merged commit 0057819 into master Feb 10, 2021

glwagner deleted the glw/hydrostatic-free-surface-model branch February 10, 2021 21:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begins the implementation of Oceananigans.HydrostaticFreeSurfaceModel for general circulation modeling #1349

Begins the implementation of Oceananigans.HydrostaticFreeSurfaceModel for general circulation modeling #1349

glwagner commented Feb 9, 2021 •

edited

Loading

ali-ramadhan Feb 10, 2021

glwagner Feb 10, 2021

ali-ramadhan Feb 10, 2021

ali-ramadhan Feb 10, 2021

glwagner Feb 10, 2021

ali-ramadhan Feb 10, 2021

francispoulin left a comment

glwagner commented Feb 10, 2021

glwagner commented Feb 10, 2021

glwagner commented Feb 10, 2021

ali-ramadhan commented Feb 10, 2021

francispoulin commented Feb 10, 2021

francispoulin left a comment

glwagner commented Feb 10, 2021

glwagner Feb 10, 2021

francispoulin Feb 11, 2021

Begins the implementation of Oceananigans.HydrostaticFreeSurfaceModel for general circulation modeling #1349

Begins the implementation of Oceananigans.HydrostaticFreeSurfaceModel for general circulation modeling #1349

Conversation

glwagner commented Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

francispoulin left a comment

Choose a reason for hiding this comment

glwagner commented Feb 10, 2021

glwagner commented Feb 10, 2021

glwagner commented Feb 10, 2021

ali-ramadhan commented Feb 10, 2021

francispoulin commented Feb 10, 2021

francispoulin left a comment

Choose a reason for hiding this comment

glwagner commented Feb 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glwagner commented Feb 9, 2021 •

edited

Loading