Add ShallowWaterEquationsWetDry1D #10
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 4 +3
Lines 3 85 +82
=========================================
+ Hits 3 85 +82 ☔ View full report in Codecov by Sentry. |
|
@JoshuaLampert thanks for investigating and finding the problem! This was very helpful. |
|
Yes, that makes sense. |
There was a problem hiding this comment.
That you for tackling this @patrickersing ! Overall I like this structure and since we have using Trixi in src/TrixiShallowWater.jl it forces us to be very specific when certain routines are reused and called from the "parent" Trixi package, e.g. the wrapper needed for cons2prim. I am open if there is a more Julian way to do this, but for new users I like how explicit and obvious it is as to what functions come from this package and what functions are called from the original package.
There was a problem hiding this comment.
Everything basically looks good from my side @patrickersing . Just one last comment needs updated and/or removed.
This PR adds the new equations
ShallowWaterEquationsWetDry1Dtogether with some tests.The PR introduces only a first version of the equations that is supposed to work without any changes to Trixi.jl. For this reason some features that require changes in Trixi.jl are not moved yet. The main idea is to have a first working PR to test and discuss how to do the separation from Trixi.jl.
Here are some important points and changes to discuss:
I think the main consideration is how we want to interface with Trixi.jl. Mainly how we
import/exportfunctions that are used in both Packages. Here from what I can tell we have different options.Currently I import only Trixi.jl and then qualify the function names with
Trixi.func. We could also import these functions explicitly, but I like that in the current way you can directly see, which functions come from Trixi.jl.I currently have to make a copy of
test_trixi.jlto get access to the testing macros. Is this okay or should we make some changes to export this from Trixi.jl?I have added a new test to check for namespace conflicts between Trixi.jl and TrixiShallowWater.jl in
runtests.jl. Can you think of additional testing we should include to ensure compatibility?For the new equation I need to introduce a lot of wrapper functions to existing Trixi.jl functions as the basic model remains in Trixi.jl, e.g.
@inline function Trixi.flux(u, orientation::Integer, equations::ShallowWaterEquationsWetDry1D) return Trixi.flux(u, orientation, Trixi.ShallowWaterEquations1D(equations.gravity, equations.H0, eps(), eps())) endLet me know if you can think of a better way to do this.
Besides that I think it would be good to check the new CI workflow that I modified.