MG transfer global coarsening: Avoid sending empty messages

[kronbichler]

To make a test case of @marcfehling run on my MPI implementation, I had to ensure that there were no empty messages in addition to #16631, otherwise an MPI process would get stuck in the BlackBoxFineDoFHandlerView::reinit() function here:

dealii/include/deal.II/multigrid/mg_transfer_global_coarsening.templates.h

Lines 972 to 974 in d292264

	const int ierr_1 = MPI_Waitall(requests.size(),
	requests.data(),
	MPI_STATUSES_IGNORE);

I believe the empty index sets in the MPI::internal::ComputeIndexOwner functions can happen when we have sparse index ranges as we might touch an std::map entry without filling it somewhere. I did not investigate that closer, but I think that all tests we have do not rely on empty messages, so I think we should expect that this is more robust anyway. @peterrum there might a place where we could filter out that earlier, but I can't say for sure where the empty index sets come from, so I would appreciate some help.

Furthermore, I can't give a test case because the one used by @marcfehling is rather large, and I could not understand what geometric configuration would be needed: Whenever I tried to reduce the complexity, the MPI deadlock would go away, so it seems my MPI implementation might simply have run out of buffers or something. @marcfehling would you mind testing your full case also with this patch, to ensure that nothing else happens here?

[marcfehling]

I tried your patch with the one testcase that caused all the issues related to #16615 and #16631. It worked fine with various counts of MPI processes (while oversubscribing). Everything went fine.

I will try some even bigger scenarios on the expanse supercomputer in the next few days. But we don't have to wait until then to merge this PR.

[peterrum]

@peterrum there might a place where we could filter out that earlier, but I can't say for sure where the empty index sets come from, so I would appreciate some help.

This does not look right. A zero-size request means that there was no request (or there was a request that failed?)!?

This should not happen. The question is whose side effect this is?

I guess the dictionary send these empty requesters here:

dealii/source/base/mpi_compute_index_owner_internal.cc

Line 636 in da635b4

send_data[i].push_back(interval.second);

Which are received by the index owners here:

dealii/source/base/mpi_compute_index_owner_internal.cc

Lines 695 to 699 in da635b4

	for (unsigned int i = offset + 1;
	i < offset + buffer[offset].second + 1;
	++i)
	my_index_set.add_range(index_offset + buffer[i].first,
	index_offset + buffer[i].second);

This indicates that

dealii/source/base/mpi_compute_index_owner_internal.cc

Lines 756 to 761 in da635b4

	auto &request = requesters[owner_index_guess];
	if (request.empty() || request.back().first != rank_of_request)
	request.emplace_back(
	rank_of_request,
	std::vector<
	std::pair<types::global_dof_index, types::global_dof_index>>());

is accidentally is adding an empty entry potentially at the wrong rank!?

I think it is worth to investigate this more to rule out that there is no bug. I cannot guarantee that I have time to debug this in the next days.

[bangerth]

i.second corresponds to an object of type IndexSet. Checking for begin() == end() is probably more easily understood if you said i.second.is_empty().

[bangerth]

Separately, I don't think this is sending an empty message. It perhaps sends an empty collection packed up into a string, but that's a non-zero length string.

[marcfehling]

Oh. I'm not sure right now whether we agreed that this patch does the right thing.

[peterrum]

Oh. I'm not sure right now whether we agreed that this patch does the right thing.

No, we did not and should investigate the reason for the issue!