-
Notifications
You must be signed in to change notification settings - Fork 99
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 elixir for shallow water equations with P4estMesh
#1424
Conversation
Thanks for adding this! Just be cautious here that one should not use AMR with the shallow water equations at the moment because the numerical scheme will not be well-balanced due to how the mortars are currently computed. Special mortar projections are required to keep the overall solver well-balanced. This was one reason I had yet to make any kind of shallow water elixir for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! It looks like the new tests are failing. Could you also please consider using a less refined mesh to save CI time?
Ok, so maybe we wait with this to be merged until AMR will keep the scheme well-balanced for a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think it's fine to merge this as it is since we could get the same problem with AMR when using the TreeMesh
- where the shallow water equations should work. Thus, I think we can merge this PR when tests pass - unless @andrewwinters5000 has another opinion.
Codecov Report
@@ Coverage Diff @@
## main #1424 +/- ##
=======================================
Coverage 95.74% 95.74%
=======================================
Files 356 356
Lines 29598 29598
=======================================
Hits 28338 28338
Misses 1260 1260
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This will be good to have for MPI testing.
Currently, there is no elixir for the shallow water equations on a
P4estMesh
. So, I thought I add a basic one. As also stated here this currently does not work with MPI. Once, this is resolved we could add this elixir also to the test_mpi_p4est_2d.jl.