Move some initial conditions (mostly 1D) #847

ranocha · 2021-09-09T07:46:49Z

Contributes to #685

codecov · 2021-09-09T12:20:33Z

Codecov Report

Merging #847 (4d9e924) into main (1dcb022) will decrease coverage by 0.00%.
The diff coverage is 96.48%.

❗ Current head 4d9e924 differs from pull request most recent head 4f3a7f2. Consider uploading reports for the commit 4f3a7f2 to get more accurate results

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
- Coverage   92.96%   92.96%   -0.00%     
==========================================
  Files         190      211      +21     
  Lines       17815    17880      +65     
==========================================
+ Hits        16561    16621      +60     
- Misses       1254     1259       +5

Flag	Coverage Δ
unittests	`92.96% <96.48%> (-<0.01%)`	⬇️

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

Impacted Files	Coverage Δ
src/Trixi.jl	`66.67% <ø> (ø)`
src/equations/compressible_euler_1d.jl	`92.45% <ø> (+0.15%)`	⬆️
src/equations/compressible_euler_2d.jl	`94.08% <ø> (-0.09%)`	⬇️
src/equations/compressible_euler_3d.jl	`95.09% <ø> (-0.11%)`	⬇️
src/equations/hyperbolic_diffusion_1d.jl	`84.51% <ø> (ø)`
src/equations/ideal_glm_mhd_1d.jl	`91.98% <ø> (-1.03%)`	⬇️
src/equations/ideal_glm_mhd_2d.jl	`93.60% <ø> (-0.20%)`	⬇️
src/equations/ideal_glm_mhd_multicomponent_1d.jl	`95.90% <ø> (-0.22%)`	⬇️
examples/tree_1d_dgsem/elixir_euler_blast_wave.jl	`88.89% <88.89%> (ø)`
...r_euler_blast_wave_neuralnetwork_perssonperaire.jl	`88.89% <88.89%> (ø)`
... 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 1dcb022...4f3a7f2. Read the comment docs.

test/test_tree_1d_mhd.jl

src/equations/compressible_euler_3d.jl

jlchan

Overall LGTM. Out of curiosity, how did you choose which ICs to keep as exported?

NEWS.md

examples/tree_1d_dgsem/elixir_eulermulti_two_interacting_blast_waves.jl

jlchan · 2021-09-11T15:03:33Z

examples/tree_1d_dgsem/elixir_mhd_shu_osher_shock_tube.jl

+  rho = x[1] <= x_0 ? 3.5 : 1.0 + 0.2 * sin(5.0 * x[1])
+  v1 = x[1] <= x_0 ? 5.8846 : 0.0
+  v2 = x[1] <= x_0 ? 1.1198 : 0.0
+  v3 = 0.0
+  p = x[1] <= x_0 ? 42.0267 : 1.0
+  B1 = 1.0
+  B2 = x[1] <= x_0 ? 3.6359 : 1.0


Same question on <= vs <

@andrewwinters5000? You added this in 53fd46a

This is the setup that Dominik and I used back in the day for the FLASH code. He found it in this PhD thesis https://uwspace.uwaterloo.ca/handle/10012/8597

jlchan · 2021-09-11T15:03:35Z

examples/tree_1d_dgsem/elixir_mhd_ryujones_shock_tube.jl

+  rho = x[1] <= 0.5 ? 1.08 : 1.0
+  v1 = x[1] <= 0.5 ? 1.2 : 0.0
+  v2 = x[1] <= 0.5 ? 0.01 : 0.0
+  v3 = x[1] <= 0.5 ? 0.5 : 0.0
+  p = x[1] <= 0.5 ? 0.95 : 1.0
+  inv_sqrt4pi = 1.0 / sqrt(4 * pi)
+  B1 = 2 * inv_sqrt4pi
+  B2 = x[1] <= 0.5 ? 3.6 * inv_sqrt4pi : 4.0 * inv_sqrt4pi


Just out of curiosity, why <= and not < here? The Brio-Wu shock tube also uses <.

@andrewwinters5000? You added this in 53fd46a

The Ryu & Jones reference is slightly ambiguous in that they just define a left and right state but don't actually state things in terms of < versus <= so I just made a decision. The same is true for the Torrilhon shocktube test. For Brio & Wu I followed how FLASH does it and they use the <

