[petsc-dev] proposed minor PetscPartitioner changes

[Jed Brown]

Vaclav Hapla <vaclav.hapla at erdw.ethz.ch> writes:

>> 10. 11. 2017 v 5:09, Smith, Barry F. <bsmith at mcs.anl.gov>:
>> 
>> 
>> 
>>> On Nov 8, 2017, at 3:52 AM, Vaclav Hapla <vaclav.hapla at erdw.ethz.ch> wrote:
>>> 
>>> 
>>>> 8. 11. 2017 v 9:06, Lisandro Dalcin <dalcinl at gmail.com>:
>>>> 
>>>> On 8 November 2017 at 05:51, Smith, Barry F. <bsmith at mcs.anl.gov> wrote:
>>>>> 
>>>>>> On Nov 7, 2017, at 1:33 AM, Lisandro Dalcin <dalcinl at gmail.com> wrote:
>>>>>> 
>>>>>> The only concern I have about PetscPartitioner is that the API depends
>>>>>> on DM (PetscPartitionerPartition_<TYPE> routines). Maybe
>>>>>> PetscPartitioner should eventually move to became more agnostic, and
>>>>>> that way it can be used to partition matrices and meshes.
>>>>> 
>>>>> This is certainly a serious flaw if PetscPartitioner is intended as THE API to use for partitioning. If it is not intended as THE API for partitioning then that is also a problem, because why have multiple APIs for what is essentially one set of abstractions.
>>>>> 
>>>> 
>>>> Note however that things looks easy to refactor. I'll try to team up
>>>> with Matt to improve things.
>>> 
>>> Wait, now we are at the beginning again. I actually wanted to do some refactoring of PetscPartitioner, starting with few cosmetic changes to make it better settable from options. But Barry kept me back of any edits since he think it's anti-systematic to keep two independent classes doing essentially the same. And I agree with that to be honest. It's strange to have two ParMetis, two Scotch and two whatever interfaces.
>> 
>>  Strange is not the word, f***up is the word
>> 
>>> The only thing I don't like on MatPartitioning is its name as it's not just for Mat Partitioning :-)
>>> 
>>> There are from my point of view multiple issues with PetscPartitioner. Let's look e.g. at PetscPartitionerPartition. It takes as arguments both PetscPartitioner and DM. This DM must be in fact DMPlex which is not checked so it will probably crash somewhere deep in the stack once the first DMPlex specific line is reached. Then there are two output arguments PetscSection partSection and IS *partition. The first must be created beforehand while the second is created inside. And I guess they must keep the same basic information just in two different forms.
>>> 
>>> Actually the original problem I wanted to solve is that src/dm/impls/plex/examples/tutorials/ex5.c fails with partitioner set to PETSCPARTITIONERPARMETIS for certain numbers of processes, see below. Let me start with pull request altering ex5.c so that partitioner type can be set from options properly and this bug can be reproduced easily.
>>> [ 0] ***ASSERTION failed on line 176 of file /scratch/petsc-dev/arch-linux-gcc-salvus/externalpackages/git.parmetis/libparmetis/comm.c: j == nnbrs
>>> [ 2] ***ASSERTION failed on line 176 of file /scratch/petsc-dev/arch-linux-gcc-salvus/externalpackages/git.parmetis/libparmetis/comm.c: j == nnbrs
>>> ex5: /scratch/petsc-dev/arch-linux-gcc-salvus/externalpackages/git.parmetis/libparmetis/comm.c:176: libparmetis__CommSetup: Assertion `j == nnbrs' failed.
>>> ex5: /scratch/petsc-dev/arch-linux-gcc-salvus/externalpackages/git.parmetis/libparmetis/comm.c:176: libparmetis__CommSetup: Assertion `j == nnbrs' failed.
>>> 
>>> I'm wondering whether the MatPartitioning interface has the same problem. But anyhow I mean it's maybe time to decide about the two interfaces before chasing all these PetscPartitioner issues.
>>> 
>>> I propose
>>> - to rename Mat Partitioning to just Partitioning/-er or take over the name PetscPartitioner,
>> 
>>  Why? What is wrong with Mat? Mat can represent any graph and graphs are always what partitioning packages actually partition. I don't see a reason for a different name. MatPartitioner does not, nor should it, directly partition meshes etc, those can/should be done by; the proper massaging of data, the creation of a MatPartitioner object, calling the partitioner and then using the result of the partitioner to build whatever appropriate data structures are needed for the mesh partitioner.
>
> Yes. Mat can represent any graph in several different ways -
> e.g. Laplacian, adjacency, incidence, oriented incidence matrix. The
> graph could be also represented in other way like a list of vertices
> and edges. 

Also known as COO format for a matrix.

> MatPartitioning picks just one representation as an input - the
> adjacency matrix. But I mean the picked representation does not
> matter, and the result is not a partitioning of any matrix, but
> partitioning of the graph. The graph is the underlying concept. This
> is why I don't consider the Mat prefix optimal.

Matrix and graph are equivalent concepts.  Mat is already extensible in
the sense that it can have many representations.

> (*) It's a similar problem as MatCreateVecs means to me "create a matrix of Vecs type" according to usual convention.

I think the key here is to look at the types.  VecCreateFromMat() might
be more clear in this context, but isn't clearly better.  Alternatively,
PetscCreateVec() and MatCreateVecs() would have this symmetry, but don't
follow our usual prefix conventions.