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

Add linear dispersion relation #168

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

JoshuaLampert
Copy link
Owner

@JoshuaLampert JoshuaLampert commented Dec 30, 2024

This adds a new feature to compute the dispersion relation and wave speed of the different equations. It also adds a new page in the docs explaining the general concept of dispersion, which uses the new LinearDispersionRelation to compare the dispersive behavior of the different equations.
I have three questions:

  • Should we provide built-in support for the full dispersion relation? If yes, I'm not sure about the best option. We could add a dummy equation EulerEquation1D as I do in the docs and dispatch on that. This, however, has the disadvantage that it might lead to the confusion that the Euler equations are supported by DispersiveShallowWater.jl.
  • I'm not sure about the hyperbolic Serre-Green-Naghdi equations. There is a dispersion relation given in Favrie and Gavrilyuk, A rapid numerical method for solving Serre-Green-Naghdi equations describing long free surface gravity waves in eq. (19), but I don't know to understand this. Since $\tau = 1/h$, I initially thought, we obtain a classical dispersion relation by $\tau_0 = 1/h_0$, but this doesn't work out dimension-wise. That this doesn't work can also be seen by eq. (18), which differs from the one in the other two papers mentioned in the code. There is an additional third power in the denominator. I think this is related to the fact that the equations are written in Lagrangian form and not in the classical variables, but I'm not sure. Do you know how this dispersion relation would needs to be translated to obtain the classical version, @ranocha?
  • I wanted to double-check that the linear dispersion relation for the BBM equation written as $\eta_t + \sqrt{gD}\eta_x + \frac{3}{2}\sqrt{\frac{g}{D}}\eta\eta_x - \frac{1}{6}D^2\eta_{xxt} = 0$ gives indeed the same as the one for the BBM-BBM equations, but I get another factor of $5/2$ in front (linearizing with $\eta = h_0 + h'$ and $D = h_0$ gives $h'_t + 5/2 \sqrt{g h_0} h'_x - 1/6 h_0^2h_{xxt} = 0$). I haven't thought about that when implementing the BBM equation. I would expect the same dispersion relation as for the BBM-BBM equations. Do you have an idea how to deal with that, @ranocha?

Closes #131.

@JoshuaLampert JoshuaLampert added enhancement New feature or request documentation Improvements or additions to documentation labels Dec 30, 2024
Copy link

codecov bot commented Dec 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.06%. Comparing base (3a32a5c) to head (040768b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   98.02%   98.06%   +0.04%     
==========================================
  Files          19       20       +1     
  Lines        1877     1916      +39     
==========================================
+ Hits         1840     1879      +39     
  Misses         37       37              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Dec 30, 2024

Benchmark Results

main 040768b... main/040768b362ad74...
bbm_1d/bbm_1d_basic.jl 14.1 ± 0.48 μs 13.9 ± 0.39 μs 1.01
bbm_1d/bbm_1d_fourier.jl 0.526 ± 0.31 ms 0.537 ± 0.0027 ms 0.979
bbm_bbm_1d/bbm_bbm_1d_basic_reflecting.jl 0.113 ± 0.0018 ms 0.12 ± 0.0033 ms 0.947
bbm_bbm_1d/bbm_bbm_1d_dg.jl 0.0338 ± 0.00047 ms 0.0343 ± 0.0005 ms 0.986
bbm_bbm_1d/bbm_bbm_1d_relaxation.jl 27.8 ± 0.53 μs 27.6 ± 0.58 μs 1.01
bbm_bbm_1d/bbm_bbm_1d_upwind_relaxation.jl 0.0488 ± 0.00056 ms 0.0489 ± 0.00038 ms 0.998
hyperbolic_serre_green_naghdi_1d/hyperbolic_serre_green_naghdi_dingemans.jl 4.15 ± 0.016 μs 4.18 ± 0.013 μs 0.995
serre_green_naghdi_1d/serre_green_naghdi_well_balanced.jl 0.2 ± 0.0048 ms 0.2 ± 0.0046 ms 1
svaerd_kalisch_1d/svaerd_kalisch_1d_dingemans_relaxation.jl 0.149 ± 0.0043 ms 0.148 ± 0.0033 ms 1.01
time_to_load 2.01 ± 0.0048 s 2 ± 0.0014 s 1

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Benchmark Plot

test/test_unit.jl Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include linear dispersion relation for all equations
1 participant