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

Reorganize elixir files #569

Closed
efaulhaber opened this issue Apr 29, 2021 · 11 comments · Fixed by #672
Closed

Reorganize elixir files #569

efaulhaber opened this issue Apr 29, 2021 · 11 comments · Fixed by #672
Assignees
Labels
discussion enhancement New feature or request

Comments

@efaulhaber
Copy link
Member

The examples folder is starting to become a bit messy. There are too many files in a single folder (especially in 2D), and it's hard to find the elixirs you're looking for. Also, it's not always clear what the purpose of a specific example file is by the name, and sometimes not even by looking into it (we may have contributed our part there).

I created this issue to discuss new ways to organize example files.

Suggestions we have so far to organize and clean up the examples:

  • Organize files by mesh and semidiscretization. Create folders tree-mesh, structured-curved-mesh, unstructured-quad-mesh, paper-self-gravitating-gas-dynamics and create folders [123]d inside (because quad mesh and self gravitating gas dynamics only have 2d examples).
  • Create a completely different folder for examples that are only relevant for tests (e.g., elixir_advection_amr_refine_twice).
  • Create a new folder for introductory examples (well commented examples to show how the different meshes work) to help beginners finding relevant examples to start with. These should be mentioned in the docs and there could be docs pages for some of these examples that explain in detail what is done in each line.
    Depending on the philosophy, this could be combined with the suggestion above to only have two folders “testing examples” and “introductory examples”.
  • Remove all restart elixirs and create one for all dimensions and examples. These files are basically identical and could be merged to one file that defines elixir_file as variable that can be replaced with kwargs of trixi_include. Maybe there's even a way to extract the number of time steps of the first simulation, so that the elixir can find the correct restart file automatically.
@efaulhaber efaulhaber added enhancement New feature or request discussion labels Apr 29, 2021
@ranocha
Copy link
Member

ranocha commented Apr 30, 2021

I definitely agree with you that it's becoming quite messy... Thanks for opening this issue and making some good suggestions! Here are some of my thoughts.

  • What kind of sorting do we want? Spatial dimension, mesh type, equations, (solver type once FD methods come to Trixi)? I don't think there is one universal choice that's good for everybody, since everyone will have different interests.
  • I definitely like the differentiation between elixirs only relevant for testing and "user-facing" example elixirs, which should be turned into tutorials in the future, cf. Add tutorial to the documentation #169.
  • I like removing redundant code but others might have a different opinion here.

@sloede
Copy link
Member

sloede commented May 8, 2021

Thank you @erik-f for bringing this up! As @ranocha pointed out, this is a non-trivial issue since everyone can have an opinion (high risk of bikeshedding) and everyone has a slightly different perspective on this.

Thus, also some thoughts from my side:

  • I am not sure about the separation into "user-facing" and "testing" elixirs. The only separation I can think of that is clear and unambiguous is "elixirs for which a tutorial exists" and "the merry rest". Since we are only starting about our tutorial efforts, I think this decision should be tabled until we actually start having full-fledged tutorials.
  • Redundancies are not very elegant, but they can lower the entry barrier for new users. However, since for restarts there seems to be literally no change for different mesh/solver types, we can maybe get away with extended documentation (or a short tutorial?).
  • Regarding the folder re-organization: I'd like to try to do this incrementally, thus I'd try to add only one additional layer of hierarchy for now. Mesh and solver seem to be very much connected to me, and the naming is still open. What about equations? E.g., we have something like
examples/
└ 1d/
└ 2d/
  └ advection/
      └ elixir_basic.jl
      └ elixir_amr.jl
  └ euler/
      └ elixir_kelvin_helmholtz_instability.jl
  └ mhd/
└ 3d/

@efaulhaber
Copy link
Member Author

The main thing that annoyed me was searching for structured/curved elixirs in the bulk of Euler tests.

So, if we create a folder for each equation, I'd suggest file names like elixir_tree_basic.jl, elixir_curved_basic.jl, etc.
This way, tree, curved and maybe coupled elixirs are bundled together.

Otherwise, I'd suggest creating a folder for each mesh type and keeping the file names as they are.

@sloede
Copy link
Member

sloede commented May 8, 2021

Hah, and so the division on opinions begins!

@sloede
Copy link
Member

sloede commented May 15, 2021

Let's try to think about some (likely to be) common scenarios. For example:

