Introduce wrappers and separate transport from time discretisations #382
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tommbendall
changed the title
… vector invariant types
jshipton
reviewed
Jul 24, 2023
gusto/common_forms.py
Outdated
| (u.∇)q = (∇×q)×u + (1/2)∇(u.q) | ||
|
|
||
| The form returned by this function corresponds to the (∇×q)×u circulation | ||
| term. An an upwind discretisation is used when integrating by parts. |
There was a problem hiding this comment.
Need to remove part about upwind discretisation.
gusto/common_forms.py
Outdated
| ubar (:class:`ufl.Expr`): the transporting velocity. | ||
|
|
||
| Raises: | ||
| NotImplementedError: the specified integration by parts is not 'once'. |
gusto/spatial_methods.py
Outdated
| Args: | ||
| equation (:class:`PrognosticEquation`): the equation, which includes | ||
| the original type of this term. | ||
| variable (str): name of the variable to set the transport scheme for |
gusto/spatial_methods.py
Outdated
| self.field = equation.X | ||
| self.test = equation.test | ||
|
|
||
| # Find the original transport term to be used, which we use to extract |
gusto/timeloop.py
Outdated
| residual = equation.residual.label_map( | ||
| lambda t: t.has_label(term_label), map_if_false=drop | ||
| ) | ||
| active_variables = [t.get(prognostic) for t in residual.terms] |
There was a problem hiding this comment.
is "active" the right word here?
jshipton
approved these changes
Jul 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
Wrapperobject for time discretisationsSpatialMethodobject to describe specific spatial discretisations, similar to physics schemes.This allows a bunch of specific code to be pulled out from the time discretisations, and gives us new ways of combining and specifying our different options.
Breakdown of changes
Wrapperobject is introduced inwrappers.py, which has pre- and post- apply methods. There are three applications of this:EmbeddedDGWrapperRecoveryWrapperSUPGWrapper(which does nothing in pre- and post- apply methods, but does introduce a different test function to be used)I have tried to keep all of this code the same, but we should instantly gain the functionality to apply these wrappers to physics and diffusion terms.
TimeDiscretisationobjects can now set up aWrapperobject when they are passed wrapper options. All of the specific wrapper code is now completely pulled out oftime_discretisation.py.SpatialMethodobject is defined inspatial_methods.py, which is the base object for specifying a spatial discretisation of some term. Why bother to introduce this? It allows us to separate the generic term in an equation [e.g.test*dot(u,grad(q))*dx], from its discretisation [e.g. an upwind discretisation]. When thesteppersets up all of the methods/schemes, it will replace the term in the equation with the discretisation in theSpatialMethod.SpatialMethodhave been introduced:TransportMethodintransport_methods.pyandDiffusionMethodindiffusion_methods.py. These are very simple and pretty much just call the methods ofSpatialMethodbut with the specifictransport/diffusionlabels. However theTransportMethodrequires more care as it needs to ensure that thetransporting_velocityis replaced correctly.diffusion.pyhas been removed, and code for theInteriorPenaltyDiffusionmethod has been moved todiffusion_methods.pyTransportMethodhave been implemented:DGUpwind(which is basically our work-horse at the moment) andDefaultTransport(which calls the basic form originally set up in the equation -- I'm open to better names for this!). The forms used inDGUpwindare stored intransport_methods.pyDGUpwind, so this has been implemented as aWrapper(to ensure that the test function in the time discretisation gets replaced). The appropriateIntegrateByPartsoption should be passed to theDGUpwindobject.transport_forms.pyhas been renamed ascommon_forms.py, and the intention is that this should contain forms that are used byPrognosticEquations and notSpatialMethods. The basic transport and diffusion forms have been added to this.transporting_velocitylabel. I have however fixed thelinear_advection_form(issue Linear transport forms need fixing #332).circulationform of the vector transport equation. I was a bit confused by what was happening before, and hopefully this clears things up:"vector_invariant": (u.∇)u = (∇×u)×u + (1/2)∇(u.u), implemented as a single transport term, with separate transported and transporting velocities"circulation": (u.∇)u = (∇×u)×u + (1/2)∇(u.u), implemented as two terms, a transport term (∇×u)×u and a K.E. term (1/2)∇(u.u) [which would now be evaluated as a forcing rather than as a transport term]transport_methods = [DGUpwind(eqn, variable_name), ...]to all of these, and ensuring that it is passed to the time stepper.