On Fri, Nov 10, 2017 at 8:56 AM, Jed Brown <[email protected]> wrote:
> Vaclav Hapla <[email protected]> writes: > > >> 10. 11. 2017 v 5:09, Smith, Barry F. <[email protected]>: > >> > >> > >> > >>> On Nov 8, 2017, at 3:52 AM, Vaclav Hapla <[email protected]> > wrote: > >>> > >>> > >>>> 8. 11. 2017 v 9:06, Lisandro Dalcin <[email protected]>: > >>>> > >>>> On 8 November 2017 at 05:51, Smith, Barry F. <[email protected]> > wrote: > >>>>> > >>>>>> On Nov 7, 2017, at 1:33 AM, Lisandro Dalcin <[email protected]> > 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. This is clearly wrong. A matrix is the coordinate representation of a linear operator, and thus has a specific behavior under coordinate transformations. A graph is just connectivity, and really just a relation. I cannot count the number of times Barry has ranted about this on petsc-maint (usually about Vecs and arrays). The mathematical object is not its data structure. Matt > 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. > -- What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. -- Norbert Wiener https://www.cse.buffalo.edu/~knepley/ <http://www.caam.rice.edu/~mk51/>
