[petsc-dev] ISRestoreNonlocalIS?

[Jed Brown]

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.


> > 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?


>    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.

Jed
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20101118/9809bf41/attachment.html>