On Fri, Feb 03, 2017 at 04:52:20PM -0600, Barry Smith wrote:
I am confused about the organization of DMFOREST, and think it could be refactored to be clearer.
It seems DMFOREST is like an abstract based class in that it can't actually do anything itself to manage a forest. Meanwhile DMP4EST and DMP8EST provide implementations of DMFOREST for 2 and 3 dimensions. There is this weird shit DMForestRegisterType(DMType); and DMIsForest(DM,PetscBool*); to try to manage the fact that though DMP4EST and DMP8EST are conceptually subclasses of DMFOREST they are actually implemented as subclasses of DM.
Question: Will p4est and p8est be the only implementations of DMFOREST or is there a thought there may be other implementations of DMFOREST in the future (be realistic)? Note that p4est and p8est are really a single implementation for different dimensions.
If the answer is: they will be the only implementations of DMFOREST then I think the code should be refactored so DMP4EST and DMP8EST as well as DMForestRegisterType(DMType); and DMIsForest() don't exist and then setup for DMFOREST directly sets the function pointers depending on the dimension to p4est or p8est functions. Much clearer conceptually.
If the answer is: there will/should be more implementations for DMFOREST then I think the code should be refactored so that there exists a DMFORESTType, DMFORESTRegister(), DMFORESTP4EST, DMFORESTP8EST etc so that these implementations are conceptually and in the code a subclass (subtype) of DMFOREST. This would be clearer for users than the strange hybrid DMP4EST is a subclass of DMFOREST but isn't really that is currently implemented.
In both cases the actually code changes that need to be made are relatively small.
Am I totally missing something in my understanding? Is there a reason for this half-plant/half-animal implementation?
I don't think you missed anything. I wrote DMFOREST with the intent of supporting other implementations. I know of at least one more project in the works that would be a good candidate for DMFOREST, but that doesn't mean it will get done. I think you're right that if that's the case then p4est and p8est should properly subclass. But in that case, it seems strange to me that DMFOREST implements so little of the core dm ops for its descendants. To me DMFOREST feels more like an interface than a base class. Toby
Barry