Remove genericgeometry subdirectory

There are plans to remove the genericgeometry subdirectory, voiced here in dune-common#29 (closed). This bug is for discussing them.

mentioned in issue dune-common#29 (closed)

Assigned to @martin.nolte, since he seems to be the one wanting to do the removal.

@martin.nolte: Another question regarding the topologytypes.hh-stuff: is that going to be integrated into topologyfactory.hh? Because topologytypes.hh is occasionally useful in some TMPs, and I can't quite tell what exactly topologyfactory.hh is good for from glancing over the comments in it (so I can't quickly tell how much of topologytypes.hh it needs etc.)

It was decided on the last developer meeting to remove this subdirectory and - being the one who created it - I volunteered to remove it. This is quite a piece of work and not all details in this directory can be removed completely. For example, some parts of genericgeometry/topologytypes.hh will have to go into type.hh.

Ah, OK. Thanks for the explanation of the current status.

mentioned in merge request !4

@oliver.sander: You requested the status in !4. As far as I remember, @carsten.graeser asked me to wait until he removed some uses of implementation details of the generic geometry from his code.

@carsten.graeser: What's the status? Can we go for it within the next month?

I don't remember this. But when searching I found two places where matrixhelper.hh was used. More precisely the pseudo-inverse rightInvA() and the function sqrtDetAAT().

It would be nice to collect such functionality somewhere in the core modules. In fact we also have a collection of matrix-vector routines in dune-fufem and dune-solvers.

So this is mainly held back by !21 (closed), no?

That said, not only does dune/grid/geometrygrid/geometry.hh use MatrixHelper, it uses GenericGeometry::DuneCoordTraits< ctype > as the template parameter, which lives in dune/geometry/genericgeometry/geometrytraits.hh...

Well, I'm still waiting for the 'go'. Then I'll try to schedule this in, but it has quite low priority to me. After all, it is just cleanup.

There are some more details that have to be moved to other places. This might require discussion (or autocracy on my side). Making this a show stopper for a potential 2.5 release could delay it for another year.

Of course, we could move all important stuff to a new subdirectory (e.g., implementation, detail, utility, ... - see !21 (closed)). But that boils down to renaming 'genericgeometry' to 'implementation'. I don't think this is the intention of this task, is it?

@oliver.sander: You requested this clean up. What is your schedule: 2.5 (in case it will happen) or 3.0?

I have no schedule, it all depends on when you (and the others involved) find the time to do it. As you said: after all it is just cleanup.

Maybe there are a few uncontroversial things in genericgeometry that can be removed before 2.5?

@oliver.sander: Sure, some clean up will not break code (unless someone uses these internals).

So, can I remove everything except the MatrixHelper and implementation details used by the core? Those details used by the core might be inlined.

I would say: yes, do it.

I second, please do it. Concerning Dune 2.5; I received the impression that the unplanned 2.5 release is neither immanent nor decided. I suggest to open an according issue and raise a vote to get the mood of the group.

What's the status for removing the directory?

@martin.nolte will this be part of 2.5? We should collect the open issues (by marking the milestone)

@christi: I will try to get this done by the end of the week.

great, in this case I mark it 2.5, so that we don't forget to merge it.

Milestone changed to %DUNE 2.5.0

!34 (merged)

Done. merge request mentioned by @marco.agnese now completely removes the genericgeometry subdirectory.

I tried to delete as much from the namespace GenericGeometry as I could. Those classes and functions that are still required were migrated either into dune/geometry/type.hh or dune/geometry/referencelements.hh. Library code went into dune/geometry/referenceelements.cc. As we decided to use the Impl namespace for implementation details, all remaining content of the GenericGeometry namespace was migrated there.

Unfortunately, these changes break some code relying on the implementation details. For this reason, there are corresponding merge requests in dune-grid and dune-localfunctions providing the necessary minor changes.

Note: I am not perfectly happy to do these quite invasive changes shortly before a release.

I wish us all some happy (but fruitful) discussions !

@martin.nolte maybe remove the WIP prefix from the MR if your the set of changes is completed

Status changed to closed

Status changed to reopened

Now that anything from GenericGeometry was marked as implementation detail, we have another problem: Topology types and topology ids are still part of the interface:

• GeometryType can be constructed from a topology type. The documentation now explicitly refers to Impl. You could also implement your own TopologyType providing the id. But unfortunately the constructor using the id does not describe what this is about but refers to the one taking a TopologyType
• StaticRefinement also needs a topology id.

In view of the above we should move the topology types from Impl to Dune.

Mentioned in merge request !38 (merged)

dune-grid now also relies on implementation details of dune-geometry (dune-grid!120 (merged)). This isn't nice: modules shouldn't use implementation details of other modules, otherwise they are no longer (internal) implementation details, but part of the interface between modules.

With the subdirectory gone, I suggest to close this issue and open new issues for follow-up discussions.

Agreed!

closed