[petsc-dev] proposed minor PetscPartitioner changes

[Jed Brown]

Matthew Knepley <knepley at gmail.com> writes:

> On Wed, Nov 8, 2017 at 10:04 AM, Vaclav Hapla <vaclav.hapla at erdw.ethz.ch>
> wrote:
>
>>
>> 8. 11. 2017 v 15:59, Matthew Knepley <knepley at gmail.com>:
>>
>> On Wed, Nov 8, 2017 at 9:14 AM, Vaclav Hapla <vaclav.hapla at erdw.ethz.ch>
>> wrote:
>>
>>>
>>> 8. 11. 2017 v 14:52, Jed Brown <jed at jedbrown.org>:
>>>
>>> Matthew Knepley <knepley at gmail.com> writes:
>>>
>>> On Wed, Nov 8, 2017 at 4: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. 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.
>>>
>>>
>>> This is wrong.
>>>
>>>
>>> Matt, do you mean what I wrote above is not true (then I'm going to prove
>>> that it's true), or the opposite that the PetscPartitionerPartition design
>>> is actually not good :-)
>>>
>>
>> I meant "they must keep the same basic information just in two different
>> forms" is wrong (I had to leave for work). The Section gives offsets for
>> each partition, and the IS stores the actual points being sent.
>>
>>
>> OK, now I see. And do you think the Section could be somehow computed from
>> just the IS?
>>
>
> No, this is the right structure.

Oh come on.  You're defending a quadratic algorithm.

      ierr = ParMETIS_V3_PartKway(vtxdist, xadj, adjncy, vwgt, adjwgt, &wgtflag, &numflag, &ncon, &nparts, tpwgts, ubvec, options, &edgeCut, assignment, &comm);
      // ...
  for (p = 0, i = 0; p < nparts; ++p) {
    for (v = 0; v < nvtxs; ++v) {
      if (assignment[v] == p) points[i++] = v;
    }
  }

MatPartitioningApply creates an IS with "assignment" and can be
converted to a global numbering with ISPartitioningToNumbering.  You
could as well have an ISPartitioningToSectionAndIS() that produces your
representation, preferably without this silly quadratic algorithm.