[WIP] Implement conservative WENO-5 #340
Draft
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.
The current implementation of the WENO-5 advection algorithm is using the non-conservative form of the advection equation (V∂T∂x). We can implement the conservative form (∂(VT)∂x) using directly the velocities on the staggered grid (see for instance here).
Three good reasons to do that:
The implementation is a simple addition: using directly the velocities of the staggered grid for the value of the fluxes after using WENO-5 to approximate the advected quantity on the sides .
I have added a boolean kwarg to
WENO_advection!()
to be able to use both the non-conservative and the conservative form, but I believe that the conservative form should be the default. That would be a breaking change, because the input velocities required would be by default the velocities on the sides as opposed to currently velocities on vertices.I have currently a problem with the boundary conditions, but I think I am mostly there. @albert-de-montserrat, is there a helper function to get only the interior points of for instance
stokes.V.Vx
? Because of the ghost points, I have to add +1 to my indexes currently and it would be nice if it could work by default with velocity fields without ghost points.Needs #339 to be merged before.