[petsc-dev] TS Terminology

[Jed Brown]

Brad Aagaard <baagaard at usgs.gov> writes:

> On 10/21/17 7:00 AM, Jed Brown wrote:
>> Matthew Knepley <knepley at gmail.com> writes:
>>
>>> On Sat, Oct 21, 2017 at 9:28 AM, Jed Brown <jed at jedbrown.org> wrote:
>>>
>>>> Matthew Knepley <knepley at gmail.com> writes:
>>>>
>>>>> On Fri, Oct 20, 2017 at 4:21 PM, Jed Brown <jed at jedbrown.org> wrote:
>>>>>
>>>>>> Matthew Knepley <knepley at gmail.com> writes:
>>>>>>
>>>>>>>> Note that TSComputeIFunction is very much like SNESComputeFunction,
>>>>>>>> which includes
>>>>>>>>
>>>>>>>>   if (snes->vec_rhs) {
>>>>>>>>     ierr = VecAXPY(y,-1.0,snes->vec_rhs);CHKERRQ(ierr);
>>>>>>>>   }
>>>>>>>>
>>>>>>>> Why haven't you complained about that?
>>>>>>>>
>>>>>>>
>>>>>>> Good point. I did not notice. This came up because the initialization
>>>>>>> of input vectors is inconsistent between TSComputeIFunction() and
>>>>>>> TSComputeIFunctionLocal(). The former does not zero the output vec,
>>>>>>> but the later does.
>>>>>>
>>>>>> The latter function doesn't exist so maybe you mean
>>>>>> TSComputeIFunction_DMDA with ADD_VALUES?  That's because the DMDA needs
>>>>>> it when using ADD_VALUES, just like SNESComputeFunction_DMDA.  When
>>>>>> using INSERT_VALUES, the user is responsible for setting every entry.
>>>>>> Is any of this different from SNES?
>>>>>>
>>>>>
>>>>> No I mean TSComputeIFunction_DMLocal().
>>>>
>>>> That's just some code you wrote for finite elements when you hard-coded
>>>> it for ADD_VALUES.
>>>>
>>>>   ierr = DMLocalToGlobalBegin(dm, locF, ADD_VALUES, F);CHKERRQ(ierr);
>>>>   ierr = DMLocalToGlobalEnd(dm, locF, ADD_VALUES, F);CHKERRQ(ierr);
>>>>
>>>> It's analogous to what TSComputeIFunction_DMDA does when you set
>>>> ADD_VALUES.  Note that with INSERT_VALUES, as is typically used for FD
>>>> and FV, you can assemble directly into the global vector
>>>>
>>>>> So if you use DMTSSetIFunction() you get different initialization
>>>>> behavior than if you use DMTSSetIFunctionLocal().  This is what Brad
>>>>> was complaining about originally.
>>>>
>>>> Maybe you should document your finite element interfaces better.
>>>>
>>>
>>> I think the fair statement would be that it is not documented either way.
>>
>> Yes, it's EXACTLY the same as how SNESComputeFunction does not zero the
>> vector, but SNESComputeFunction_DMLocal does because it's a finite
>> element/ADD_VALUES interface.  You could start by documenting that.
>
>  From the perspective of a PETSc user, I think 
> TSComputeFunctionWhatever() should zero out the global function vector 
> before calling the user ifunction and user rhsfunction. 

What about SNESComputeFunction?  Why wasn't this a source of confusion
there?

What about MatShell and PCShell callbacks?  What about matrices?

There is no semantic harm in zeroing the vector, but it's an extra pass
over memory that isn't required when a user is inserting (rather than
adding) values.  It's a little bit of boilerplate to zero when needed,
but this is how PETSc has worked since the 90s.

> The top-level function knows when to zero out the function vector; the
> user doesn't unless they look at the details of the calling
> function. Additionally, the user code should not have to change if the
> top level calling function decides to reuse the global function vector
> rather than using a temporary. This way, it doesn't matter if the user
> inserts values into the global vector using ADD_VALUES or
> INSERT_VALUES. With the current implementation the user must zero out
> of the global vector in both the user ifunction and the user
> rhsfunction in assembling with ADD_VALUES.  If someone changes the
> ComputeIFunctionWhatever() to reuse the same global vector in both
> calls, it would break the user code because intermediate values in the
> global function vector would get zeroed out.  It seems obvious to me
> that the most robust approach is to zero out the global function
> vector in the top level code (TSComputeFunctionWhatever) so that the
> user code can be written to work with INSERT_VALUES or ADD_VALUES,
> doesn't care about the calling order of the user ifunction and user
> rhsfunction, and works with either temporary global function vectors
> being passed in or the same global function vector.

Wait, you're saying that user code should NEVER zero the output vector?
But if they are inserting (rather than adding) values, they will still
overwrite what was there.  The caller cannot use the same vector and
expect it to be added.  I'm pretty sure we never do that in PETSc and
the documentation never indicates that it would be acceptable.

> ... and, yes, it would be nice if the state of the global vectors passed 
> to the user were documented or at least assumptions about what the user 
> needs to do are clearly documented in the API.
>
> Brad