Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modular and unified user interface for advection schemes #2454

Closed
glwagner opened this issue Apr 21, 2022 · 6 comments
Closed

Modular and unified user interface for advection schemes #2454

glwagner opened this issue Apr 21, 2022 · 6 comments
Labels
cleanup 🧹 Paying off technical debt feature 🌟 Something new and shiny

Comments

@glwagner
Copy link
Member

glwagner commented Apr 21, 2022

We need to overhaul the way we specify advection schemes, whose current limitations were exposed when "WENO vector invariant" was implemented. I propose that we implement a hierarchical specification system using a new object called AdvectionScheme that specifies

  1. The "form" of the advection operator for momentum and tracers (either VectorInvariant or FluxForm for momentum, always FluxForm for tracers)
  2. For FluxForm, the reconstruction scheme for the advected component
  3. For VectorInvariant, the stencil for computing vorticity, as well as the method for reconstructing vorticity at the momentum locations
  4. For VectorInvariant with WENO reconstruction, the type of the smoothness stencil (vorticity or velocity)
  5. The method for reconstruction velocities in mass conservation (continuity) as well as tracer advection

We also may want to specify how we reconstruction advecting velocities for flux form momentum advection (right now we use centered advection with an accuracy one order lower than the accuracy of the advected component reconstruction --- ie with UpwindBiasedFifthOrder() we reconstruct advecting components with a centered fourth order scheme).

Finally, we need to allow different schemes for every tracer.

This modular design will allow more detailed specification of an advection scheme and also us to remove the option for WENO reconstruction for vector invariant more easily, which @simone-silvestri pointed out is experimental.

We also would like to explore alternative discretization for the continuity equation. For this we need a "mass" reconstruction scheme, and the advecting scheme for tracers needs to reconstruct advected_velocity in a way that's consistent with the mass reconstruction.

Here's a sketch of what these new types might look like:

""" Specifies a reconstruction scheme for fluxes of the form u * c,
where u is the `advecting_velocity` and c is the `advected_quantity` """
struct FluxFormAdvection
    advected_field_reconstruction
    advecting_velocity_reconstruction
end

""" Specifies a reconstruction scheme for the vector invariant formulation of
momentum advection."""
struct VectorInvariantMomentumAdvection
     vorticity_stencil
     vorticity_reconstruction
     # velocity_reconstruction?
end

struct UpwindWENOVorticityReconstruction
    weno
    smoothness
end

struct AdvectionScheme
    mass # TBD... only support CenteredSecondOrder now
    momentum # FluxForm or VectorInvariant
    tracers # NamedTuple of FluxForm
end
@glwagner glwagner added feature 🌟 Something new and shiny cleanup 🧹 Paying off technical debt labels Apr 21, 2022
@glwagner glwagner changed the title Advection scheme refactor Modular and unified user interface for advection schemes Apr 21, 2022
@francispoulin
Copy link
Collaborator

This sounds great. What do you think of having advectied_field instead of advected_quantity?

@glwagner
Copy link
Member Author

This sounds great. What do you think of having advectied_field instead of advected_quantity?

That makes sense!

@glwagner
Copy link
Member Author

@simone-silvestri if we want to unify the user interface across all models, we could introduce the type AdvectionScheme (which will be rather trivial now, but we can update it in the future). Then we can have syntax like

advection = AdvectionScheme(momentum=WENO5(grid), tracers=UpwindBiasedThirdOrder())
model = NonhydrostaticModel(; grid, advection)

Or,

model = NonhydrostaticModel(; grid, advection=WENO5())

which the model constructor interprets as

advection = AdvectionScheme(momentum=WENO5(grid), tracers=WENO5(grid))

(this is nice too, because we can build WENO5 on the grid under the hood)

We could also always "regularize" the advection scheme with grid, so that

advection = AdvectionScheme(momentum=WENO5(), tracers=UpwindBiasedThirdOrder())
model = NonhydrostaticModel(; grid, advection)

translates to

advection = AdvectionScheme(momentum=WENO5(grid), tracers=UpwindBiasedThirdOrder())

under the hood.

@glwagner
Copy link
Member Author

I'm also open to other names than AdvectionScheme... that's just what's written above and seems reasonably interpretable.

@francispoulin
Copy link
Collaborator

The name seems very clear and I like how this is structured.

@glwagner
Copy link
Member Author

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt feature 🌟 Something new and shiny
Projects
None yet
Development

No branches or pull requests

2 participants