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

Unify signatures of mesh constructors #1194

Open
ranocha opened this issue Aug 8, 2022 · 4 comments
Open

Unify signatures of mesh constructors #1194

ranocha opened this issue Aug 8, 2022 · 4 comments

Comments

@ranocha
Copy link
Member

ranocha commented Aug 8, 2022

It may be nice to unify the signatures of some mesh constructors or provide additional convenience constructors. This can be nice when testing different meshes, e.g., for #1191. Right now, we have

julia> TreeMesh(

TreeMesh(coordinates_min::Real, coordinates_max::Real; kwargs...) in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/tree_mesh.jl:161
TreeMesh(coordinates_min::Tuple{Vararg{Real, NDIMS}}, coordinates_max::Tuple{Vararg{Real, NDIMS}}; n_cells_max, periodicity, initial_refinement_level, refinement_patches, coarsening_patches) where NDIMS in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/tree_mesh.jl:93
TreeMesh(::Type{TreeType}, args...) where {NDIMS, TreeType<:Trixi.AbstractTree{NDIMS}} in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/tree_mesh.jl:80

julia> StructuredMesh(

StructuredMesh(cells_per_dimension, faces::Tuple; RealT, periodicity) in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/structured_mesh.jl:86
StructuredMesh(cells_per_dimension, mapping; RealT, periodicity, unsaved_changes, mapping_as_string) in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/structured_mesh.jl:47
StructuredMesh(cells_per_dimension, coordinates_min, coordinates_max; periodicity) in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/structured_mesh.jl:120

julia> P4estMesh(

P4estMesh(trees_per_dimension; polydeg, mapping, faces, coordinates_min, coordinates_max, RealT, initial_refinement_level, periodicity, unsaved_changes) in Trixi at /home/hendrik/.julia/dev/Trixi/src/meshes/p4est_mesh.jl:146

For example, it wold be nice to just replace TreeMesh by StructuredMesh or P4estMesh and get something that just works.

@ranocha ranocha mentioned this issue Dec 2, 2022
4 tasks
@ranocha ranocha mentioned this issue Dec 24, 2022
10 tasks
@jlchan
Copy link
Contributor

jlchan commented Apr 6, 2023

Bumping to add that we should do the same for DGMultiMesh constructors. For example, cells_per_dimension is a keyword argument here

    DGMultiMesh(dg::DGMulti; cells_per_dimension,
                coordinates_min=(-1.0, -1.0), coordinates_max=(1.0, 1.0),
                is_on_boundary=nothing,
                periodicity=ntuple(_ -> false, NDIMS))

but not for the StructuredMesh equivalent

    DGMultiMesh(dg::DGMulti{NDIMS}, cells_per_dimension, mapping;
                is_on_boundary=nothing,
                periodicity=ntuple(_ -> false, NDIMS), kwargs...) where {NDIMS}

@ranocha
Copy link
Member Author

ranocha commented Apr 6, 2023

Yes, it would definitely be great to unify these 👍

@DanielDoehring
Copy link
Contributor

I agree, I also do not like that for P4est meshes we explicitly supply the dimensionality as a type (not sure if this is the correct Julian term), i.e., P4estMesh{2} while for TreeMesh, StructuredMesh and DGMultiMesh the dimensionality is inferred from the arguments while again for the unstructured 2d mesh the constructor reads UnstructuredMesh2D.

@ranocha ranocha mentioned this issue Feb 23, 2024
2 tasks
@ranocha ranocha mentioned this issue Jul 1, 2024
10 tasks
@ranocha ranocha mentioned this issue Oct 10, 2024
5 tasks
@DanielDoehring
Copy link
Contributor

So something I particularly dislike is the order of the coordinates and the argument that governs the discretization accuracy, i.e., initial_refinement_level, cells_per_dimension, trees_per_dimension which is different for the TreeMesh vs Structured/P4estMesh.
Also with #2129 around the corner we should add the RealT keyword for the TreeMesh. Might be the way to get #2129 itself non-breaking.

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

No branches or pull requests

3 participants