> On 3 May 2019, at 17:34, Smith, Barry F. <[email protected]> wrote:
>
>
> I think this thing maybe over-designed. When I wrote it I was obsessed with
> having a single Flush method that one calls on the viewer that handles any
> type of flushing, yet efficiently. Hence the trickery. If we had a separate
> flush method for the synch calls I think we may be able to remove the
> push/pop and only use the synch flush in the code when necessary. I'll give
> it a try.
Sounds good. It would reflect PetscSynchronizedPrintf/Flush behavior which I
think is an advantage for user's understanding.
BTW Perhaps
PetscSynchronizedPrintf(comm
could then become just a shorthand to
PettscViewerASCIISynchronizedPrintf(PETSC_VIEWER_STDOUT_(comm)
The same for SynchronizedFlush.
Vaclav
>
> Barry
>
>
>> On May 2, 2019, at 11:08 PM, Jed Brown <[email protected]> wrote:
>>
>> "Smith, Barry F." <[email protected]> writes:
>>
>>>> On May 2, 2019, at 6:15 PM, Jed Brown <[email protected]> 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.
>>
>> I would think printing in the order of the pops would be okay. It means
>> each push would use a fresh buffer so only the content printed inside
>> that level of nesting would print. SegBuffer would be convenient for
>> this buffer management.
>>
>>>> (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.
>>
>> Yup, but it should be possible to deduplicate, even if via an internal
>> helper.
>>
>>> 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 ;)).
>>
>> Insert a check for allowsynchronized > 1 and run the test suite.
>>
>>> 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.
>>
>> I suspect that's right.
>
