Split unordered and ordered grid function space implementation
Summary
There are several issues with the current gfs-ordering combinations:
- The ordering part on the different nodes is basically a copy-paste.
- The deduction of the ordering type (tree transformation) is done with an incomplete type. That is because the tree transformation uses the
GFS
type to create the ordering that is placed in theGFS
(notice the circle dependency). - The grid function space has to hold a mutable ordering in order to delay its construction because it is unknown when the whole tree is complete. A side-effect of creating the mutable ordering is that the gfs data has to be
const_cast
ed for every node of the tree and shared with the ordering. Things like this are not only unintuitive but it also violates all the const-correctness we all like C++ for. - The way the gfs data is shared with the ordering, only allows one ordering to be linked to one gfs tree.
- Because of the strange relationship between gfs data and ordering, it is always unclear how to store the gfs.
- Since the update is now done in the ordered gfs class, this removes the need of the Barton–Nackman trick all together in the GFS base class and simplifies code greately.
What does this MR do?
Grid function spaces classes are split into two: Unordered GFS
s and Ordered GFS
s. In few words, this is a clean up of the grid function space interface. In detail:
- Remove duplicated code for creating ordering: "Do not repeat yourself"
😉 - This change enforces a clear separation between the root node (
OrderedGFS
) and intermediate space representations (UnorderedGFS
). This is very similar todune-functions
where pre-basis are used to build the final basis tree. - When using Unordered
GFS
s to declare space trees, the ordering type is not deduced. This potentially saves a lot of compile-time on complex trees since that's not needed. - Make the root node hold an additional entity set for assembly (possibly with another type), and all intermediate unordered versions will not have an entity set anymore. This makes a more clear separation of concerns that the one introduced in !557 (merged). The intention is that this also works for multidomain grids where the leaf nodes have differents types than the ordered node. The only requirement is that leaf node entity sets accept entities from the assembly grid. This is already possible in
dune-multidomaingrid
. - The new ordered GFS will be able to create the ordering at construction time which is the expected behavior.
- The GFS will have now value semantics. In other words, they can be copied and they will still refer to the same data. This means that we don't have to rely on the ugly trick of
stackobject_to_shared_ptr
to construct the tree. Now we can just copy the contents into a shared pointer to makeTypeTree
happy. - This MR removes the extra layer of traits and base classes for power and composite nodes. Before, They were necessary to ensure that the traits type is complete on ordering instantiation, but that's not needed anymore. Now, gfs nodes are very very simple.
Backwards compatibility
The gfs can be used exactly as before. We do this by inheriting from the ordered grid function space and constructing such base class "in place". But it comes with few caveats:
- The gfs was allowed to be copied under special conditions and have several nodes with orderings active at the same time (See !559 (diffs)). This was never the intent and is a clear misuse. The new behavior will correctly catch this an throw a
GridFunctionSpaceHierarchyError
. - Power and composite GFS constructors forward their arguments to the base class (!559 (diffs) and !559 (diffs)). This means that initialization lists are not allowed anymore because templates cannot deduce them (See !559 (diffs)). This could be fixed by making a verbatim copy of the constructor for ordered and unordered versions, the question is if we want that. I think that this is only relevant for permuted and chunked orderings.
- Ordered GFS nodes do not inherit from
GridFunctionSpaceOrderingData
anymore, but this was an implementation detail that no one out there should be using. - The method
setEntitySet
is removed in favor of initialization at construct time in the ordered GFS. But this interface has not been released so it should not be a problem. - Because
GFS
nodes don't necessarily haveOrdering
, theDOFAccessor
is not available anymore in the local function space. This breaks theSingleCodimMapper
mapper support. However, I proved in #179 that this is quite broken anyways. - And, algorithms now shall not assume that intermediate nodes hold an entity set nor an ordering:
- The local function space should deprecate the entity set traits. Instead, entity binds use a template that is forwarded to the entity set.
- The sub-space gfs won't work with unordered gfs because it assumes the existence of entity sets on every node. Thus this will need to be refactored to make it work. The refactor will be needed in order to use
HybridTreePath
s anyways. Nonetheless, it is still full backwards compatible with trees that have ordering on every node.
Can this MR be accepted?
-
Implemented -
Added/Updated tests -
Added/Updated documentation -
Pipelines passing -
Added entry to CHANGELOG.md
Related issues
Edited by Santiago Ospina De Los Ríos