-
Notifications
You must be signed in to change notification settings - Fork 4
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 ShallowWaterEquationsWetDry1D (2) #15
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15 +/- ##
============================================
- Coverage 100.00% 88.73% -11.27%
============================================
Files 1 1
Lines 3 71 +68
============================================
+ Hits 3 63 +60
- Misses 0 8 +8 ☔ View full report in Codecov by Sentry. |
Ci seems to work correctly 🥳 |
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.
Overall this looks good to me, I only left a few smaller comments. however, I did not check the implementations in detail - I assume they are just copy-pasted from Trixi.jl and adapted for types (i.e., Trixi.AbstractEquations
instead of AbstractEquations
)?
Note that you should also modify .github/workflows/ci.yml
to process the coverage data for the examples/
directory, similarly to how we do it in Trixi.jl
Yes, currently there are no additional features in this package and implementations were copied and adapted from I have modified the As the problem with the status checks also popped up in this PR, I would resume to work on the original PR and close this one as soon as your comments are resolved. |
Besides this comment,
I think all is resolved. |
With the fix in 00247c4 the modified |
🎉 |
This PR adds the new equations ShallowWaterEquationsWetDry1D together 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.