Add kaHIP k-way partitioning

[reguly]

It looks like currently if you have both parmetis and kahip, and as for kway, kahip will be used, and there is no way to get parmetis. Please allow the use of both. In the call to op_partition, you can specify both combinations, so it's just a matter of propagating this information into op_partition_kway

[reguly]

Yes, that is a good idea! this is already a cpp file, and we don’t really need extern “C” for op_partition_kway, just op_partition itself. So you can go ahead and template it!

On 2022. Jul 20., at 13:50, mgsium @.***> wrote:

@mgsium commented on this pull request.

In op2/src/mpi/op_mpi_part_core.cpp https://github.com/OP-DSL/OP2-Common/pull/231#discussion_r925512443:

@@ -68,6 +68,14 @@ typedef int idx_t;

endif

endif

+// kaHIP header +#ifdef HAVE_KAHIP +#include + +typedef idxtype idx_t; Could we define a generic op_partition_kway_generic outside the scope of extern "C", then call this from op_partition_kway. Afaik this remains C-compatible if we just expose the signature of op_partition_kway with extern "C", and keep the body outside?

— Reply to this email directly, view it on GitHub https://github.com/OP-DSL/OP2-Common/pull/231#discussion_r925512443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJWVVKFZBKXAYKWLETVFBDVU7RW3ANCNFSM53TWZXLQ. You are receiving this because you commented.

[mgsium]

I think some overloading is still necessary because the two partition functions don't take the same arguments. The best I could think of was:

/*******************************************************************************
 * Generic wrapper for kway-partition functions
 *******************************************************************************/

template <class T>
void do_partition(T *vtxdist, T *xadj, T *adjncy, T *wgtflag, T *numflag, 
            T *ncon, T *nparts,real_t *tpwgts, real_t *ubvec, T *options, 
            T *edgecut, T *part, MPI_Comm *comm) {}

#ifdef HAVE_PARMETIS
template<> void do_partition<idx_t>(idx_t *vtxdist, idx_t *xadj, idx_t *adjncy, 
            idx_t *wgtflag, idx_t *numflag, idx_t *ncon, idx_t *nparts,real_t *tpwgts, 
            real_t *ubvec, idx_t *options, idx_t *edgecut, idx_t *part, MPI_Comm *comm) {
  ParMETIS_V3_PartKway( vtxdist, xadj, adjncy, NULL, NULL, wgtflag, numflag, ncon, 
                        nparts, tpwgts, ubvec, options, edgecut, part, comm);
}
#endif

#ifdef HAVE_KAHIP
template<> void do_partition<idxtype>(idxtype *vtxdist, idxtype *xadj, idxtype *adjncy, 
            idxtype *wgtflag, idxtype *numflag, idxtype *ncon, idxtype *nparts, real_t *tpwgts, 
            real_t *ubvec, idxtype *options, idxtype *edgecut, idxtype *part, MPI_Comm *comm) {
  double imb = 0.03;
  ParHIPPartitionKWay( vtxdist, xadj, adjncy, NULL, NULL, (int*) nparts, &imb, 
                       false, 1, ULTRAFASTMESH, (int*) edgecut, part, comm);
}
#endif

The long argument list is a little ugly (although it can be made a little better by omitting the names of unused arguments). Not an ideal solution but it feels less cumbersome than overloading op_partition_kway.

[reguly]

Could you not template the whole op_partition_kway function?

i.e. template <bool use_kahip, typename indextype> void op_partition_kway() { then use indextype throughout }

On 2022. Jul 20., at 19:33, mgsium @.***> wrote:

I think some overloading is still necessary because the two partition functions don't take the same arguments. The best I could think of was:

/***

• Generic wrapper for kway-partition functions ***/

template void do_partition(T vtxdist, T xadj, T adjncy, T wgtflag, T numflag, T ncon, T nparts,real_t tpwgts, real_t ubvec, T options, T edgecut, T part, MPI_Comm *comm) {}

ifdef HAVE_PARMETIS

template<> void do_partition(idx_t vtxdist, idx_t xadj, idx_t adjncy, idx_t wgtflag, idx_t numflag, idx_t ncon, idx_t nparts,real_t tpwgts, real_t ubvec, idx_t options, idx_t edgecut, idx_t part, MPI_Comm *comm) { ParMETIS_V3_PartKway( vtxdist, xadj, adjncy, NULL, NULL, wgtflag, numflag, ncon, nparts, tpwgts, ubvec, options, edgecut, part, comm); }

endif

ifdef HAVE_KAHIP

template<> void do_partition(idxtype vtxdist, idxtype xadj, idxtype adjncy, idxtype wgtflag, idxtype numflag, idxtype ncon, idxtype nparts, real_t tpwgts, real_t ubvec, idxtype options, idxtype edgecut, idxtype part, MPI_Comm comm) { double imb = 0.03; ParHIPPartitionKWay( vtxdist, xadj, adjncy, NULL, NULL, (int) nparts, &imb, false, 1, ULTRAFASTMESH, (int*) edgecut, part, comm); }

endif

The long argument list is a little ugly (although it can be made a little better by omitting the names of unused arguments). Not an ideal solution but it feels less cumbersome than overloading op_partition_kway.

— Reply to this email directly, view it on GitHub https://github.com/OP-DSL/OP2-Common/pull/231#issuecomment-1190561424, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJWVVI2IWZ775QWPUUKAWLVVAZ7NANCNFSM53TWZXLQ. You are receiving this because you commented.

[mgsium]

I'm doing that too, but there's still the problem of choosing whether to call the kahip or parmetis function at runtime.

[mgsium]

On second thoughts, do_partition doesn't need to be templated. This should suffice:

#ifdef HAVE_PARMETIS
void do_partition(idx_t *vtxdist, idx_t *xadj, ...) {
  ParMETIS_V3_PartKway( vtxdist, xadj, adjncy, NULL, NULL, wgtflag, numflag, ncon, 
                        nparts, tpwgts, ubvec, options, edgecut, part, comm);
}
#endif

#ifdef HAVE_KAHIP
void do_partition(idxtype *vtxdist, idxtype *xadj, ...) {
  double imb = 0.03;
  ParHIPPartitionKWay( vtxdist, xadj, adjncy, NULL, NULL, (int*) nparts, &imb, 
                       false, 1, ULTRAFASTMESH, (int*) edgecut, part, comm);
}
#endif

[reguly]

let's see :)

[mgsium]

Great! I'll just change the include and library paths and that should be it

[gihanmudalige]

@reguly thanks for following up on this. Seems @mgsium has completed the work in record time !

@mgsium I assume this is now working with the Makefile system in the master branch ? If we are to merge this I think it would be good to add some documentation on the new capabilities. For example adding details to : https://op2-dsl.readthedocs.io/en/latest/api.html?highlight=partition#c.op_partition Furthermore if there is a need to have any runtime flags to activate this partiotioner capabilities, that needs to be documented as well. Finally have you checked the coding standards and run it through git clang-format ?

Thanks.

[bozbez]

The Makefiles look good to me. Once this is merged it would also be worth updating the spack recipe.

[mgsium]

@gihanmudalige I updated the docs and ran clang-format on the changes. I've been using the Makefile to build and test so all should be good there.