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

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

Closed
6 tasks done
Tracked by #720
stillyslalom opened this issue Jun 30, 2021 · 5 comments · Fixed by #895
Closed
6 tasks done
Tracked by #720
Labels

Comments

@stillyslalom
Copy link
Contributor

stillyslalom commented Jun 30, 2021

When creating a new simulation from scratch, I had to bounce between the equations/ folder and the examples/ folder to figure out the syntax for a new set of initial & boundary conditions. Ideally, the examples would be self-contained; from the user's point of view, there's nothing special about boundary_condition_two_interacting_blast_waves, which is exported as part of Trixi's API - it's only useful in the context of the associated elixir in the examples/.../ folder.

TODO

  • 1D (Move some initial conditions (mostly 1D) #847)
  • 2D
  • 3D
  • What about initial_condition_weak_blast_wave? → Keep it in src since it's really just an academic test case used for EC verification
  • What about hyperbolic diffusion stuff?
  • Look for TODO: ICs
@ranocha ranocha added the good first issue Good for newcomers label Jun 30, 2021
@ranocha
Copy link
Member

ranocha commented Jun 30, 2021

Good suggestion. We've already discussed this in #237. The consensus was to use the following rough guideline.

  • Functions that are only used in one elixir should be defined there.
  • Functions that are verification tests or academic setups used in multiple elixirs are defined in equations/.
  • Functions that are used in multiple elixirs may be defined in equations/ or in the elixirs.
  • Functions that describe well-known applications from literature (Sedov, Kelvin-Helmholtz, Rotor, Orzag-Tang, ...) may be defined directly in the elixir

If boundary_condition_two_interacting_blast_waves is only used in one elixir, it falls into the first category and should be moved to the appropriate elixir.

@sloede
Copy link
Member

sloede commented Jun 30, 2021

The consensus was to use the following rough guideline.

Should we maybe add this to https://trixi-framework.github.io/Trixi.jl/stable/conventions/?

@stillyslalom
Copy link
Contributor Author

It seems like almost every elixir in examples/ is used as a verification test, which means that in practice, there are very few self-contained examples. There's only one 1D elixir that defines its own initial condition:

grep "function initial_condition" Trixi/examples/1d/*
Trixi/examples/1d/elixir_burgers_linear_stability.jl:function initial_condition_linear_stability(x, t, equation::InviscidBurgersEquation1D)

...while there are dozens that use predefined ICs:

grep "initial_condition = " Trixi/examples/1d/*
Trixi/examples/1d/elixir_advection_amr.jl:initial_condition = initial_condition_gauss
Trixi/examples/1d/elixir_advection_amr_nonperiodic.jl:initial_condition = initial_condition_gauss
Trixi/examples/1d/elixir_advection_extended.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_advection_nonperiodic_curved.jl:initial_condition = initial_condition_gauss
Trixi/examples/1d/elixir_burgers_basic.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_euler_blast_wave.jl:initial_condition = initial_condition_blast_wave
Trixi/examples/1d/elixir_euler_convergence_test_pure_fv.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_euler_density_wave.jl:initial_condition = initial_condition_density_wave
Trixi/examples/1d/elixir_euler_ec.jl:initial_condition = initial_condition_weak_blast_wave
Trixi/examples/1d/elixir_euler_nonperiodic.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_euler_nonperiodic_curved.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_euler_positivity.jl:initial_condition = initial_condition_sedov_blast_wave
Trixi/examples/1d/elixir_euler_sedov_blast_wave.jl:initial_condition = initial_condition_sedov_blast_wave
Trixi/examples/1d/elixir_euler_sedov_blast_wave_pure_fv.jl:initial_condition = initial_condition_sedov_blast_wave
Trixi/examples/1d/elixir_euler_shockcapturing.jl:initial_condition = initial_condition_weak_blast_wave
Trixi/examples/1d/elixir_euler_source_terms.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_euler_source_terms_curved.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_eulergravity_eoc_test.jl:initial_condition = initial_condition_eoc_test_coupled_euler_gravity
Trixi/examples/1d/elixir_eulermulti_ec.jl:initial_condition = initial_condition_weak_blast_wave
Trixi/examples/1d/elixir_eulermulti_eoc_ec.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_eulermulti_eoc_es.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_eulermulti_es.jl:initial_condition = initial_condition_weak_blast_wave
Trixi/examples/1d/elixir_eulermulti_two_interacting_blast_waves.jl:initial_condition = initial_condition_two_interacting_blast_waves
Trixi/examples/1d/elixir_hypdiff_harmonic_nonperiodic.jl:initial_condition = initial_condition_harmonic_nonperiodic
Trixi/examples/1d/elixir_hypdiff_nonperiodic.jl:initial_condition = initial_condition_poisson_nonperiodic
Trixi/examples/1d/elixir_mhd_alfven_wave.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_mhd_briowu_shock_tube.jl:initial_condition = initial_condition_briowu_shock_tube
Trixi/examples/1d/elixir_mhd_ec.jl:initial_condition = initial_condition_briowu_shock_tube
Trixi/examples/1d/elixir_mhd_ryujones_shock_tube.jl:initial_condition = initial_condition_ryujones_shock_tube
Trixi/examples/1d/elixir_mhd_shu_osher_shock_tube.jl:initial_condition = initial_condition_shu_osher_shock_tube
Trixi/examples/1d/elixir_mhd_torrilhon_shock_tube.jl:initial_condition = initial_condition_torrilhon_shock_tube
Trixi/examples/1d/elixir_mhdmulti_briowu_shock_tube.jl:initial_condition = initial_condition_briowu_shock_tube
Trixi/examples/1d/elixir_mhdmulti_ec.jl:initial_condition = initial_condition_briowu_shock_tube
Trixi/examples/1d/elixir_mhdmulti_eoc.jl:initial_condition = initial_condition_convergence_test
Trixi/examples/1d/elixir_mhdmulti_es.jl:initial_condition = initial_condition_briowu_shock_tube

It seems as if the full grid of verification tests would be better-located in the test/ folder, with examples/ dedicated to pedagogical examples. Separately, instead of organizing the elixirs by dimensionality, they could be organized in subfolders corresponding to the equations being solved.

@ranocha
Copy link
Member

ranocha commented Jul 3, 2021

Separately, instead of organizing the elixirs by dimensionality, they could be organized in subfolders corresponding to the equations being solved.

That was discussed and decided in #569.

It seems as if the full grid of verification tests would be better-located in the test/ folder, with examples/ dedicated to pedagogical examples.

There are plans to make the examples more searchable (#610) and create tutorials (#204, #169) in the documentation. These tutorials would basically serve the purpose of pedagogical examples. We just need more time and people to work on these issue.

There's always a tension between keeping code for a specific task in one place vs. avoiding code duplication. Based on #237 (comment), we might decide to move more "standard application" setups to the elixirs. For example, given your 1D list above, these could be

  • initial_condition_sedov_blast_wave
  • initial_condition_shu_osher_shock_tube
  • initial_condition_briowu_shock_tube
  • initial_condition_torrilhon_shock_tube (used in only one elixir - should definitely be moved)
  • initial_condition_ryujones_shock_tube (used in only one elixir - should definitely be moved)

However, that would be a breaking change, so we need to be careful when deprecating stuff etc.

@ranocha
Copy link
Member

ranocha commented Oct 7, 2021

We moved quite a lot of initial and boundary conditions to the elixirs in examples - we only kept very academic test cases in src.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants