Fix convergence problems for flux_nonconservative_ersing_etal_local_jump#131
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
=======================================
Coverage 99.28% 99.28%
=======================================
Files 94 96 +2
Lines 5116 5146 +30
=======================================
+ Hits 5079 5109 +30
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9cdc98b to
a474475
Compare
|
@amrueda To prevent this problem in the future, should we just remove the option to pass the averaged normal direction to the |
That's a good point. In general, I don't think that we'll ever want to pass the averaged normal direction to the I suggest we just remove the normal direction argument in that routine for the time being. If we ever realize we need the two directions in the future, we can create a new PR. |
amrueda
left a comment
There was a problem hiding this comment.
Thanks, @patrickersing!
This looks very good already. Besides not passing the averaged direction to the jump routine, I have the following suggestion:
The current implementation of
flux_nonconservative_ersing_etal_local_jumpdoes not reach the expected convergence order on curvilinear meshes, when used withVolumeIntegralSubcellLimiting. This is because by passing the averaged normal direction we add a symmetric term to the jump part, which is inconsistent with the flux-differencing formula.Luckily, this can be fixed by instead passing only the local normal direction to the local part of the flux. Below are convergence results for the newly added
elixir_shallowwater_multilayer_convergence_sc_subcell_curved.jlfor the local & averaged normal direction.I have also verified entropy conservation for the implementation with the local normal direction for
elixir_shallowwater_multilayer_ec_subcell_sc.jl.Averaged normal direction (Old implementation)
polydeg=3:polydeg=4:Local normal direction (new implementation)
polydeg=3: (Convergence drop probably from EC fluxes)polydeg=4: