-
Couldn't load subscription status.
- Fork 131
Default BC functions for P4est , Tree and Structured meshes #2515
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
base: main
Are you sure you want to change the base?
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Would it maybe be a cleaner interface to have only one function boundary_condition_default(mesh, boundary_condition) , which dispatches on the mesh type? Then you can use another mesh, but still get the correct default boundary condition. This goes into the direction of #1510.
Could you also use the new functions in at least one of the elixirs each, such that they are tested?
|
Thanks for your contribution! I like the convenience, but maybe we start with the non-tree mesh types since for the latter (as you mentioned) this functionality actually already exists Trixi.jl/examples/tree_2d_dgsem/elixir_advection_amr_nonperiodic.jl Lines 12 to 26 in 3ce2033
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2515 +/- ##
=======================================
Coverage 96.81% 96.81%
=======================================
Files 535 536 +1
Lines 42813 42829 +16
=======================================
+ Hits 41446 41462 +16
Misses 1367 1367
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
Co-authored-by: Joshua Lampert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more formatting comments below (you can also go to the "Files changed" tab and add suggestions to a batch, such that multiple suggestions can be bundled in one commit to keep the commit history clean).
Co-authored-by: Joshua Lampert <[email protected]>
…ary_condition_default(mesh, boundary_condition)
…dition_default function
…ndition_default(mesh, boundary_condition) function instead of writing it in the sort_boundary_conditions.jl file
Thanks for the comments and the idea with the batch 👍 |
| if isa(mesh, TreeMesh) | ||
| ndims_mesh = ndims(mesh) | ||
| if ndims_mesh == 1 | ||
| return boundary_condition_default_tree_1D(boundary_condition) | ||
| elseif ndims_mesh == 2 | ||
| return boundary_condition_default_tree_2D(boundary_condition) | ||
| elseif ndims_mesh == 3 | ||
| return boundary_condition_default_tree_3D(boundary_condition) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please switch to a dispatch-based approach instead of having if statements based on the type? This makes it easier to extend the functionality in the future. For example, you would write
function boundary_condition_default(mesh::TreeMesh1D, boundary_condition)
return (x_neg = boundary_condition,
x_pos = boundary_condition)
endand remove the functions boundary_condition_default_tree_1D etc.
| # @eval due to @muladd | ||
| @eval Adapt.@adapt_structure(UnstructuredSortedBoundaryTypes) | ||
| end # @muladd | ||
| end#@muladd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| end#@muladd | |
| end # @muladd |
Please revert the whitespace change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! We are close to merging this PR.
Co-authored-by: Hendrik Ranocha <[email protected]>
… boundary_condition)` is used in the examples/tree_3d_dgsem folder
Changes
New boundary-condition functions added
boundary_condition_default_p4est_2Dboundary_condition_default_p4est_3Dboundary_condition_default_structured_1Dboundary_condition_default_structured_2Dboundary_condition_default_structured_3Dboundary_condition_default_tree_1Dboundary_condition_default_tree_2Dboundary_condition_default_tree_3DThese functions apply the same boundary condition on every face of the mesh for P4est, structured, and tree meshes.
Documentation updated
Trixi.jl/docs/src/meshes/p4est_mesh.mdTrixi.jl/docs/src/meshes/structured_mesh.mdTrixi.jl/docs/src/meshes/tree_mesh.md