jlchan · 2021-09-11T15:03:50Z

examples/tree_1d_dgsem/elixir_mhd_shu_osher_shock_tube.jl

+  rho = x[1] <= x_0 ? 1.0 + 0.2 * sin(5.0 * x[1]) : 3.5
+  v1 = x[1] <= x_0 ? 0.0 : -5.8846
+  v2 = x[1] <= x_0 ? 0.0 : -1.1198
+  v3 = 0.0
+  p = x[1] <= x_0 ? 1.0 : 42.0267
+  B1 = 1.0
+  B2 = x[1] <= x_0 ? 1.0 : 3.6359


Same question on <= vs <

jlchan · 2021-09-11T15:03:57Z

examples/tree_1d_dgsem/elixir_mhd_torrilhon_shock_tube.jl

+  rho = x[1] <= 0 ? 3.0 : 1.0
+  v1 = 0.0
+  v2 = 0.0
+  v3 = 0.0
+  p = x[1] <= 0 ? 3.0 : 1.0
+  B1 = 1.5
+  B2 = x[1] <= 0 ? 1.0 : cos(1.5)
+  B3 = x[1] <= 0 ? 0.0 : sin(1.5)


Same question on <= vs <

@andrewwinters5000? You added this in 518899d

jlchan · 2021-09-11T15:05:24Z

examples/tree_1d_dgsem/elixir_mhdmulti_briowu_shock_tube.jl

+    prim_rho  = SVector{ncomponents(equations), real(equations)}(2^(i-1) * (1-2)/(1-2^ncomponents(equations)) * rho for i in eachcomponent(equations))
+  else
+    rho       = 0.125
+    prim_rho  = SVector{ncomponents(equations), real(equations)}(2^(i-1) * (1-2)/(1-2^ncomponents(equations)) * rho for i in eachcomponent(equations))


Why (1-2) and not just -1?

@Cczernik? You added this in #449

No deeper meaning, -1 can also be used

src/equations/compressible_euler_2d.jl

sloede

I didn't do a full review but left a few comments. Overall, I'm ok with this PR, I just would like to have at least one "canonical" setup in the Trixi that is entropy-producing (but I think the medium Sedov blast accounts for that).

jlchan · 2021-09-11T15:14:37Z

I didn't do a full review but left a few comments. Overall, I'm ok with this PR, I just would like to have at least one "canonical" setup in the Trixi that is entropy-producing (but I think the medium Sedov blast accounts for that).

Doesn't any discontinuous IC also produce entropy? I seem to remember the weak blast wave being used as an entropy conservation test.

sloede · 2021-09-11T15:58:55Z

I didn't do a full review but left a few comments. Overall, I'm ok with this PR, I just would like to have at least one "canonical" setup in the Trixi that is entropy-producing (but I think the medium Sedov blast accounts for that).

Doesn't any discontinuous IC also produce entropy? I seem to remember the weak blast wave being used as an entropy conservation test.

Yes, but the weak blast wave is so weak that it is not a very hard test case for EC/ES schemes. You can run it with an EC flux and it remains stable IIRC

Co-authored-by: Jesse Chan <[email protected]>

sloede

with the remaining comments resolved, this LGTM

src/equations/compressible_euler_3d.jl

test/test_tree_1d_mhd.jl

Co-authored-by: Michael Schlottke-Lakemper <[email protected]>

ranocha · 2021-09-12T07:29:57Z

