-
Notifications
You must be signed in to change notification settings - Fork 94
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
Rework the AbstractCell
interface
#679
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 #679 +/- ##
==========================================
- Coverage 93.44% 92.88% -0.57%
==========================================
Files 29 30 +1
Lines 4259 4326 +67
==========================================
+ Hits 3980 4018 +38
- Misses 279 308 +29
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. |
Is there a way to ensure that |
Supersedes the prototype given in #519 ? |
That can be encoded in the type system: julia> abstract type AbstractRefShape{dim} end
julia> struct RefInterval <: AbstractRefShape{1} end
julia> struct RefTriangle <: AbstractRefShape{2} end
julia> abstract type AbstractCell{refdim, refshape <: AbstractRefShape{refdim}} end
julia> struct Nope <: AbstractCell{2, RefInterval} end
ERROR: TypeError: in AbstractCell, in refshape, expected refshape<:AbstractRefShape{2}, got Type{RefInterval}
Stacktrace:
[1] top-level scope
@ REPL[5]:1
julia> struct Yep <: AbstractCell{2, RefTriangle} end
Yea, this roughly corresponds to the Grid changes in that PR. |
16d3ba3
to
c0be534
Compare
This patch reworks the `AbstractCell` interface (refer to #677 for motivation and discussion for alternatives to this patch). Specifically, this patch - introduces the reference dimension as a type parameter on `AbstractRefShape`, - introduces `RefLine`, `RefTriangle`, `RefQuadrilateral`, and `RefHexahedron` as new subtypes of `AbstractRefShape{refdim}` (note that interpolations are not updated to this system yet since it can be done in a separate patch), - deprecates the old `Cell{refdim, nnodes, nfaces}` struct, - introduces new individual structs for all supported cell types, e.g. `struct Triangle <: AbstractCell{2, RefTriangle}`, - defines the unique vertex/edge/face identifiers for DoF-distribution in terms of dispatch on the reference shape. The new parametrization makes it possible to dispatch on the reference shape instead of union of concrete implementations, i.e. `::AbstractCell{2, RefTriangle}` instead of `::Union{Triangle, QuadraticTriangle}`, and similarly for the reference dimension. One drawback is that it is no longer possible to use "anonymous celltypes" after this patch, i.e. using, e.g., `Cell{3, 20, 6}` and defining methods for that instead of explicitly naming it `SerendipityQuadraticHexahedron` or similar. Instead, you now have to define a struct and some methods. Arguably this is a good thing -- nobody understands the meaning of `Cell{3, 20, 6}`. For normal usage this patch is not breaking (see e.g. no required changes for examples), unless one dispatches on the `Cell` struct directly. Fixes #677.
c0be534
to
74f6b14
Compare
This patch reworks the
AbstractCell
interface (refer to #677 for motivation and discussion for alternatives to this patch). Specifically, this patchAbstractRefShape
,RefInterval
,RefTriangle
,RefQuadrilateral
, andRefHexahedron
as new subtypes ofAbstractRefShape{refdim}
(note that interpolations are not updated to this system yet since it can be done in a separate patch),Cell{refdim, nnodes, nfaces}
struct,struct Triangle <: AbstractCell{RefTriangle}
,The new parametrization makes it possible to dispatch on the reference shape instead of union of concrete implementations, i.e.
::AbstractCell{RefTriangle}
instead of::Union{Triangle, QuadraticTriangle}
, and similarly for the reference dimension.One drawback is that it is no longer possible to use "anonymous celltypes" after this patch, i.e. using, e.g.,
Cell{3, 20, 6}
and defining methods for that instead of explicitly naming itSerendipityQuadraticHexahedron
or similar. Instead, you now have to define a struct and some methods. Arguably this is a good thing -- nobody understands the meaning ofCell{3, 20, 6}
.For normal usage this patch is not breaking (see e.g. no required changes for examples), unless one dispatches on the
Cell
struct directly.Specific things that remain to be decided:
struct Triangle <: AbstractCell{2, RefTriangle}
orstruct Triangle <: AbstractRefCell{2, RefTriangle} <: AbstractCell
? I don't know if it is too restrictive to "force"{refdim, refshape}
on all subtypes ofAbstractCell
. On the other hand, if one implements a non-refshape based element one can define a dummy reference shape. For a real-world example, looking at e.g. https://github.com/kimauth/FerriteCohesiveZones.jl/blob/main/src/cohesive_cell.jl I think that would simply be implemented asjulia struct CohesiveQuadrilateral <: AbstractCell{2, RefQuadrilateral} nodes::NTuple{4, Int} end
which I don't see a direct problem with, other than the fact that the cell will now have 4 edges (according to the fallback definitions). To solve this, one would have to implement a method forfaces(::CohesiveQuadrilateral)
that only return the upper and lower edge. I don't think this is too much to ask for custom elements like this. Alternatively, with Wedge Support #581 merged it is possible to distribute different number of dofs for different edges, so this can also be solved on the (default) interpolation level by distributing 0 dofs on the left and right edge.AbstractCell{refdim, refshape}
or justAbstractCell{refshape}
? The dimension is implicit from the shape now, and you can dispatch onAbstractCell{<:AbstractRefShape{refdim}} where refdim
. Personally I am in favor of including the dimension, even if redundant. It does not cost anything, and sometimes you want to dispatch on the dimension, sometimes on the refshape.Fixes #677.