Construction of CoordFunction in Backup-Restore for GeometryGrid
Summary
This MR extends the interface of BackupRestoreFacilities
for GeometryGrid
to allow the construction of the CoordinateFunction.
Details
Not all CoordinateFunctions passed to a GeometryGrid are detault-constructible. This complicates the implementation of backup-restore facilities, since one has to pass the CoordinateFunction in the constructor of GeometryGrid. This MR extends the interface of the restore()
method in BackupRestoreFacilities
to pass a second parameter, a functor that accepts a grid reference and returns an object of CoordinateFunction that can be passed to the constructor of GeometryGrid. With this creator the CoordinateFunction can then be constructed.
Discussion
In the comments of BackupRestoreFacilities<GeometryGrid>
it is noted that one should backup (and restore) the coordinate function as well. This is not implemented and might need a generalization of the BackupRestore
class to be specialized also for non-grid types. Then, the user has to provide a specialization for the CoordinateFunction in order to backup and restore it from file or stream. But, often the CoordinateFunction depends on the restored grid or its gridView (see also the examples in GeometryGrid) and thus restoring it from file alone could be complicated. The creator approach is one way to solve this problem, but there might be others.
Merge request reports
Activity
@simon.praetorius: I would stick with the concept by adding a BackupRestoreFacility for the CoordFunction. Then you would write:
BackupRestoreFacility< GeoGrid >::backup( ostream& out ) { BackupRestoreFacility< CoordFunction >::backup( coordFunction, out ); BackupRestoreFacility< HostGrid >::backup( geoGrid.hostGrid(), out ); }
and for restore
GeoGrid* BackupRestoreFacility< GeoGrid >::restore( istream& in ) { std::shared_ptr< CoordFunction > coordFct = BackupRestoreFacility< CoordFunction >::restore( in ); std::shared_ptr< HostGrid > hostGrd = BackupRestoreFacility< HostGrid >::restore( in ); return new GeoGrid( hostGrd, coordFct ); }
This way the user has to take care of the backup/restore implementation of CoordFunction and the interface stays the same. Does that work for you?
I don't think that this will work, although I thought about this as well. This problem already occurs in the example for
dune/grid/geometrygrid.hh
template <class GridView> class DeformationFunction : public Dune::DiscreteCoordFunction<double, dim, DeformationFunction<GridView>> { public: DeformationFunction(const GridView& gridView, const double* deformedPosition); //... }
We need at least a GridView to construct the
DeformationFunction
. while thedeformedPosition
could be stored in file thus backup-restore will work for the second argument.The standard
BackupRestoreFacility
does not establish a connection between the grid and the CoordFunction, so one can not restore theDeformationFunction
easily.With my approach, one could implement something like
std::vector<double> deformedPosition; // restore positions from file using CoordFunction = DeformationFunction<typename HostGrid::LeafGridView>; using GeoGrid = GeometryGrid<HostGrid, CoordFunction>; std::unique_ptr<GeoGrid> grid(BackupRestoreFacility<GeoGrid>::restore(filename, [&](auto const& grid) { return new CoordFunction(grid.leafGridView(), deformedPosition.data()); });
And there will not be a compiler-error in case the CoordFunction can not be default-constructed. (This was the reason why I have implemented this patch at first)
@simon.praetorius: The problem with your approach is, that you introduce a new restore method which is not interface conform. In that case I think it maybe better to let the user application do everything, since only there all the details are known, e.g.
... // backup everything to out BackupRestoreFacility< CoordFunction >::backup( coordFunction, out ); BackupRestoreFacility< HostGrid >::backup( geoGrid.hostGrid(), out ); // backup gridView if needed
Upon restore:
// restore, types and order are known auto coordFct = BackupRestoreFacility< CoordFunction >::restore( in ); auto hostGrd = BackupRestoreFacility< HostGrid >::restore( in ); // restore gridView if needed auto gridView = ... // create GeoGrid as usual GeoGrid grid ( gridView, coordFct );
This function can be implemented in this way just for GeoGrid and all other grids just forward this implementation to the facility.
I have implemented this in dune-fem including GeoGridPart which works in this way. So its possible with the interfaces that are available.
Also, its hard to judge, since the full background information is missing. But this topic can be put on the list for the next meeting.
OK, this is fine for me. My proposal was just an attempt to solve this problem. A generic solution is a bit harder to define.
I just needed a way to decide at compile-time whether I can use the backup-restore facilities or have to use my fallback solution. Currently, the
GeometryGrid
has a capability that says: yes, it can be used, but when I try to actually instantiate and call the restore function I get an error.This MR contains two parts:
- solve the problem of non-default-constructible CoordFunctions, by using tag dispatching
- Provide an extension for other CoordFunctions
I agree that 2. needs more discussion. Maybe for 1. I could modify the capability that it returns true only in case the CoordFunction is default-constructible. That would solve my original problem and is a more accurate capability value.
Ok, then I would say there is a bug in the GeometryGrid implementation, if you get yes and then the error.
Maybe you could look at how I implemented the backup/restore for the Gmsh boundary projections (same problem). Although this backup/restore for I/O is slightly easier because you know all types and usually this is done a the beginning of the program so all you can even assume that the same order of object creation is followed which makes things a lot easier. For the redistribution during loadBalance with the boundary projections that is not the case since one process does not know what kind of object the other process created. Then you need this kind of object registry etc and I think this should be avoid if possible.
In dune-fem there is an example for this backup/restore. I think adding GeometryGrid for a test case should be possible. Then it should be easier to figure out what the minimal (and or best) interface extension would be.
In the meantime I guess you can live with your patch.
Edited by Robert Kmentioned in merge request !340 (merged)
I have extracted the patch for the capabilities in a separate MR, to leave the discussion here untouched. See !340 (merged)
mentioned in commit ac8cba05