High-order advection stencils, overhaul of boundary treatment, and bugfix in advection reconstruction #2603
Conversation
|
I just realized that this might have been the problem which generated instabilities in #2510 |
|
The problem was that we were generating spurious extrema when using high order advection near immersed boundaries. These spurious extrema were by no means negligible. I think that in the vertical direction this problem might have a larger impact on the stability due to the smaller grid scales and lower diffusion, while in the horizontal direction, diffusion could hide the extrema more effectively |
….jl into ss/advection_refactor
….jl into ss/advection_refactor
| const WENOVectorInvariantVel = Union{WENOVectorInvariantVel3, WENOVectorInvariantVel5} | ||
| const WENOVectorInvariant{FT, XT, YT, ZT, XS, YS, ZS, VI} = | ||
| Union{WENO3{FT, XT, YT, ZT, XS, YS, ZS, VI}, | ||
| WENO5{FT, XT, YT, ZT, XS, YS, ZS, VI}} where {FT, XT, YT, ZT, XS, YS, ZS, VI<:SmoothnessStencil} |
There was a problem hiding this comment.
Why is WENOVectorInvariant identified as a WENO with a SmoothnessStencil?
The design proposed by #2454 proposed to disentangle the reconstruction scheme from the "formulation":
struct VectorInvariantMomentumAdvection
vorticity_stencil
vorticity_reconstruction
endTherefore "WENOVectorInvariant" is VectorInvariantMomentumAdvection with vorticity_reconstruction::WENO5.
There was a problem hiding this comment.
I didn t get to that part yet
|
It doesn't look like things are moving towards #2454 exactly. Is there a plan to eventually do this, or is another design being proposed? |
There was a problem hiding this comment.
Great work @simone-silvestri!
Tests seem to be passing so ready to merge?
|
I d like to try to improve performance a bit first... Anyways, by next week I ll merge |
|
@tomchor I guess we are hoping we can merge without the 3/4 performance penalty. |
|
comparison of run wall time for the bickley jet This is running the code till 200 seconds
Looking at these timings, everything seems in the ballpark (except for the immersed boundary) so I am not sure what might be the issue... I guess we would need an in depth profiling to understand this? I say we merge. The benefit from time step increase covers for the performance decrease. And finding the root of such a decrease might be very difficult |
|
What's are the possible causes of the performance difference? What code should we focus on if we want to close this performance gap in the future? |
|
I believe it might be warp divergence. The way we have boundaries (and immersed boundaries) implemented, we create instances of branch divergence because the scheme is different near the boundaries. In this PR, for 5th order schemes there are two instances of divergence instead of one (the scheme changes twice instead of once). I am not super sure about it but it might be an explanation. A proper profile using a visual profiler will probably show it |
|
Okay, so before this PR, we generate two stencils for each flux calculation --- second order and fifth order. The fifth order result is then used in the interior and the second order result near boundaries. In this new scheme, we generate three stencils rather than two stencils, because we limit to third order, and then to second order? |
Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>
|
For a future PR (probably #2642):
|
In this PR:
Centered,UpwindBiasedandWENOorderkeyword argument which goes up to 12 forCenteredand 11 for upwind schemes_symmetric_,_left_biased_and_right_biasedhave their own boundary treatment which entails checking the points for each method differentlyboundary_schemefield which will is used to reconstruct values near the boundary. Boundary schemes use the same method as the parent scheme but with a lower order:For example, when using a WENO 9th order, closing in near the boundaries the order will be progressively decreased to 7th, 5th, 3rd and finally 1st.
It also solves various bugs associated with advection and immersed boundary.