Merge Taal into master

[ranocha]

This PR will track the overall development of Taal (Trixi as a library). Separate work packages will be implemented in PRs into the dev branch until we're ready to merge into master.

Here is our TODO list from #187

TODO

• basic infrastructure, semidiscretizations etc.
• Euler + gravity
• weak form
• split form
• shock capturing
• boundary conditions
• non-conservative terms (MHD)
• restarts (tracked in Taal: restarts #271)
• SummaryCallback
• AMRCallback
• AMRCallback for Euler + gravity
• StepsizeCallback
• AnalysisCallback
• SaveSolutionCallback
• AliveCallback
• make it easier to create new callbacks by hiding some DiffEq-specific stuff? (tracked in Taal: Make it easier to create new callbacks #261)
• add a "default I/O" callback (or method that returns something like it) that creates a SummaryCallback, AliveCallback, AnalysisCallback, and SaveSolutionCallback with sane defaults and keyword arguments for all relevant arguments (tracked in Taal: Make it easier to create new callbacks #261)
• positivity preservation (tracked in Taal: Positivity preserving limiter #270, implemented in PositivityPreservingLimiterZhangShu #274)
• SteadyStateCallback stuff for hyperbolic diffusion
• trixi_include with the ability to modify stuff in the REPL via keyword arguments
• make polydeg an explicit parameter in order to be able to modify it via keyword arguments in trixi_include
• move definitions of abstract types to abstract_types.jl to enable dispatch on them throughout Trixi.jl
• allow different real types for the mesh and all other data structures
• ensure that we have enough flexibility in the caches to enable AD (tracked in Taal: Enable AD #260)
• elixir: linear structure
• elixir: Jacobian using FD (Taal: simple Jacobian computation using central FD #262)
• elixir: fluctuation stuff (tracked in Taal: Create a new elixir for fluctuation stuff #269)
• elixir: convtest
• migrate every test case (tracked in Taal: Porting parameters_*.toml to elixir_*.jl #219)
• update docs and README (update docs to Taal and fix typos #298)
• more inline documentation and explanations
• consistent naming of Vector-like u from DiffEq.jl and "our" solution array u and documentation/explanation of wrap_array
• consistent use of return, update the dev docs accordingly
• consistent and descriptive naming of the examples; elixir_eqs_specification.jl in general, e.g. elixir_advection_basic.jl instead of parameters.jl.
• We need to rename parameters_file (and everything else where parameters occurs) to elixir or something similar.
• Add one non-DiffEq implemention for a RK scheme (e.g. Carpenter/Kennedy LSRK45) for easy experimenting/prototyping (Simple self-cooked time integration faking the interface of OrdinaryDiffEq #272)
• fix all remaining # TODO: Taal
• 1D
• 2D
• 3D (Taal 3D #255)
• Merge MPI from master into dev and resolve conflicts, make everything work, ...
• remove all old code not needed in Taal anymore
• sort tests by equation (Taal: sort and restructure tests #301)
• prettify simulation summary output (Taal: Prettify simulation summary  #302)
• re-create asciicasts (Taal: Re-create ASCII casts for the docs #303)

Connections to issues

Xref #161 #19

Closes #237; closes #236; closes #197; closes #185; closes #177; closes #176; closes #173; closes #150; closes #142 (obsolete); closes #139; closes #136 (obsolete); closes #98; closes #83; closes #74 (obsolete); closes #73; closes #70; closes #68 (outdated); closes #42; closes #10; closes #9 (outdated)

Blocked by #207, #219, #233, #236, #240, #243, #267, #287, #301 (i.e., do not merge before these are resolved or removed from this list).

[codecov]

Codecov Report

Merging #200 (2692ef0) into master (662da42) will decrease coverage by 2.98%.
The diff coverage is 85.40%.

@@            Coverage Diff             @@
##           master     #200      +/-   ##
==========================================
- Coverage   91.49%   88.50%   -2.99%     
==========================================
  Files          42       76      +34     
  Lines        8867     8885      +18     
==========================================
- Hits         8113     7864     -249     
- Misses        754     1021     +267     

Flag	Coverage Δ
unittests	88.50% <85.40%> (-2.99%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Trixi.jl	100.00% <ø> (ø)
src/auxiliary/mpi.jl	91.66% <ø> (ø)
src/callbacks_stage/positivity_zhang_shu_dg1d.jl	0.00% <0.00%> (ø)
src/callbacks_stage/positivity_zhang_shu_dg2d.jl	0.00% <0.00%> (ø)
src/equations/ideal_glm_mhd_3d.jl	89.70% <ø> (ø)
src/equations/linear_scalar_advection_1d.jl	80.32% <ø> (ø)
src/equations/linear_scalar_advection_2d.jl	77.50% <ø> (ø)
src/equations/linear_scalar_advection_3d.jl	76.47% <ø> (ø)
src/mesh/abstract_tree.jl	90.37% <ø> (+0.10%)	⬆️
src/mesh/mesh.jl	80.32% <ø> (-5.98%)	⬇️
... and 129 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 662da42...2692ef0. Read the comment docs.

[sloede]

Alright, I've reviewed the entire dev branch at b37c070. Again, great work @ranocha, this is really a huge milestone for Trixi! And at >6000 lines touched...

I left a few comments, some of which are open questions to start a discussion. Overall, there are a few things I would like to note, which more or less apply to the entire changeset:

• dimension-agnostic solvers: It seems like at some places the distinction between the 1D, 2D, and 3D solvers becomes blurry, and in same cases (e.g., indicators) it's already gone. This is at least something we should discuss together.
• restructuring of where methods/structs reside: Some methods and structs feel awkwardly placed, and I've marked this on several occasions. Maybe once Taal is finished in 2D in terms of features, we should sit together and think about a good file and directory structure that makes sense to everyone. Maybe we'll end up with what we have right now, but at least to me some things seemed counter-intuitive.
• distinct method names & method comments: * This code now heavily relies on Julia's dynamic dispatch capabilities. That is, many methods have the same name and are used in different parts of the code for a similar purpose (e.g., 7x compute_coefficients(!), 5x analyze, 14x create_cache). I'd argue that it might be sensible to find different names for some of these (e.g., create_solver_cache), if that is possible at all. I strongly argue that with such a large changeset coming, we should make sure that we at least add a one-line comment to each method that states its purpose and, preferably, from where it is (usually) called. @ranocha started doing this already, but I think it is really necessary for all methods.
• inline comments: I think most of the new code is severely undercommented (is that even a word? If not, I claim first use!). That is, besides providing summary comments that describe an entire method's purpose, we should also add many more comments to the method body itself.
  This is especially true for the infrastructure methods (I am looking at you, callbacks!), since there the meaning and purpose of the code is sometimes not easily discernible without knowing much more about the inner workings of Trixi and DiffEq.jl. Also, it would help us when directing students if the code is well-documented at this stage, at least until everyone has started to get a feel for Taal. Right now, I doubt that anyone except @ranocha (and a little bit me by now) is able to navigate Taal, and even I would not be able to explain to a student the overall infrastructure of Trixi anymore (at least when going beyond the 3-4 top-level methods called directly from the elixirs).
• review by everyone: I know this is going to be hard, but I strongly urge @gregorgassner and @andrewwinters5000 to also go through the code and try to understand it not just in a high-level fashion, but also in detail - at least for things like callbacks, indicators, and how the new mesh-equations-solver-cache infrastructure is set up. Otherwise it will be hard to change anything fundamental, which is antithetical to the most basic goal of Trixi being an easy-to-understand code.

If you find anything unclear in my comments, please let me know. If I occasionally seem to be overly critical, please don't mind the straight language and know that I wrote everything with a constructive intent in mind. Finally, sorry it took me so long, but it was a lot to digest 🤓

[ranocha]

Thanks a lot for your detailed review and comments, @sloede! I will go through them. Let's have some nice discussions and improve Taal for all of us. Some general reactions to your major comments:

• dimension-agnostic solvers

  It seems like at some places the distinction between the 1D, 2D, and 3D solvers becomes blurry, and in same cases (e.g., indicators) it's already gone. This is at least something we should discuss together.

  You're definitely right (to some extend). Please, keep in mind that the previous Dg2D instances now correspond to the semi::SemidiscretizationHyperbolic instances, which are made up of mesh, equations, solver, cache (and more ingredients). In particular, the new dg::DG objects do not know anything about the dimension to which they are applied - they just encapsulate the general kind of volume integral and surface fluxes. To make use of multiple dispatch in a more fine-grained manner, if redirected dispatch to a lower level than before and often use the dimensions of the given data/coefficients u, du. We should definitely discuss how we want to structure dispatch and which distinctions we want to make.

• restructuring of where methods/structs reside

  Some methods and structs feel awkwardly placed, and I've marked this on several occasions. Maybe once Taal is finished in 2D in terms of features, we should sit together and think about a good file and directory structure that makes sense to everyone. Maybe we'll end up with what we have right now, but at least to me some things seemed counter-intuitive.

  We should definitely discuss this (and I know that some of us have rather strong opinions on things like these 😉). I think we will get a better idea once we have at least two different space dimensions in Taal.

• distinct method names & method comments

  This code now heavily relies on Julia's dynamic dispatch capabilities. That is, many methods have the same name and are used in different parts of the code for a similar purpose (e.g., 7x compute_coefficients(!), 5x analyze, 14x create_cache). I'd argue that it might be sensible to find different names for some of these (e.g., create_solver_cache), if that is possible at all. I strongly argue that with such a large changeset coming, we should make sure that we at least add a one-line comment to each method that states its purpose and, preferably, from where it is (usually) called. @ranocha started doing this already, but I think it is really necessary for all methods.

  Yeah, I started doing this but there are definitely many opportunities for improvements. From my point of view, it's very similar to documentation: People writing the code are usually the worst at documenting it. Instead, people who read the code and have questions are welcome. After discussing things and explaining the code, both writers and readers get better insights into what's going on and everybody benefits. The people having read the code know which parts are most troublesome and are ideally suited to write good comments and documentation. This comment basically also applies to the next point.

• inline comments

  I think most of the new code is severely undercommented (is that even a word? If not, I claim first use!). That is, besides providing summary comments that describe an entire method's purpose, we should also add many more comments to the method body itself.
  This is especially true for the infrastructure methods (I am looking at you, callbacks!), since there the meaning and purpose of the code is sometimes not easily discernible without knowing much more about the inner workings of Trixi and DiffEq.jl. Also, it would help us when directing students if the code is well-documented at this stage, at least until everyone has started to get a feel for Taal. Right now, I doubt that anyone except @ranocha (and a little bit me by now) is able to navigate Taal, and even I would not be able to explain to a student the overall infrastructure of Trixi anymore (at least when going beyond the 3-4 top-level methods called directly from the elixirs).

[ranocha]

examples/2d/elixir_mhd_rotor_shockcapturing_amr.jl fails again - why?