CliMA / Oceananigans.jl

🌊 Julia software for fast, friendly, flexible, ocean-flavored fluid dynamics on CPUs and GPUs
https://clima.github.io/OceananigansDocumentation/stable
MIT License
965 stars 190 forks source link

User interface for specifying advection schemes #695

Closed ali-ramadhan closed 4 years ago

ali-ramadhan commented 4 years ago

I'm thinking of allowing users to specify advection schemes via a model constructor kwarg advection.

Some examples:

  1. Centered second-order advection for all fields
    model = IncompressibleModel(..., advection=Centered2)
  2. Fifth-order WENO advection for momentum and DST3 for tracers
    model = IncompressibleModel(..., advection=(momentum=WENO5, tracers=DST3))
  3. Fifth-order WENO for momentum and T, S but DST3 for other tracers
    model = IncompressibleModel(..., advection=(momentum=WENO5, T=WENO5, S=WENO5, CO2=DST3, N2=DST3))

So this is somewhat similar to the way we allow users to specify advection schemes, and constructors handle everything on the backend to properly assign an advection scheme to every field.

In #77 I proposed that we make advection_scheme a field property.

glwagner commented 4 years ago

Adding an advection subtype to IncompressibleModel makes sense to me. This is what the current design of IncompressibleModel looks like to me:

Subtypes corresponding to the model "state"

Subtypes that correspond to the model "physics"

Subtypes that correspond to model numerics

Some concepts are mixed; for example model.grid mixes physical information about the domain (domain extent, topology) and numerical information (grid spacing). model.timestepper carries around both tendencies (aspects of the model state) and numerical parameters (either explicitly or implicitly) associated with the time-stepping scheme.

advection would join the grid, timestepper, architecture, and pressure solver as a subtype that specifies the numerical properties of the model.

I'm not sure it makes sense to include advection_scheme among the properties of a Field. I think Fields are independent of PDEs, and not all fields are advected. I think that at least in concept one should be able to apply many differencing schemes to the same field without having to re-instantiate the field. This suggesting that the differencing scheme is independent from a field.

ali-ramadhan commented 4 years ago

I think you meant to comment on #77 (which I saw you already did) as this issue is more about how users will specify advection schemes via model constructors.

But I do agree now that adding an advection subtype to models makes sense.

glwagner commented 4 years ago

I guess this is at least partially resolved by #815 ?

ali-ramadhan commented 4 years ago

Yes I think we agreed on passing in a named tuple to the model constructor which would make the API for selecting advection consistent with boundary conditions and (soon) forcing functions.

Closing this issue but will keep #77 open as making advection schemes a property of the field might be a nice feature.