[petsc-dev] PetscViewerASCIIPushSynchronized

[Smith, Barry F.]

> On May 2, 2019, at 6:15 PM, Jed Brown <jed at jedbrown.org> wrote:
> 
> I think this is a consequence of PetscViewerFlush_ASCII flushing any
> synchronized messages (code is basically copied from
> PetscSynchronizedFlush).
> 
>  if (vascii->allowsynchronized) {
>    PetscMPIInt   tag,i,j,n = 0,dummy = 0;
>    char          *message;
>    MPI_Status    status;
> 
>    ierr = PetscCommDuplicate(comm,&comm,&tag);CHKERRQ(ierr);
> 
>    /* First processor waits for messages from all other processors */
>    if (!rank) {
> 
> 
> But if all ranks don't agree about whether synchronized messages may
> exist, then this slow code path would always be taken.  In the current
> code, PetscViewerASCIIPopSynchronized doesn't do anything other than
> tracking that the pushes were matched.  It should probably flush

   What about the push? Does it need to do a flush also?  Note that the queued up messages are not tagged by the synchronized count so with 
multiple pushes there can be jumbled mess. 

> (or
> provide equivalent semantic on ordering).

Maybe the whole chunk of code

 if (vascii->allowsynchronized) {
   PetscMPIInt   tag,i,j,n = 0,dummy = 0;
    ....

could just be moved to the Pop. Or even better if possible have it call PetscSynchronizedFlush()  and
avoid the duplicate code. I'm not sure why I duplicated the code  but there must have been a reason.

The current code is a bit defective. Maybe the push/pop model should be abandoned and instead require that Allow be closed before another one is open. So PetscViewerASCIIOpenSynchronized(), synchronized writes, PetscViewerASCIICloseSynchronized() and the close automatically flushes. I'm not sure if the current routines are used in a nested manner (nor how to search for that ;)).

After a few hours I'm becoming more convinced that the reason for the Allow/Push/Pop model is just to avoid the global synchronization.

 
> 
> "Smith, Barry F. via petsc-dev" <petsc-dev at mcs.anl.gov> writes:
> 
>>   Good question. I added the concept in 7b23a99a4cafc5f74ba930d377af92d6b7c0a4c1 and unfortunately didn't document why, nor could I find any mention of why in the mailing lists. The one thing I do remember is that there was a definite reason why it was needed (I just don't remember what).
>> 
>> Maybe it was as simple as the flush was too slow when it just was done always by default. Note that you cannot count on every process calling PetscViewerASCIISynchronizedPrintf() so not all processes will KNOW if synchronized output is in use or not, this means every flush ALWAYS requires contact with everyone to see if there is anything to flush. (Yes the current flush code is very inefficient, perhaps making it much faster would help, perhaps
>> each flush could start with an MPI_Allreduce to know if it should proceed.). 
>> 
>> The push/pop design was suggested by Jed (and is lightly documented in the email archives).
>> 
>> So my conclusion is, with the current flush code the option has too exist; with much faster flush code I don't know if the flag is needed.
>> 
>>   Barry
>> 
>> 
>> 
>> 
>>> On May 2, 2019, at 11:13 AM, Hapla Vaclav via petsc-dev <petsc-dev at mcs.anl.gov> wrote:
>>> 
>>> Is PetscViewerASCIIPushSynchronized / PetscViewerASCIIPopSynchronized really needed?
>>> 
>>> I just got a feedback it's really confusing.
>>> Why we cannot just switch to the "synchronized mode" with the first call to PetscViewerASCIISynchronizedPrintf, and then hold this mode until PetscViewerFlush, which must be called anyway to get the anticipated output?
>>> What is the motivation for multiple consecutive pushes when this is a boolean state?
>>> 
>>> I would at least try to improve the documentation anyway. For example, PetscViewerASCIISynchronizedPrintf mentions PetscViewerASCIISynchronizeAllow which no longer exists. I will fix that at once.
>>> 
>>> Thanks,
>>> Vaclav