I would like to keep the questions on specific implementations of some ICs open (since this PR didn't change them at all, just moved them somewhere else). This allows us to merge this one soon and move on while we can keep discussing the open questions here and make new PRs if appropriate.

ranocha · 2021-09-12T07:32:14Z

Out of curiosity, how did you choose which ICs to keep as exported?

I concentrated on 1D here. Thus, I touched only the 2D/3D parts that were necessary after doing the 1D changes. My rules were

Keep academic test cases in src (constant IC, convergence test, weak blast wave as EC test)
Move "known" examples to examples (Sedov blast wave)

I tried to follow #685 (comment) and #237 (comment)

jlchan · 2021-09-12T12:17:33Z

Sounds good. Since my comments were mostly on old commits we can ignore those for this PR. LGTM - feel free to merge when ready.

ranocha added 8 commits September 9, 2021 07:15

fix NEWS

f99f330

fix typo in NEWS

73f6fb4

Brio-Wu shock tube

67144d1

Torrilhon shock tube

184f315

Ryu-Jones shock tube

e68f3ac

Shu-Osher shock tube

ea2f6e8

blast wave and other stuff

6fa989e

Sedov blast wave

9efdd48

ranocha requested a review from sloede September 9, 2021 07:46

ranocha mentioned this pull request Sep 9, 2021

Refactor pre-defined initial & boundary conditions from src/equations/ into examples/.../ #685

Closed

6 tasks

ranocha added 2 commits September 9, 2021 12:56

Trixi.initial_condition_sedov_blast_wave for dummy 3D test

0b21953

Merge branch 'main' into hr/ics

238956e

ranocha requested a review from jlchan September 11, 2021 09:11

sloede reviewed Sep 11, 2021

View reviewed changes

test/test_tree_1d_mhd.jl Show resolved Hide resolved

sloede reviewed Sep 11, 2021

View reviewed changes

src/equations/compressible_euler_3d.jl Outdated Show resolved Hide resolved

jlchan requested changes Sep 11, 2021

View reviewed changes

sloede reviewed Sep 11, 2021

View reviewed changes

src/equations/compressible_euler_2d.jl Show resolved Hide resolved

sloede reviewed Sep 11, 2021

View reviewed changes

remove obsolete comment

3576c75

Co-authored-by: Jesse Chan <[email protected]>

ranocha requested review from sloede and jlchan September 12, 2021 04:59

sloede previously approved these changes Sep 12, 2021

View reviewed changes

src/equations/compressible_euler_3d.jl Outdated Show resolved Hide resolved

test/test_tree_1d_mhd.jl Show resolved Hide resolved

Update test/test_tree_1d_mhd.jl

8f9956b

Co-authored-by: Michael Schlottke-Lakemper <[email protected]>

ranocha dismissed sloede’s stale review via 8f9956b September 12, 2021 07:25

ranocha and others added 2 commits September 12, 2021 09:26

remove 3D Sedov blast wave

c695909

Merge branch 'main' into hr/ics

4d9e924

jlchan approved these changes Sep 12, 2021

View reviewed changes

Merge branch 'main' into hr/ics

4f3a7f2

ranocha merged commit c792bb7 into main Sep 12, 2021

ranocha deleted the hr/ics branch September 12, 2021 13:20

ranocha mentioned this pull request Oct 5, 2021

move initial/boundary conditions to elixirs #895

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move some initial conditions (mostly 1D) #847

Move some initial conditions (mostly 1D) #847

ranocha commented Sep 9, 2021

codecov bot commented Sep 9, 2021 •

edited

Loading

jlchan left a comment

jlchan Sep 11, 2021

ranocha Sep 12, 2021

andrewwinters5000 Sep 13, 2021

jlchan Sep 11, 2021

ranocha Sep 12, 2021

andrewwinters5000 Sep 13, 2021

jlchan Sep 11, 2021

jlchan Sep 11, 2021

ranocha Sep 12, 2021

jlchan Sep 11, 2021

ranocha Sep 12, 2021

Cczernik Sep 13, 2021

sloede left a comment

jlchan commented Sep 11, 2021

sloede commented Sep 11, 2021

sloede left a comment

ranocha commented Sep 12, 2021

ranocha commented Sep 12, 2021 •

edited

Loading

jlchan commented Sep 12, 2021 •

edited

Loading

Move some initial conditions (mostly 1D) #847

Move some initial conditions (mostly 1D) #847

Conversation

ranocha commented Sep 9, 2021

codecov bot commented Sep 9, 2021 • edited Loading

Codecov Report

jlchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sloede left a comment

Choose a reason for hiding this comment

jlchan commented Sep 11, 2021

sloede commented Sep 11, 2021

sloede left a comment

Choose a reason for hiding this comment

ranocha commented Sep 12, 2021

ranocha commented Sep 12, 2021 • edited Loading

jlchan commented Sep 12, 2021 • edited Loading

codecov bot commented Sep 9, 2021 •

edited

Loading

ranocha commented Sep 12, 2021 •

edited

Loading

jlchan commented Sep 12, 2021 •

edited

Loading