[petsc-dev] confused about DMFOREST

[Barry Smith]

> On Feb 5, 2017, at 11:22 PM, Tobin Isaac <tisaac at uchicago.edu> wrote:
> 
> 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.

   Ok, then eventually we should refactor so that p4est and p8est are DMForestTypes instead of DM types, but no huge hurry (definitely before you add another implementation :-).

>  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.

   Your DMFOREST is then an abstract base class (i.e. an interface :-). Regardless of which way you look at it I think DMForestTypes is a much clearly way to organize it rather than having a bunch of DMxxx that support the DMFOREST interface. 

   This is why I insist on naming conventions that reflect the hierarchies in PETSc: for example KSPGMRESSetReset() tells people that this affects the GMRES subclass of KSP. So DMFORESTP4EST tells the user clearly that P4EST supplies everything that DM and DMFOREST provide, while calling it DMP4EST people won't immediately see that p4est actually also supports the DMFOREST interface; hence why I don't like the current model. Whenever a clear name can save someone looking at documentation that is a very good thing :-)



> 
>  Toby
> 
>> 
>>   Barry