[petsc-dev] ISRestoreNonlocalIS?

[Barry Smith]

On Nov 18, 2010, at 3:18 PM, Jed Brown wrote:

> On Thu, Nov 18, 2010 at 21:44, Barry Smith <bsmith at mcs.anl.gov> wrote:
> Cacheing information about the parallel structure of the IS input into MatGetSubMatrix() inside the gotten matrix is IMHO not a good model (my god, what if someone passed in a different IS next time? just a joke).
> 
> I understand that there needs to be a less ad-hoc consistency model, but how are you going to check that the user didn't pass in a different IS, maybe of different size and maybe not?  Seems like the old IS still needs to be cached in some form, so you can check whether it has changed.
> 
> Here is the model we are proposing. An IS is a collection of indices, often stored in parallel. Some operations on IS's only require information about the local part of the indices, other operations require information about parts of the indices on other processes. Ok, that information about remote indices can be obtained through a collective operation. Sometimes one may want that same information about the remote part several different times, well it could do a collective each time or it could cache on the first time and reuse later (whether it caches or recommunicates is internal to the object and merely an optimization and need not be actively visible to the user.) The previous solution to this need for remote information was to use an ISAllGather() and stash that information "somewhere else" for later use (and it is currently stashed elsewhere as a SEQUENTIAL IS), but since that information is ONLY information about that particular IS I submit that any stashing (should it occur) should be inside the IS not inside some other IS object.
> 
> I am still skeptical that ISAllGather is ever really required, except as a convenience.  I realize that figuring out who to communicate with in MatGetSubMatrices is (even) messier without it, but there must be a more scalable way to implement it.  Especially when the overlap is local.
> 
> There is no VecGetTotalArray(), just sayin'.
>  
> > What is going on here?  If IS is caching this nonlocal stuff, then every function causing mutation (e.g. ISGeneralSetIndices, and a few others) needs to be visited and made to discard the gathered and nonlocal caches.
> 
>    This is just a coding detail and could be added.  But actually we should probably decide once and for all if IS's are mutable, I tend to think they shouldn't be but we are not completely consistent yet.
> 
> I would be happy with always immutable.  The only tricky thing is the in-place ISSort() that can operate on const data that the user passed in with ISGeneralCreate().  I'm still waiting for someone to pass in an array from a read-only page.  Sure, making a copy is not necessary for the algorithm, but I think the simplicity of a strictly immutable interface would be worth it.

   We could bag the in-place  ISSort() and make it return a new IS.

>   
> > I just don't understand the point of all these new functions, they feel like an inferior interface to something that is currently available.
> 
>   They are a proper replacement to a previous inferior interface that creates this strange new IS from a given IS. All the new stuff is is a API for accessing nonlocal parts of the IS efficiently and consistently and simply.
> 
> Could we at least get rid of ISGetTotalIndices() and ISGetNonlocalIndices() in favor of ISGetNonlocalIS() followed by ISGetIndices(), so that the interface allows working with ISStride and ISBlock efficiently?

  The problem is what is the "meaning" of that new IS? What communicator does it live on? It is returning information about a given IS, it really isn't an IS at all given my definition of IS from the previous email.  If you could come up with proper model for this beast (and have it cached in the original IS) then I would be happy if it was an IS and we didn't need the GetNonlocalIndices() and GetAllIndices().  With ISAllGather() it returns a sequential IS that is then passed in from MatGetSubMatrix_MPIAIJ() to MatGetSubMatrix_MPIAIJ_Private() to MatGetSubMatrices() but now you are passing a sequential IS to MatGetSubMatrices() while the isrow may be (or is) a parallel IS.  Explain this to me and we'll have a good model.

>  
>    For now ISAllGather() still exists, but in the long run I think we will be able to toss it.
> 
> Or rename it ISGetTotalIS() and make it cached by the IS.  For one-off (often preprocessing) use of gathered indices, perhaps there should be an advanced function ISCollectGarbage (or PetscObjectCollectGarbage) which could free the expensive gathered indices (and matrices used inside MatGetSubMatrix).
>  
> > Same question for ISAllGatherIndices which has been around for a long time.  It looks to me like the user can do ISCreateGeneral(), ISAllGather(), ISGetIndices() to achieve exactly the same thing (no performance difference).  Why even call it IS when no IS (or PETSc object of any kind is used by this function)?
> 
>   It is called an IS because it involves the manipulation of sets of indices which is what IS does. It is a static method for the IS class :-)
> 
> It seems a lot like the MatGetSubMatrixRaw that you were so offended by.

   In the past we did not always wrap "collections of indices" as an IS, I think it still exists from that time. If it is never used and will not be needed it could be removed.

   Barry

> 
> Jed