cpp to MPI type mapping improvements

[schnellerhase]

mpi_type is redesigned in favor of a type trait based system that is easier to extend. Especially this no longer requires a function call to obtain the mpi type, the interface change is

dolfinx::MPI::mpi_type<T>() -> dolfinx::MPI::mpi_t<T>

Additionally the mapping is extended to previously non matched types.

Oddly the mapping of the fixed width std types to their MPI equivalent, for example std::int32_t -> MPI_INT32_T seems to break on some platforms. No clue why, because using the standard C type namings works, so are the MPI types not pointing to the same ones?

[jhale]

Could you be more specific about which platforms the fixed width conversion fails? And what is the compile error?

[jhale]

uint_8 is unsigned char on some platforms see e.g. https://en.cppreference.com/w/cpp/types/integer leading to the error with the redefinition

[schnellerhase]

Yes, just missed that one to revert :)

The errors that you see now in the CI were the ones I discovered. Locally I couldn't reproduce it, but I also didn't check other versions.

long long unsigned int is somehow not matched here.

[schnellerhase]

C standard seems to only guarantee at least 64 bit width of unsigned long long not exactly equal, see https://en.cppreference.com/w/cpp/language/types.

However I couldn't find anything hinting a non 64-bit width on these platforms.

[garth-wells]

For integers, to avoid any ambiguity I'd suggest providing interfaces only for fixed-width integers with the exception of int. We shouldn't (and don't) use long int, long long int, etc, in DOLFINx.

[garth-wells]

I'm not convinced that dolfinx::MPI::mpi_t<std::int32_t> adds anything over MPI_INT32_T. The latter is simpler.

dolfinx::MPI::mpi_t is helpful where we have templated over the type being communicated.

[schnellerhase]

Not the perfect interface for sure, but it allows us to stick to one type system in the code base.

We could also go for something like dolfinx::MPI::mpi_t<sendsizes::value_type>, which removes the possibility of a type mismatch, but increases code complexity for the caller.

[schnellerhase]

For integers, to avoid any ambiguity I'd suggest providing interfaces only for fixed-width integers with the exception of int. We shouldn't (and don't) use long int, long long int, etc, in DOLFINx.

Agreed, if we can get it to work I'd be happy to see the fixed width version used as well. My understanding was always that the fixed width types of std are just replaced with the corresponding C-type on the machine, so if for example unsigned long long is 64 bit, std::uint64_t would just be a different name for it.

Its most certainly caused by this type def https://github.com/FEniCS/dolfinx/blob/main/cpp/dolfinx/graph/partitioners.cpp#L622 - but we need it as Kahip expects that type https://github.com/KaHIP/KaHIP/blob/master/parallel/parallel_src/interface/parhip_interface.h.

Anybody got an idea whats causing this?

[jhale]

Regarding the kahip interface, does anyone remember why we are we using unsigned long long there?

Edit: I missed the link to the kahip source. Does kahip have a native C++ interface? Seems odd to have to go through a C interface.

[schnellerhase]

Don't think there is any other interface, at least it is the only occurrence of ParHIPPartitionKWay in the repo 😄

[schnellerhase]

Couldn't get the long long unsigned int to work on all platforms. Switched to the fixed width overloads and made the single occasion of the long long unsigned int for Kahip explicit with MPI_UNSIGNED_LONG_LONG

[jhale]

This is the right approach if KaHiP is really using long long unsigned underneath. Still odd they don't provide a native C++ call...

Looks good to me. @garth-wells you still had an open review comment about whether it's best just to use the MPI types directly in some cases. Could you comment further on that now?

We should consider fixed width floats when we move to C++23 https://en.cppreference.com/w/cpp/types/floating-point

[IgorBaratta]

This is the right approach if KaHiP is really using long long unsigned underneath. Still odd they don't provide a native C++ call...

https://github.com/KaHIP/KaHIP/blob/master/parallel/parallel_src/interface/parhip_interface.h#L15
KaHiP uses unsigned long long for its interface and also internally.