Skip to content
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

WIP: Taal #187

Merged
merged 66 commits into from
Oct 6, 2020
Merged

WIP: Taal #187

merged 66 commits into from
Oct 6, 2020

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Sep 17, 2020

This implements a first prototype of Taal: Trixi as a library. There is still a lot of work to do before we reach feature parity with the previous versions, but we can run a simple example such as examples/2d/parameters.tomlexamples/2d/parameters.jl. I wanted to push this now so that you can get an impression of the look & feel and we can discuss how to proceed.

TODO

  • basic infrastructure, semidiscretizations etc.
  • Euler + gravity
  • weak form
  • split form
  • shock capturing
  • boundary conditions
  • non-conservative terms (MHD)
  • restarts
  • SummaryCallback
  • AMRCallback
  • AMRCallback for Euler + gravity
  • StepsizeCallback
  • AnalysisCallback
  • SaveSolutionCallback
  • AliveCallback
  • make it easier to create new callbacks by hiding some DiffEq-specific stuff?
  • positivity preservation
  • Trixi.include with the ability to modify stuff in the REPL via keyword arguments
  • make polydeg and cfl explicit parameters in order to be able to modify them 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
  • elixir: linear structure
  • elixir: Jacobian (FD)
  • elixir: fluctuation stuff
  • elixir: convtest
  • migrate every test case
  • update docs and README
  • 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, e.g. elixir_eqs_specification.jl in general: elixir_advection_basic.jl instead of parameters.jl
  • Add one non-DiffEq implemention for a RK scheme (e.g. Carpenter/Kennedy LSRK45) for easy experimenting/prototyping
  • fix all remaining # TODO: Taal
  • 1D
  • 3D

Connections to issues

Xref #185 #177 #161 #150 #139 #136 #83 #70 #19 #10

Closes #173; closes #73

@ranocha ranocha added enhancement New feature or request high-priority labels Sep 17, 2020
@ranocha ranocha marked this pull request as draft September 17, 2020 17:29
@ranocha ranocha requested a review from sloede September 17, 2020 17:30
@ranocha
Copy link
Member Author

ranocha commented Sep 17, 2020

By the way, I've marked this as "high-priority", since @gregorgassner categorized Taal in this way in our last meeting.

@codecov
Copy link

codecov bot commented Sep 18, 2020

Codecov Report

Merging #187 into dev will decrease coverage by 2.29%.
The diff coverage is 80.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #187      +/-   ##
==========================================
- Coverage   91.53%   89.24%   -2.30%     
==========================================
  Files          36       52      +16     
  Lines        8037     9923    +1886     
==========================================
+ Hits         7357     8856    +1499     
- Misses        680     1067     +387     
Impacted Files Coverage Δ
src/Trixi.jl 100.00% <ø> (ø)
src/equations/3d/hyperbolic_diffusion.jl 78.98% <0.00%> (-0.51%) ⬇️
src/solvers/dg/1d/dg.jl 92.93% <0.00%> (ø)
src/solvers/dg/3d/dg.jl 92.10% <0.00%> (ø)
src/solvers/solvers.jl 90.90% <ø> (ø)
src/auxiliary/containers.jl 90.75% <42.85%> (ø)
src/equations/2d/linear_scalar_advection.jl 76.54% <44.44%> (-4.54%) ⬇️
src/equations/3d/ideal_glm_mhd.jl 92.49% <50.00%> (-0.21%) ⬇️
src/mesh/tree.jl 94.63% <50.00%> (ø)
src/solvers/dg/3d/containers.jl 91.80% <50.00%> (-0.14%) ⬇️
... and 47 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 bea72a0...abd7087. Read the comment docs.

@ranocha
Copy link
Member Author

ranocha commented Sep 18, 2020

We now have more analysis stuff, but that requires SciML/OrdinaryDiffEq.jl#1272 for 2N low-storage methods.

Memo: Set compat entry for dependencies on OrdinaryDiffEq accordingly once that PR is merged and a new version is released.
Edit: compat entry updated.

@ranocha
Copy link
Member Author

ranocha commented Sep 30, 2020

I've now finished a first draft of the most important design decisions and most features are implemented in 2D. Thus, it's a good time to get some feedback!

The current test tolerances make use of one of my PRs into OrdinaryDiffEq.jl. Hence, tests will currently fail on Travis and locally for you (unless you pull the current master of OrdinaryDiffEq). Nevertheless, the basic setup should also work for you locally with a current release of OrdinaryDiffEq. Edit: OrdinaryDiffEq.jl had a new release and tests pass.

@sloede @gregorgassner @andrewwinters5000 Could you please review this PR carefully?

@ranocha
Copy link
Member Author

ranocha commented Sep 30, 2020

When everything looks okay to you, I would like to merge this into trixi-framework:dev and continue to develop Trixi as a library in my fork, creating PRs for every separate WP into that branch until we're ready to merge into trixi-framework:master.

@ranocha ranocha changed the base branch from master to hr/taal September 30, 2020 16:47
@ranocha ranocha changed the base branch from hr/taal to dev October 1, 2020 05:15
@ranocha ranocha marked this pull request as ready for review October 4, 2020 14:44
.gitignore Show resolved Hide resolved
README.md Show resolved Hide resolved
Comment on lines +43 to +47
# TODO: Taal, decide where to define the entry points of our type hierarchy,
# e.g. AbstractEquations, AbstractSemidiscretization etc.
# Placing them here will allow us to make use of them for dispatch even for
# other stuff defined very early in our include pipeline, e.g.
# IndicatorLöhner(semi::AbstractSemidiscretization)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the downsides to doing this? I don't see any, except that I'd prefer to put those entry points into a separate file if it becomes more than ten lines or so, just to keep Trixi.jl easy to read for someone new.

Copy link
Member Author

@ranocha ranocha Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay for me to put these into a separate file. The only downside could be that someone would expect to find the definition of AbstractEquations in equations.jl instead of abstract_types.jl. But that might not be a problem if we leave a comment in Trixi.jl and use a descriptive name for the source file containing the abstract type definitions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

@ranocha ranocha merged commit dcaa7cd into trixi-framework:dev Oct 6, 2020
Comment on lines +33 to +50
walkexpr(f, expr::Expr) = f(Expr(expr.head, (walkexpr(f, arg) for arg in expr.args)...))
walkexpr(f, x) = f(x)

function mapexpr(expr; kwargs...)
walkexpr(expr) do x
if x isa Expr
for (key,val) in kwargs
if x.head === Symbol("=") && x.args[1] === Symbol(key)
x.args[2] = :( $val )
# dump(x)
end
end
end
return x
end
end

function test_trixi_include(parameters_file; l2=nothing, linf=nothing,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods should get at least some rudimentary high-level comment/docstring on what their purpose is and how they do it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do that.

test/test_trixi.jl Show resolved Hide resolved
README.md Show resolved Hide resolved
Comment on lines +43 to +47
# TODO: Taal, decide where to define the entry points of our type hierarchy,
# e.g. AbstractEquations, AbstractSemidiscretization etc.
# Placing them here will allow us to make use of them for dispatch even for
# other stuff defined very early in our include pipeline, e.g.
# IndicatorLöhner(semi::AbstractSemidiscretization)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

src/auxiliary/containers.jl Show resolved Hide resolved
src/callbacks/alive.jl Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high-priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the number of analysis nodes a parameter
2 participants