Proper codim and dim entities

[KnutAM]

As agreed with @lijas, this PR continues from his work in #789, but introduces facets on the user-facing side. See #789 for an extended discussion.

The defintions in this PR are

• face: Always 2d entity
• edge: Always 1d entity
• vertex: Always 0d entity
• facet: Always codim 1 entity relative the cell's reference dimension.

With this PR, all code with grids with a single reference dimension, the code changes from using face to using facet. FacetIndex and FacetValues are introduced (this also includes embedded elements).

FEValues

Renaming
FaceValues -> FacetValues
FaceIterator -> FacetIterator
FaceQuadratureRule -> FacetQuadratureRule

Grid sets

An important change is then that the Grid struct looses facesets and edgesets, which are replaced by facetsets. The removal is motivated by avoiding confusion between facetset and faceset, and for current examples facesets and edgesets are not needed. To not remove the options, the following functions are introduced

• create_vertexset, create_edgeset, create_faceset, and create_facetset: (::AbstractGrid, ::Function; all=true)
• create_boundaryvertexset, create_boundaryedgeset, create_boundaryfaceset, and create_boundaryfacetset: (::AbstractGrid, ::ExclusiveTopology, ::Function; all=true)

So that sets can be created outside the grid. (add*! functions are available for vertex and facet).

To avoid too many issues in dependent packages, addfaceset! automatically translates the given Set{FaceIndex} to Set{FacetIndex} and adds it, warning the users to change their code. Similar for creating a Grid and giving a faceset but not a facetset.

Topology

get_facet_facet_neighborhood is defined, which gives the neighborhood depending on the reference dimension of the grid. If the grid has cells with different reference dimensions, an ArgumentError is thrown, asking the users to access the vertex_vertex_neighbor, edge_edge_neighbor, or face_face_neighbor fields explicitly.

getneighborhood can be querried with FacetIndex only if all cells have the same reference dimension. If not, an ArgumentError is thrown, asking users to use VertexIndex, EdgeIndex, or FaceIndex explicitly.

Extract Boundary entities (#595)

For the code in #595, I struggled a bit to update it, and found it easier to rewrite using the boundaryfunction instead of @eval function generation. This also means that getfaceedges, getfacevertices, getedgevertices are not used, and these are only in devdocs. So I commented out these for now along with the tests. Let me know if these should be kept (@AbdAlazezAhmed / @termi-official)?

Breaking dependent packages

FerriteGmsh and FerriteMeshParser break with this PR. The computational_homogenization and porous media examples include the required method redefinitions that should be included there. In addition, these packages should update to not use FaceIndex. Once I get feedback on this PR, I'll add PRs to those for updating them. If desired, supporting both the old and new version will be possible.

This should close #901 and #899.
Not tested, but at minimum it simplifies solving #843 without adding new fields to ExclusiveTopology.

TODO

• Open PRs in FerriteGmsh.jl and FerriteMeshParser.jl
• Fix formatting: Proper codim and dim entities #914 (comment)
• Fix test coverage / delete unused methods
• Fix Proper codim and dim entities #914 (comment)
• Move definitions: Proper codim and dim entities #914 (comment)
• Fix issue with unordered dofs on faces, Proper codim and dim entities #914 (comment)

[codecov]

Codecov Report

Attention: Patch coverage is 97.17514% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 93.56%. Comparing base (9fd9d34) to head (a8b9fb8).

Files	Patch %	Lines
src/Dofs/ConstraintHandler.jl	95.48%	6 Missing ⚠️
src/deprecations.jl	81.81%	6 Missing ⚠️
src/FEValues/FacetValues.jl	94.11%	2 Missing ⚠️
src/Grid/grid.jl	97.26%	2 Missing ⚠️
src/iterators.jl	93.93%	2 Missing ⚠️
src/Dofs/DofHandler.jl	95.83%	1 Missing ⚠️
src/interpolations.jl	98.21%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #914      +/-   ##
==========================================
+ Coverage   93.30%   93.56%   +0.26%     
==========================================
  Files          36       36              
  Lines        5257     5272      +15     
==========================================
+ Hits         4905     4933      +28     
+ Misses        352      339      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[termi-official]

Topology

get_facet_facet_neighborhood is defined, which gives the neighborhood depending on the reference dimension of the grid. If the grid has cells with different reference dimensions, an ArgumentError is thrown, asking the users to access the vertex_vertex_neighbor, edge_edge_neighbor, or face_face_neighbor fields explicitly.

getneighborhood can be querried with FacetIndex only if all cells have the same reference dimension. If not, an ArgumentError is thrown, asking users to use VertexIndex, EdgeIndex, or FaceIndex explicitly.

That should fix the type stability issue.

Extract Boundary entities (#595)

For the code in #595, I struggled a bit to update it, and found it easier to rewrite using the boundaryfunction instead of @eval function generation. This also means that getfaceedges, getfacevertices, getedgevertices are not used, and these are only in devdocs. So I commented out these for now along with the tests. Let me know if these should be kept (@AbdAlazezAhmed / @termi-official)?

We use something similar in the p4est PR. However, we need to iterate over the tests as you said.

Breaking dependent packages

FerriteGmsh and FerriteMeshParser break with this PR. The computational_homogenization and porous media examples include the required method redefinitions that should be included there. In addition, these packages should update to not use FaceIndex. Once I get feedback on this PR, I'll add PRs to those for updating them. If desired, supporting both the old and new version will be possible.

FerriteViz and FerriteDistributed will also break. To fix the latter I will need more time. xref Ferrite-FEM/FerriteDistributed.jl#39

[KnutAM]

FerriteViz and FerriteDistributed will also break.

Thanks for pointing out. I hope that for at least FerriteDistributed this PR will make things simpler!

We use something similar in the p4est PR. However, we need to iterate over the tests as you said.

Thanks for the heads-up. I couldn't find that, could you point me to where?

[fredrikekre]

Is #789 supposed to be merged first?

[fredrikekre]

Squashed and wrote a commit message locally. Merged to master in 2ca4f94 (thought it could be useful to keep the commit history around here instead of first pushing the squashed-commit to this branch).

I know there are some more todos but this PR was already huge and touches a lot of code so the risk of merge conflicts was pretty big.

Thanks @lijas for pushing for this initially and @KnutAM for finishing the PR!

[KnutAM]

Thanks for the quick review and actions!

could be useful to keep the commit history around

Good idea!

I know there are some more todos but this PR was already huge and touches a lot of code so the risk of merge conflicts was pretty big.

Agreed, I'll open separate (smaller) prs for those!