Issue #54 resolved

Rework organization of --xxx_view and XXXSetFromOptions()

BarryFSmith
created an issue

The discussion below is complete nonsense, I should look at code before talking so damn much. Anyways the code is scattered with

Mat/XXXViewFromOptions(mat,prefix,option_name)

It is not reasonable to put into MatSetFromOptions() a seperate check for each of these locations (especially since more locations may be added over time in various parts of PETSc).

The problem with the current code is

1) each XXXViewFromOptions() has a bunch of string tests impacting performance for small problems 2) each XXXViewFromOptions() creates the viewer (if it is related to a specific filename) and then destroys the viewer at the end of the call so that same file cannot be used for later views

Now if the XXXViewFromOptions() stashed that it had been checked from that location (hash the filename and line?) and stashed the viewer (object compose?) then there would be no performance hit from having these calls scattered around?

Crap below

Say we have a KSP problem. The user calls KSPView() to view the entire hierarchy of the solvers/matrices and KSPView() calls the PCView() which calls the MatView() etc ALL ON THE SAME VIEWER. Now say the user wants to view JUST the PC with a specific viewer (like a draw). In the code they can do KSPGetPC(), PCView() with that specific viewer.

Now we want to reproduce that same capability via the options database. -ksp_view OPTIONS triggers a hierarchical view of KSP, PC, Mat with the given OPTIONS (and possible viewer in the options). When does the view actually take place? for KSP I guess it makes sense after KSPSolve.

Now it seems doing -pc_view OPTIONS should have the same effect as KSPGetPC(), PCView() and where does the view actually take place? As you point out after the PCSetUp() is a logically place. Note that one could anally argue we should use the option -ksp_pc_view but let’s not.

Ok now let’s consider the given case for viewing the matrix. In the code one could do KPSGetPC(), PCFactorGetMatrix(), MatView() so anally the option could be -ksp_pc_factor_mat_view and when would the view get triggered? Well it makes sense right after the factorization. And if we shorten the option name out completely we get -mat_view (but we already have a mat_view for the originally matrix so to prevent problems we could have -factor_mat_view or -pc_factor_mat_view.

My conclusion based on the above is that Jed was right and this is the complete model. To implement we can do the following

1) Each XXXSetFromOptions() looks for -prefix_xxx_view and scarfs up the viewer and stashes it internally

2) Each XXX has a “trigger point” where the actual view is done (if the viewer exists in the option) for regular Mat it is MatAssemblyEnd() for factored matrix it is after the factorization, for PC it is after PCSetUp() for KSP after KSPSolve, for SNES after SNES solver for TS after TSSolve

3) Each XXX also a a variant of the trigger point for “before the operation” (that is used for example by the SAWs view, currently called -ksp_view_pre), for KSP for example it is before KSPSolve.

The current implementation more or less mimics this from the user point of view except some, like -pc_factor_mat_view disappeared or don’t exist and the options checks are generally done at the trigger point instead of in the setfromoptions.

I am fine with this model and over time “fixing” the code to move the checks into the appropriate setfromoptions and out of KSPSolve etc.

Perhaps so we should bump this operation process up to the level of PetscObject to get reuse of the code rather than rewriting the management Mat, PC, KSP etc etc.

Need to have a XXXSetViewer() to have a programatic interface that gives the same results as the options database interface

Dmitry suggests that XXSetFromOptions could always be called from XXXSetUp(). Well except for setting the type :-)

Comments (3)

  1. BarryFSmith reporter

    Mostly bad ideas. Now use XXXViewFromOptions() consistently and it would be easy to add runtime support to skip all option checking in these calls for performance for small problems

  2. Log in to comment