[petsc-dev] use of deprecated DMSetFunction() approach

[Barry Smith]

On Nov 27, 2012, at 7:53 AM, Matthew Knepley <knepley at gmail.com> wrote:

> On Tue, Nov 27, 2012 at 7:30 AM, Barry Smith <bsmith at mcs.anl.gov> wrote:
>> 
>>   Matt,
>> 
>>     I have updated your dmcomplex examples to use DMSNESSetFunction/JacobianLocal() instead of DMSetLocalFunction/Jacobian() which I have removed.
> 
> Did you run the tests? :)

   I ran 3 of the four examples using a test in builder.py they produced the same results before and after. The fourth example didn't compile (was out-dated) hence I did not run a test.

  Barry

> 
>   Matt
> 
>>   Barry
>> 
>> On Nov 25, 2012, at 4:16 AM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
>> 
>>> On Sun, Nov 25, 2012 at 5:20 AM, Barry Smith <bsmith at mcs.anl.gov> wrote:
>>> 
>>>   Matt,
>>> 
>>>     The use of DMSetFunction/Jacobian was deprecated many months ago when KSPDM, SNESDM, and TSDM were introduced but you seem to be merrily using it to build a complete DMSetLocalFunction() infrastructure?   I already gave Jed and Peter a serious tongue lashing for "providing backward compatible support for DMSetFunction()" and not completely stripping it out when they wrote the replacement, as the PETSc style guide requires they should have done.
>>> 
>>> 
>>>   Everyone,
>>> 
>>>   But now what are we going to do? We need to support
>>> 
>>> 1) the usual "global" SNES/TS/KSP Set Function/Jacobian
>>> 
>>> 2) special DM specific function/Jacobian evaluations such as
>>> 
>>>     a)  finite elment style SNES/TS/KSP Set Local (ghosted) Function/Jacobin
>>> 
>>>      b) DA oriented set local function/Jacobian
>>> 
>>> 
>>> The KSPDM/SNESDM/TSDM model seems ok for managing 1)  but how do we plan to manage all the 2)?
>>> 
>>> DMDASNESSetFunctionLocal() and similar. To use those routines, you must "know about DM", thus there is no hardship in setting your callbacks on the DM.
>>> 
>>> Note that SNESSetFunction() trivially redirects into DMSNESSetFunction() so it is doing nothing more than hiding DM from users that are not interested in using it.
>>> 
>>> The comments in the code
>>> 
>>>  /* This context/destroy pair allows implementation-specific routines such as DMDA local functions. */
>>>  PetscErrorCode (*destroy)(KSPDM);
>>>  void *data;
>>> 
>>> A duplicate routine should be here, but nobody has written DMDAKSPSetComputeOperators() so it wasn't being used.
>>> 
>>> https://bitbucket.org/petsc/petsc-dev/changeset/026daea85c5c24fc64f04357af2f13afc6b23697
>>> 
>>> 
>>>  /* This context/destroy pair allows implementation-specific routines such as DMDA local functions. */
>>>  PetscErrorCode (*destroy)(SNESDM);
>>>  PetscErrorCode (*duplicate)(SNESDM,DM);
>>>  void *data;
>>> 
>>>  /* This context/destroy pair allows implementation-specific routines such as DMDA local functions. */
>>>  PetscErrorCode (*destroy)(TSDM);
>>>  void *data;
>>> 
>>> Duplicate was needed here.
>>> 
>>> https://bitbucket.org/petsc/petsc-dev/changeset/d5619197e506fdb0622ba24b0e3c26ddbee596aa
>>> 
>>> 
>>> seem to indicate someone has thought about his but how the f it is planned to be done is unclear (and why SNES requires a duplicate but KSP and TS do not?).  In particular what is the user interface would it be
>>> 
>>> SNESDMDASetLocalFunction/Jacobian()?  or DMDASNESSetLocalFunction/Jacobian()?
>>> 
>>> DMDASNESSetFunctionLocal(), it has been there since March.
>>> 
>>> 
>>> I am also bothered by the more fundamental question of what is the expected user interface when there exists both
>>> 
>>> SNESSetFunction()   and DMSNESSetFunction()?
>>> 
>>> Are users suppose to either of them or just SNESSetFunction()? If just SNESSetFunction() then why is the level of DMSNESSetFunction() just advanced and not developer.   Having both of these is a major recipe for complete confusion for both users and developers.
>>> 
>>> My expectation is that 90% of users of DM will be able to use the impl-specific local routines like DMDASNESSetFunctionLocal(). For the few that need something more general (e.g., multiple communication phases in residual evaluation), they could use either. DMSNESSetFunction() is slightly more powerful than SNESSetFunction() because it can be set independent of the SNES, but I think that is rarely important.
>>> 
>>> SNESSetFunction() is purely cosmetic, allowing people to use SNES without needing to know about the DM concept. We can delete it if "more than one way to do it" is worse that "yet another object to interact with", but I'm doubtful.
>>> 
>>> 
>>> Also all the half-assed legacy support crap that has gotten in there makes the code incredibly fragile and is harder to get rid of then it should be.
>>> 
>>> We need to pick a single consistent extensible model now and change everything to match that model, the current code makes us look like a bunch of Trilinos developers.
>>> 
>>>   Barry
>> 
> 
> 
> 
> --
> What most experimenters take for granted before they begin their
> experiments is infinitely more interesting than any results to which
> their experiments lead.
> -- Norbert Wiener