I am a new user and would like to simulate a particular problem with Trixi. I am most likely set on the system of equations I want to use, but I probably do not care such much about which particular simulation approach I use (e.g., DG or FV). However, if I have at least a working knowledge of mesh types, I probably know whether my problem is defined on a box, or requires AMR, or even high-order curved boundaries.

For this user, it would probably be most helpful if they could browse the existing elixirs by dim -> meshtype -> elixirs and see what kind of equations are already available and/or implemented for their particular favorite solver.

Another scenario:

I am a Trixi developer and I am looking for a suitable elixir to extend a feature of a particular mesh-solver combination. I don't even care about the particular set of equations, so I'm likely going to start with scalar advection and, once that works, work my way up to compressible Euler.

This user likely does not care much either way, but since we do not have truly different solvers at the moment, they probably best served by browsing by dim -> meshtype -> elixirs.

Thus, it's 2:0 for @erik-f 's suggestion to go by mesh instead of equations. However, I still feel like dim should go first, since this is likely to be the one that is the least "variable" for a user. Also, it gives them the possibility to immediately grasp which alternative mesh types they have if their favorite mesh is not available yet.

Now, are there other typical workflows that I missed that could tilt this in a different direction?

@ranocha
Copy link
Member

ranocha commented May 16, 2021

Well, if I were your first user, I would know that I want to simulation the MHD equations but I don't really care about the mesh - it should just work, that's what I am interested in.

But I'm fine either way - we should discuss it at the next Trixi meeting and just pick whatever the majority of people likes.

@sloede
Copy link
Member

sloede commented May 25, 2021

After today's discussion, I put forward the following initial concrete proposal for renaming the directories:

examples/
├── README.md
├── paper-self-gravitating-gas-dynamics
│   ├── README.md
│   ├── elixir_euler_eoc_test.jl
│   ├── ...
│   └── elixir_hypdiff_eoc_test.jl
├── special_elixirs
│   └── elixir_euler_ad.jl
├── structured_1d
│   ├── elixir_advection_basic.jl
│   ├── ...
│   └── elixir_euler_source_terms.jl
├── structured_2d
│   ├── elixir_advection_basic.jl
│   ├── ...
│   └── elixir_euler_source_terms.jl
├── structured_3d
│   ├── elixir_advection_basic.jl
│   ├── ...
│   └── elixir_mhdmulti_es.jl
├── tree_2d
│   ├── elixir_advection_amr.jl
│   ├── ...
│   └── elixir_mhdmulti_rotor.jl
├── tree_3d
│   ├── elixir_advection_amr.jl
│   ├── ...
│   └── elixir_mhd_ec.jl
└── unstructured_2d
    ├── elixir_euler_basic.jl
    ├── ...
    └── elixir_euler_wall_bc.jl

Open questions:

  • Should we use _ instead of -, i.e., unstructured_2d vs unstructured-2d? The former would be more consistent with Trixi, I guess. EDIT Already changed as per @ranocha's suggestion.
  • Should we rename the "special" elixir folders special_elixirs and paper-... or move them to a new subfolder (for potentially increase clarity)?
  • I already renamed "unstructured quad" to unstructred-xd and "curved" to structured-xd, following a discussion we had some time before. Should we do this before we rename the mesh types themselves? (I think yes; the renaming of the mesh types is a larger change that will require more time I guess)

@ranocha
Copy link
Member

ranocha commented May 25, 2021

  • Should we use _ instead of -, i.e., unstructured_2d vs unstructured-2d? The former would be more consistent with Trixi, I guess.

I prefer using the same character _ here, too, Mr. Consistency

@ranocha
Copy link
Member

ranocha commented May 25, 2021

  • I already renamed "unstructured quad" to unstructred-xd and "curved" to structured-xd, following a discussion we had some time before. Should we do this before we rename the mesh types themselves? (I think yes; the renaming of the mesh types is a larger change that will require more time I guess)

Sounds reasonable to me.

@andrewwinters5000
Copy link
Member

  • I already renamed "unstructured quad" to unstructred-xd and "curved" to structured-xd, following a discussion we had some time before. Should we do this before we rename the mesh types themselves? (I think yes; the renaming of the mesh types is a larger change that will require more time I guess)

I agree with this name change as well. It leaves room for the natural growth of the unstructured solver to 3d instead of having a new mesh type called UnstructuredHexMesh, although Trixi the witch would be interested in hexes ;) .

This naming convention would also allow the triangular mesh elixirs to live in the unstructred-xd folder.

@ranocha
Copy link
Member

ranocha commented May 28, 2021

We should also include the basic solver type if we want to extend Trixi, e.g.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants