Extract boundary entities + getfaceedges helper #606
Conversation
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #606 +/- ##
==========================================
- Coverage 92.90% 92.85% -0.05%
==========================================
Files 30 30
Lines 4324 4378 +54
==========================================
+ Hits 4017 4065 +48
- Misses 307 313 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I think the PR really looks great! Thanks for your work. I have a last minor thing to fix (i.e. test should always be inside testsets to not leak any information) and if the CI does not fail, then things are fine from my side.
cc @fredrikekre
There was a problem hiding this comment.
Sorry, I totally missed that tests for 3D are missing in the PR. Did we just discuss it in the comments/slack? Anyway, can you provide the test coverage for 3D grids? (Prism, Tet, Hex) I think we really can need the helpers in this PR for a full automated consistency check of interpolation implementations. :)
I also have a quick question on one of the tests.
src/Grid/grid.jl
Outdated
| function getinstances(grid::AbstractGrid, topology::ExclusiveTopology, entity::T, _ftype::Function, _indextype::Type) where T <: BoundaryIndex | ||
| _set = Set{_indextype}() | ||
| cells = getcells(grid) | ||
| cell = cells[entity[1]] | ||
| verts = _ftype(cell)[entity[2]] #_fparent returns global vertices ids of face/edge. _fentities transforms them to pairs for edges. | ||
| for cell_idx in topology.vertex_to_cell[verts[1]] #∩([topology.vertex_to_cell[vert] for vert in verts]...) took couple seconds more in testing than topology.vertex_to_cell[verts[1]] | ||
| entity_idx = findfirst(_x-> all( [x ∈ verts for x in _x]),_ftype(cells[cell_idx])) | ||
| !isnothing(entity_idx)&& push!(_set, _indextype(cell_idx, entity_idx)) | ||
| end | ||
| return _set | ||
| end |
There was a problem hiding this comment.
We might be able to speed these functions up with code generators instead of passing the types and functions by hand. See e.g. https://github.com/Ferrite-FEM/Ferrite.jl/blob/master/src/Grid/grid.jl#L948-L961 . If it is too difficult to setup, then feel free to skip this comment. :)
fredrikekre
force-pushed
the
boundary-entities
branch
from
8d3009e
to
14284ce
Compare
|
I pushed some formatting changes and squashed. Also changed Thanks for the contribution, and sorry for the slow review! |
This patch adds `addboundaryfaceset!`, `addboundaryedgeset!`, `addboundaryvertexset!` for adding entities that are located on the boundary (defined as no cell neighbor). In addition, this adds internal functions for getting subentites from entities and various other helpers (`getfaceedges`, `getfacevertices`, `getedgevertices`, `getfaceinstances`, `getedgeinstances`, `getvertexinstances`, `filterfaces`, `filteredges`, `filtervertices`). Fixes Ferrite-FEM#595.
fredrikekre
force-pushed
the
boundary-entities
branch
from
14284ce
to
b71f0d0
Compare
fixes #595 (
addboundaryfaceset!,addboundaryedgeset!,addboundaryvertexset!) and adds helper functions to get face/edge edges/vertices (getfaceedges,getfacevertices,getedgevertices).