Let me follow up discussion on creating/controlling viewers from options.

Barry agrees to rename PetscOptionsGetViewer() to 
PetscViewerCreateFromOptions() in Issue 
#291<https://bitbucket.org/petsc/petsc/issues/291/>.

But now I can see a problem: it mixes Get and Create behavior, as it returns 
the singleton provided by PETSC_VIEWER_*_() if possible, otherwise creates a 
new viewer using PetscViewerCreate(). So actually both names 
PetscOptionsGetViewer() and PetscViewerCreateFromOptions() are confusing.

I think this design is unnecessarily complicated and not really useful.
I don't see any advantage of reusing the singleton in this context.
And leads to some IMHO unexpected behavior:
* -myviewer hdf5:: and -myviewer hdf5 is not the same! I didn't know that but 
hdf5:: creates a new viewer and hdf5 reuses the singleton.
* Any properties (including options prefix) set to the viewer returned by 
PetscOptionsGetViewer() actually affect the singleton and vice versa.
  (See https://bitbucket.org/petsc/petsc/commits/324f959)

So in my opinion PetscOptionsGetViewer()
* should really be renamed PetscViewerCreateFromOptions(),
* should _always_ return a new instance and call PetscViewerSetFromOptions() on 
it,
* could also set the passed prefix to the viewer, i.e. 
PetscViewerCreateFromOptions(comm, options, "pre_", ...) would result in the 
viewer having prefix pre_

Do you think it would break anything?

Thanks,
Vaclav

BTW all PETSC_VIEWER_XXX_() manpages say "Creates a XXX PetscViewer shared by 
all processors in a communicator."
This is apparently not true - they are singletons stashed to the communicator 
by MPI_Comm_set_attr() and next time reused if found using MPI_Comm_get_attr(), 
right?
So it should be Returns, not Creates.

Reply via email to