[petsc-dev] setting viewer format from options

[Smith, Barry F.]

   I think the following should satisfy your needs for the example.

 ierr = PetscViewerCreate(PETSC_COMM_WORLD,&fd);CHKERRQ(ierr);
  ierr = PetscViewerSetType(fd,PETSCVIEWERBINARY);CHKERRQ(ierr);
  ierr = PetscViewerSetFromOptions(fd);CHKERRQ(ierr);
           PetscOptionsGetEnum(NULL,NULL,"-viewer_format",PetscViewerFormats,(PetscEnum*)format,&flg);
           if (flg) {
              PetscViewerPushFormat(fd,format);
           }
  ierr = PetscViewerFileSetMode(fd,FILE_MODE_READ);CHKERRQ(ierr);
  ierr = PetscViewerFileSetName(fd,file);CHKERRQ(ierr);

   I'm very hesitant to have the -viewer_format supported by PETSc because it reintroduces the concept of "setting" the viewer format, as opposed to pushing and popping formats. I think it is a mistake to have both the ability to set the format and to push/pop it, they don't play well together.

   Barry




> On Jan 17, 2019, at 4:49 AM, Hapla Vaclav <vaclav.hapla at erdw.ethz.ch> wrote:
> 
> 
> 
>> On 17 Jan 2019, at 00:47, Smith, Barry F. <bsmith at mcs.anl.gov> wrote:
>> 
>> 
>> 
>>> On Jan 16, 2019, at 6:47 AM, Hapla Vaclav via petsc-dev <petsc-dev at mcs.anl.gov> wrote:
>>> 
>>> I need to distinguish a PetscViewerFormat in DMLoad() and MatLoad().
>> 
>>  Presumably this information would be stored in the *.info file associated with the datafile you are opening for reading. It makes no sense for it to come from the command line since it is a property of the file being read. 
> 
> Please take into account all formats and all creatures that can be loaded. I'm currently interested in Mat, Vec, IS and DMPlex. I don't know about further classes but I guess there are some more ones that can stored into a file.
> 
> Do you want to introduce the concept of .info files for DMPlex?
> Do you want users saving matrices into MATLAB MAT file to write the .info file on their own just to be able to load it?
> Or the same for a mesh stored in HDF5/XDMF which is already two files (.h5 and .xdmf)?
> 
> Yes, the format is the property of the file being read. But the file being read might also come from command line.
> 
> The most user friendly way would be to sniff the file and decide the format automatically. Sometimes it could be just decided by file extension, sometimes the file would have to be read for specific structure or metadata (e.g. different HDF5 flavors). This way we could also get rid of DMPlexCreateFromFile() and just use DMLoad() instead. But I feel like this automatic decision should still be overridable. Opinions?
> 
>> 
>>  Why not have an option in MatLoad(), -matload_viewer_format that gets the format, use a PetscViewerPushFormat(), do the (*matload)() and then pop the format?
> 
> Format is the property of the viewer. Why do you think it's more systematic to set in Mat API? Do you want to introduce the same concept for IS, Vec and DMPlex?
> 
>> 
>>  Barry
>> 
>> Note that other properties of the data file are handled this way: MAT_SYMMETRIC, MAT_SPD
> 
> Yes but these are matrix properties and not viewer ones. So I don't think this is directly related.
> 
> 
> You also wrote at BB:
>> I don’t think I like this pull request. It reintroduces a concept that was removed recently (the ability to set a viewer format; we switched to pushing and poping viewers). PetscOptionsGetViewer() get viewer was introduced to manage handling viewers from the options database with a lot of flexibility and with formats. Why can’t your use case use the PetscOptionsGetViewer() approach instead of introducing this anachronism?
> 
> 
> PetscViewer is PetscObject as any other. Why do you favor something like CreateFromOptions (the name is actually PetscOptionsGetViewer which is a bit confusing IMHO) over the usual Create + SetFromOptions approach?
> 
> My motivation can be seen e.g. in
> https://bitbucket.org/haplav/petsc/src/master/src/mat/examples/tutorials/ex10.c#lines-40
> I want to be able to set PETSCVIEWERBINARY as default but able to override it from options. What's wrong on it? Why this should differ from any other classes?
> 
> We have many alternative ways to create a viewer and set its type and format:
> - PetscViewer*Open() (e.g. PetscViewerHDF5Open) - always limited to one PetscViewerType and don't set format
> - PetscObjectViewFromOptions() + all those <Class>ViewFromOptions() - set the viewer properties, and view at the same time - limited to View, and all must be set from options
> - PetscOptionsGetViewer() - creates the viewer, and sets type and format - not allowing to set defaults programmatically as I do in ex10.c
> - have I forgotten something?
> 
> All these have their limitations and do more things at once whereas usual Create + SetFromOptions seems to me more flexible and maintainable.
> 
> But if you still don't agree, then PetscViewerSetFromOptions() is actually only useful for setting some type- or format-specific options. Current examples with PetscViewerSetFromOptions() (except of ex10.c modified by me, mentioned above) have indeed the type and format hard-wired and call PetscViewerSetFromOptions() afterwards. But why then PetscViewerSetFromOptions() gets -viewer_type? I think it should get both type and format, or none of them, otherwise it's confusing.
> 
> Vaclav
> 
>> 
>> 
>>> And it would be handy to be able to set it from options. So I wanted to add -viewer_format option into PetscViewerSetFromOptions().
>>> 
>>> But the problem is PetscViewerSetFormat() is deprecated and PetscViewerPushFormat() needs to be paired with PetscViewerPopFormat() which I don't know where to place correctly.
>>> 
>>> Can you see some clean way to proceed?
>>> 
>>> Thanks
>>> 
>>> Vaclav