Add Y-partition and XY-partition tests

[simone-silvestri]

The code should allow all types of partitioning but there is a bug preventing the tests from passing for y- and xy-partitions.

This PR aims to fix the bug (and consequently add the excluded y- and xy-partitioning tests)

rework fill_halo_regions! to split out non-communicating and communicating boundary conditions such that the latter are executed alone.

To give an example, for

julia> boundary_conditions = FieldBoundaryConditions(west = NoFluxBoundaryCondition(), east = NoFluxBoundaryCondition(), south = ImpenetrableBoundaryCondition(), north = DistributedCommunicationBoundaryCondition(), bottom = nothing, top = nothing)
Oceananigans.FieldBoundaryConditions, with boundary conditions
├── west: FluxBoundaryCondition: Nothing
├── east: FluxBoundaryCondition: Nothing
├── south: OpenBoundaryCondition: Nothing
├── north: DistributedBoundaryCondition: Nothing
├── bottom: Nothing
├── top: Nothing
└── immersed: DefaultBoundaryCondition (FluxBoundaryCondition: Nothing)

in main:

julia> halo_tuple = permute_boundary_conditions(boundary_conditions);

julia> for i in 1:length(halo_tuple[1])
              @info "operation $(halo_tuple[1][i]) with bcs $((halo_tuple[2][i], halo_tuple[3][i]))"
       end
[ Info: operation fill_bottom_and_top_halo! with bcs (nothing, nothing)
[ Info: operation fill_west_and_east_halo! with bcs (FluxBoundaryCondition: Nothing, FluxBoundaryCondition: Nothing)
[ Info: operation fill_south_and_north_halo! with bcs (OpenBoundaryCondition: Nothing, BoundaryCondition{Oceananigans.BoundaryConditions.DistributedCommunication, Nothing})

in this PR

julia> halo_tuple = permute_boundary_conditions(boundary_conditions);

julia> for i in 1:length(halo_tuple[1])
              @info "operation $(halo_tuple[1][i]) with bcs $(halo_tuple[2][i])"
       end
[ Info: operation fill_bottom_and_top_halo! with bcs (nothing, nothing)
[ Info: operation fill_south_halo! with bcs (OpenBoundaryCondition: Nothing,)
[ Info: operation fill_west_and_east_halo! with bcs (FluxBoundaryCondition: Nothing, FluxBoundaryCondition: Nothing)
[ Info: operation fill_north_halo! with bcs (DistributedBoundaryCondition: Nothing,)

[glwagner]

What's all this for?

[simone-silvestri]

in case we have a tuple of FieldBoundaryConditions instead of just one FieldBoundaryConditions where we can just do (bc.west, bc.east)

[glwagner]

Oh okay, so this will extract a "tuple of west bcs" and a "tuple of east bcs".

Can you write that in a comment?

[glwagner]

Isn't there an assumption hidden in using bcs[1]? What is that assumption? Document your assumption with a comment.

[glwagner]

This doesn't explain it to me completely. Why would buffered communication require halos to be filled last? What do buffers have to do with it? Is that point that you have to complete other halo filling tasks before communicating, if you want to avoid filling the halos again after communication?

It seems like we could communicate first before filling halos, right?

[simone-silvestri]

I will update the comment. If you want asynchronous communication there are two options:

1. Fill all local halos - fill the buffers and start the communication - perform any required computation - complete the communication.
2. Fill the buffers and start the communication - perform any required computation - complete the communication - Fill any local halos.

with the first option, nothing changes in the case of non-distributed fields so I went in that direction in the #3125.

[glwagner]

By "complete the communication" you mean "wait until communication finishes" right? Eg there is not a separate action associated with "completing" the communication; it's just that it may not be finished at the time that you are done performing the preliminary computations. Or does "completing" mean something else?

[simone-silvestri]

completing the communication requires waiting until the communication is finished and then filling the relevant halos from the received buffer. In practice, we can think about it as waiting until the communication is finished (assuming that filling the relevant halos is part of the communication process).

[glwagner]

Okay so the first step is to initiate communication, and the second step is to wait for communication to finish and then fill halos from the buffer. The code is misleading, because the initiation of communication is currently called "fill halo regions!". But halo filling is actually occurring in the second step, right?

[simone-silvestri]

in asynchronous communication fill_halo_regions!(f; async = true) yes, the communication is completed in the synchronize_communication!(f) otherwise everything is completed in the fill_halo_regions! function

[glwagner]

Perhaps finish_fill_halo_regions!(f) or something would be helpful for the second step

[glwagner]

And probably neesd a comment in the code to indicate that the halo regions are not actually filled into the second step in the case of a distributed computation

[simone-silvestri]

We assume that the second step completes everything: i.e. wait for communication to complete and fill the halos

Oceananigans.jl/src/DistributedComputations/distributed_fields.jl

Lines 56 to 77 in d0b7ec8

	synchronize_communication!(field)
	complete the halo passing of `field` among processors.
	"""
	function synchronize_communication!(field)
	arch = architecture(field.grid)
	# Wait for outstanding requests
	if !isempty(arch.mpi_requests)
	cooperative_waitall!(arch.mpi_requests)
	# Reset MPI tag
	arch.mpi_tag[] -= arch.mpi_tag[]
	# Reset MPI requests
	empty!(arch.mpi_requests)
	end
	recv_from_buffers!(field.data, field.boundary_buffers, field.grid)
	return nothing
	end

[glwagner]

What is the meaning of "length(halo_tuple[1])"?

[simone-silvestri]

changed to number_of_tasks

[simone-silvestri]

I think I should have addressed all comments