[petsc-dev] PetscViewerGetSubcomm_ASCII() broken?

[Dmitry Karpeev]

More accurately, the PetscViewerASCII subcomm viewer mechanism appears
broken.
I'm ready to push a fix, but since this is a heavily used and sensitive
part of the code
I wanted to make sure I'm not missing anything this close to a release.
The problem seems to start here:
http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l827
In particular,

   854 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l854>
  (*outviewer)->ops->destroy = PetscViewerDestroy_ASCII_Singleton;

   855 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l855>
  /* following might not be correct??? */

   856 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l856>
  if (rank) {

   857 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l857>
    (*outviewer)->ops->flush = 0;

   858 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l858>
  } else {

   859 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l859>
    (*outviewer)->ops->flush = PetscViewerFlush_ASCII_Singleton_0;

   860 <http://petsc.cs.iit.edu/petsc/petsc-dev/file/dc597ff7b29f/src/sys/viewer/impls/ascii/filev.c#l860>
  }

I totally agree with the comment in the above snippet.  In particular, if
rank 0 of the original
comm isn't in the subcomm, no flushing will occur.  And if rank 0 makes it
into the subcomm,
it will be the only proc writing to the file.  I actually experience both
of these scenarios
when viewing GASM subdomain ISs, which can live on arbitrary subcomms
(including
subcomm==comm).

The fix here seems to be relatively simple -- just leave the original
function pointer in the vtable.
There are other things to navigate around -- for example, we can't just
copy the fd, since only the
original rank 0 has it, and we have to be careful about reopening the file
when subcomm==comm,
but those are easily taken care of.

I've run some tests, and it seems to work okay.  I'm sure it will be caught
in the nighlies, if this commit
breaks something.  Otherwise, as currently implemented
PetscViewerGetSubcomm appears to be useless
(unless subcomm=PETSC_COMM_SELF; by the way, why don't we just use
PetscViewerGetSubcomm(view, PETSC_COMM_SELF, &sviewer) instead of the
odd-sounding
PetscViewerGetSingleton(view, &sviewer)?)

Does anybody object to my pushing a fix to this?

Dmitry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120506/95b17000/attachment.html>