Davis wave speed estimate for shallow water equations

[JoshuaLampert]

There are two different things I noticed related to the Davis wave speed estimate for the shallow water equations that were introduced in #1561. First, I think there is a typo for the one-dimensional SWE. The second one is a question. The PR #1561 introduced min_max_speed_davis and #1501 introduced min_max_speed_chen_noelle for the SWE, which are basically the same functions (in 1d and in 2d) except that min_max_speed_chen_noelle also has zero(eltype(u_ll)) in the min and max. Is this known and is it intended to have (almost) the same function twice or does it suffice to keep just one? If both are necessary maybe a comment would be helpful for people looking at the source and wonder about these two functions.

cc @DanielDoehring

[ranocha]

Thanks for noticing this!

The PR #1561 introduced min_max_speed_davis and #1501 introduced min_max_speed_chen_noelle for the SWE, which are basically the same functions (in 1d and in 2d) except that min_max_speed_chen_noelle also has zero(eltype(u_ll)) in the min and max. Is this known and is it intended to have (almost) the same function twice or does it suffice to keep just one? If both are necessary maybe a comment would be helpful for people looking at the source and wonder about these two functions.

@DanielDoehring @andrewwinters5000?

[ranocha]

I'll close and reopen this since CI fails for some weird reasons

[andrewwinters5000]

The additional zero(eltype(u)) in the min_max_speed_chen_noelle is necessary to ensure that their HLL flux is a positive Riemann solver and avoids any mass flux from a point that is deemed "dry".

In the separation of Trixi and TrixiShallowWater the routine min_max_speed_chen_noelle is planned to move, so it makes sense to keep both it and min_max_speed_davis. As with the compressible Euler equations, there is a whole cottage industry of wave speed estimates for the shallow water eqautions. A fairly good study is given by J. P. Vila.

[andrewwinters5000]

Good catch on the typo! Once the tests run we will see if any reference values need updated. But for now I will approve this.

[codecov]

Codecov Report

Merging #1601 (f65bd5d) into main (ce81702) will increase coverage by 3.23%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1601      +/-   ##
==========================================
+ Coverage   92.90%   96.13%   +3.23%     
==========================================
  Files         402      402              
  Lines       33034    33091      +57     
==========================================
+ Hits        30690    31812    +1122     
+ Misses       2344     1279    -1065     

Flag	Coverage Δ
unittests	96.13% <100.00%> (+3.23%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/equations/shallow_water_1d.jl	100.00% <100.00%> (ø)

... and 40 files with indirect coverage changes

[DanielDoehring]

There are two different things I noticed related to the Davis wave speed estimate for the shallow water equations that were introduced in #1561. First, I think there is a typo for the one-dimensional SWE.

Absolutely correct, sorry for that - I thought I double checked everything.

The second one is a question. The PR #1561 introduced min_max_speed_davis and #1501 introduced min_max_speed_chen_noelle for the SWE, which are basically the same functions (in 1d and in 2d) except that min_max_speed_chen_noelle also has zero(eltype(u_ll)) in the min and max. Is this known and is it intended to have (almost) the same function twice or does it suffice to keep just one? If both are necessary maybe a comment would be helpful for people looking at the source and wonder about these two functions.

I just had a very brief look at the paper - from that, it appears to me that their numerical flux is equivalent to HLL-Davis.

The standard HLL flux with checking directionality of the speeds is given by

$$ \boldsymbol F = \begin{cases} \boldsymbol f(\boldsymbol u_-) & s_- \geq 0 \\ \boldsymbol f^\star(\boldsymbol u_-, \boldsymbol u_+) & s_- < 0 < s_+ \\ \boldsymbol f(\boldsymbol u_+) & s_+ \leq 0 \end{cases}$$

where the intermediate flux $\boldsymbol f^\star(\boldsymbol u_-, \boldsymbol u_+)$ is given by

$$ \boldsymbol f^\star(\boldsymbol u_-, \boldsymbol u_+) = \frac{s_+ \boldsymbol f(\boldsymbol u_-) - s_- \boldsymbol f(\boldsymbol u_+) + s_+s_i \left(\boldsymbol u_+ - \boldsymbol u_-\right)}{s_+ - s_i} $$

By enhancing the usual definition of the wavespeeds as done by Chen & Noelle, you can basically spare yourself from the case computation and always use $\boldsymbol f^\star$.