Utilities::MPI::create_mpi_data_type_n_bytes

[tjhei]

part of #12752 and #12873

[bangerth]

Looks good. The design here invited resource deallocation failures because the type has no destructor associated with it. The function is probably still useful, but in a follow-up it might be useful to move the function into an internal namespace and write a wrapper function that returns something like std::unique_ptr<MPI_Datatype, ...> with a custom deallocation function as second argument that takes care of the call to MPI_Type_free. I can probably write that for you.

[bangerth]

Did you mean this?

Suggested change

	// We create an MPI datatype that has the layout A*nB where A is
	// We create an MPI datatype that has the layout A*n+B where A is

[bangerth]

Suggested change

	test_data_type(long long n_bytes)
	test_data_type(const std::uint64_t n_bytes)

[tjhei]

ok. what is your preference for the argument ofcreate_mpi_data_type_n_bytes()? MPI_Count, size_t, std::uint64_t?

[bangerth]

The whole point of this function is to work around limitations in MPI data type sizes :-) So let's do std::uint64_t.

[bangerth]

Or std::size_t. Maybe that's the better choice because that's always going to be big enough to address all elements of any array you can think of.

[tjhei]

and write a wrapper function that returns something like std::unique_ptr<MPI_Datatype, ...> with a custom deallocation function as

I am not a big fan of mixing c-style MPI calls (which will happen with this newly created MPI_Datatype) with c++ style. See duplicate_communicator for an example that is handled similarly. I changed to return the result by value. I am assume this works because this is typically some kind of handle (32 or 64 bit int). Do you like that better?

[bangerth]

Suggested change

	create_mpi_data_type_n_bytes(std::size_t n_bytes);
	create_mpi_data_type_n_bytes(const std::size_t n_bytes);

[bangerth]

indent this line a bit more

[bangerth]

Suggested change

	create_mpi_data_type_n_bytes(std::size_t n_bytes)
	create_mpi_data_type_n_bytes(const std::size_t n_bytes)

[tjhei]

All good points. Thanks.