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

Jo/support 1d #188

Merged
merged 59 commits into from
Oct 1, 2020
Merged

Jo/support 1d #188

merged 59 commits into from
Oct 1, 2020

Conversation

JuliaOd
Copy link
Contributor

@JuliaOd JuliaOd commented Sep 18, 2020

First attempt for a 1D solver.

Copy link
Member

@sloede sloede left a comment

Choose a reason for hiding this comment

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

@JuliaOd I did a first round of reviews, and most of your changes look great! I haven't reviewed dg.jl and dg/amr.jl yet, since they were changed in the current master and might need to be redone (especially when the new boundary condition stuff is merged, which we'll want to have in 1D as well). However, this looks to be on a very good track!

Note that I left a lot of comments with suggestions; if you approve of them, you can apply them automatically (see also https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request).

Project.toml Outdated Show resolved Hide resolved
examples/1d/parameters.toml Show resolved Hide resolved
examples/2d/parameters.toml Outdated Show resolved Hide resolved
examples/2d/parameters.toml Outdated Show resolved Hide resolved
examples/2d/parameters.toml Outdated Show resolved Hide resolved
src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/run.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/dg.jl Outdated Show resolved Hide resolved
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
@codecov
Copy link

codecov bot commented Sep 21, 2020

Codecov Report

Merging #188 into master will decrease coverage by 0.06%.
The diff coverage is 91.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
- Coverage   91.60%   91.53%   -0.07%     
==========================================
  Files          31       36       +5     
  Lines        6958     8037    +1079     
==========================================
+ Hits         6374     7357     +983     
- Misses        584      680      +96     
Impacted Files Coverage Δ
src/solvers/dg/dg.jl 100.00% <ø> (ø)
src/run.jl 94.50% <66.66%> (-0.36%) ⬇️
src/equations/1d/linear_scalar_advection.jl 82.25% <82.25%> (ø)
src/equations/1d/compressible_euler.jl 87.20% <87.20%> (ø)
src/solvers/dg/1d/containers.jl 91.17% <91.17%> (ø)
src/solvers/dg/1d/dg.jl 92.93% <92.93%> (ø)
src/solvers/dg/1d/amr.jl 94.89% <94.89%> (ø)
src/equations/equations.jl 78.04% <100.00%> (+2.37%) ⬆️
src/solvers/solvers.jl 90.90% <100.00%> (+0.90%) ⬆️
... and 4 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 c2d57f6...67aabd4. Read the comment docs.

@JuliaOd JuliaOd marked this pull request as ready for review October 1, 2020 12:57
@sloede sloede requested review from ranocha and sloede October 1, 2020 13:36
@sloede
Copy link
Member

sloede commented Oct 1, 2020

@JuliaOd You should add your name to AUTHORS.md

src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/dg.jl Outdated Show resolved Hide resolved
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Looks good to me (except some minor comments). Great work 👍

One feature I would like to have in 1D concerns visualization: Instead of using trixi2txt from utils/trixi2txt.jl to write a .txt file and read that again in Julia to plot something, could we just have a function returning the respective x and u arrays directly? Then, users can just create their own plots in Julia directly without having to deal with another set of files.

examples/1d/parameters.toml Outdated Show resolved Hide resolved
examples/1d/parameters_amr.toml Outdated Show resolved Hide resolved
examples/1d/parameters_amr_nonperiodic.toml Outdated Show resolved Hide resolved
examples/1d/parameters_ec.toml Outdated Show resolved Hide resolved
examples/1d/parameters_ec.toml Outdated Show resolved Hide resolved
src/equations/1d/compressible_euler.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/containers.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/dg.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/dg.jl Outdated Show resolved Hide resolved
src/solvers/dg/1d/dg.jl Outdated Show resolved Hide resolved
@sloede
Copy link
Member

sloede commented Oct 1, 2020

One feature I would like to have in 1D concerns visualization: Instead of using trixi2txt from utils/trixi2txt.jl to write a .txt file and read that again in Julia to plot something, could we just have a function returning the respective x and u arrays directly? Then, users can just create their own plots in Julia directly without having to deal with another set of files.

This sounds like a good idea. I'll open an issue for it.

Update: #199

@sloede sloede merged commit bea72a0 into trixi-framework:master Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants