clean up Cartesian topology functions (#536)

Fixes #533.

Author: simonbyrne

docs/src/topology.md

@@ -1,13 +1,12 @@
 # Topology
 
 ```@docs
-MPI.Cart_coords
-MPI.Cart_coords!
+MPI.Dims_create
 MPI.Cart_create
 MPI.Cart_get
+MPI.Cart_coords
 MPI.Cart_rank
 MPI.Cart_shift
 MPI.Cart_sub
 MPI.Cartdim_get
-MPI.Dims_create!
 ```

src/deprecated.jl

@@ -224,3 +224,14 @@ import Base: @deprecate
 @deprecate(recv(source::Integer, tag::Integer, comm::Comm), recv(comm, MPI.Status; source=source, tag=tag), false)
 @deprecate(Sendrecv!(sendbuf, dest::Integer, sendtag::Integer, recvbuf, source::Integer, recvtag::Integer, comm::Comm),
            Sendrecv!(sendbuf, recvbuf, comm, MPI.Status; dest=dest, sendtag=sendtag, source=source, recvtag=recvtag)[2], false)
+
+@deprecate(Cart_create(comm_old::Comm, ndims::Integer, dims::MPIBuffertype{Cint}, periods::MPIBuffertype{Cint}, reorder),
+           Cart_create(comm_old, dims; periodic=periods, reorder=reorder), false)
+@deprecate(Cart_create(comm_old::Comm, dims::AbstractArray{T}, periods::Array{T}, reorder) where {T <: Integer},
+           Cart_create(comm_old, dims; periodic=periods, reorder=reorder), false)
+@deprecate(Dims_create!(nnodes::Integer, ndims::Integer, dims::MPIBuffertype{T}) where {T<:Integer},
+           dims .= Dims_create(nnodes, dims), false)
+@deprecate(Dims_create!(nnodes::Integer, dims::AbstractArray{T,N}) where {T<:Integer, N},
+           dims .= Dims_create(nnodes, dims), false)
+@deprecate(Cart_coords!(comm::Comm, rank::Integer, coords::MPIBuffertype{Cint}),
+           coords .= Cart_coords(comm, rank), false)

src/topology.jl

@@ -1,62 +1,78 @@
 """
-    Dims_create!(nnodes::Integer, [ndims::Integer], dims)
+    newdims = Dims_create(nnodes::Integer, dims)
 
-Create a division of `nnodes` processes in a Cartesian grid.
+A convenience function for selecting a balanced Cartesian grid of a total of `nnodes`
+nodes, for example to use with [`MPI.Cart_create`](@ref).
+
+`dims` is an array or tuple of integers specifying the number of nodes in each dimension.
+The function returns an array `newdims` of the same length, such that if `newdims[i] =
+dims[i]` if `dims[i]` is non-zero, and `prod(newdims) == nnodes`, and values `newdims` are
+as close to each other as possible.
+
+`nnodes` should be divisible by the product of the non-zero entries of `dims`.
 
 # External links
 $(_doc_external("MPI_Dims_create"))
 """
-function Dims_create!(nnodes::Integer, ndims::Integer, dims::MPIBuffertype{T}) where {T <: Integer}
-    # int MPI_Dims_create(int nnodes, int ndims, int dims[])
+function Dims_create(nnodes::Integer, dims)
+    dims = Cint[dim for dim in dims]
     @mpichk ccall((:MPI_Dims_create, libmpi), Cint,
-                  (Cint, Cint, Ptr{Cint}), nnodes, ndims, dims)
+                  (Cint, Cint, Ptr{Cint}), nnodes, length(dims), dims)
+    return dims
 end
 
-function Dims_create!(nnodes::Integer, dims::AbstractArray{T,N}) where {T<:Integer, N}
-    cdims = Cint.(dims[:])
-    ndims = length(cdims)
-    Dims_create!(nnodes, ndims, cdims)
-    dims[:] .= cdims
-end
 
 """
-    comm_cart = Cart_create(comm_old::Comm, [ndims::Integer], dims, periods, reorder)
+    comm_cart = Cart_create(comm::Comm, dims; periodic=map(_->false, dims), reorder=false)
+
+Create new MPI communicator with Cartesian topology information attached.
 
-Create new MPI communicator with Cartesian structure from an existent communicator.
+`dims` is an array or tuple of integers specifying the number of MPI processes in each
+coordinate direction, and `periodic` is an array or tuple of `Bool`s indicating the
+periodicity of each coordinate. `prod(dims)` must be less than or equal to the size of
+`comm`; if it is smaller than some processes are returned a null communicator.
+
+If `reorder == false` then the rank of each process in the new group is identical to its
+rank in the old group, otherwise the function may reorder the processes.
+
+See also [`MPI.Dims_create`](@ref).
 
 # External links
 $(_doc_external("MPI_Cart_create"))
 """
-function Cart_create(comm_old::Comm, ndims::Integer, dims::MPIBuffertype{Cint}, periods::MPIBuffertype{Cint}, reorder)
+function Cart_create(comm::Comm, dims; periodic = map(_->false, dims), reorder=false)
+    if !(dims isa Array{Cint})
+        dims = Cint[dim for dim in dims]
+    end
+    if !(periodic isa Array{Cint})
+        periodic = Cint[flag for flag in periodic]
+    end
+    length(dims) == length(periodic) || error("dims and periodic arguments are not the same length")
     comm_cart = Comm()
     # int MPI_Cart_create(MPI_Comm comm_old, int ndims, const int dims[],
     #                     const int periods[], int reorder, MPI_Comm *comm_cart)
     @mpichk ccall((:MPI_Cart_create, libmpi), Cint,
                   (MPI_Comm, Cint, Ptr{Cint}, Ptr{Cint}, Cint, Ptr{MPI_Comm}),
-                  comm_old, ndims, dims, periods, reorder, comm_cart)
+                  comm, length(dims), dims, periodic, reorder, comm_cart)
     if comm_cart != COMM_NULL
         finalizer(free, comm_cart)
     end
     comm_cart
-end
-
-function Cart_create(comm_old::Comm, dims::AbstractArray{T,N}, periods::Array{T,N}, reorder) where T <: Integer where N
-    cdims    = Cint.(dims[:])
-    cperiods = Cint.(periods[:])
-    ndims    = length(cdims)
-    Cart_create(comm_old, ndims, cdims, cperiods, reorder)
-end
+end    
 
 """
     rank = Cart_rank(comm::Comm, coords)
 
-Determine process rank in communicator `comm` with Cartesian structure.
-The `coords` array specifies the Cartesian coordinates of the process.
+Determine process rank in communicator `comm` with Cartesian structure.  The `coords`
+array specifies the 0-based Cartesian coordinates of the process. This is the inverse of [`MPI.Cart_coords`](@ref)
 
 # External links
 $(_doc_external("MPI_Cart_rank"))
 """
-function Cart_rank(comm::Comm, coords::MPIBuffertype{Cint})
+function Cart_rank(comm::Comm, coords)
+    if !(coords isa Vector{Cint})
+        coords = Cint[coord for coord in coords]
+    end
     rank = Ref{Cint}()
     # int MPI_Cart_rank(MPI_Comm comm, const int coords[], int *rank)
     @mpichk ccall((:MPI_Cart_rank, libmpi), Cint,
@@ -65,10 +81,6 @@ function Cart_rank(comm::Comm, coords::MPIBuffertype{Cint})
     Int(rank[])
 end
 
-function Cart_rank(comm::Comm, coords::AbstractArray{T}) where {T <: Integer}
-    ccoords = Cint.(coords[:])
-    Cart_rank(comm, ccoords)
-end
 
 """
     dims, periods, coords = Cart_get(comm::Comm)
@@ -112,34 +124,24 @@ function Cartdim_get(comm::Comm)
 end
 
 """
-    Cart_coords!(comm::Comm, rank::Integer, coords)
+    coords = Cart_coords(comm::Comm, rank::Integer=Comm_rank(comm))
 
-Determine coordinates of a given process in Cartesian communicator.
+Determine coordinates of a process with rank `rank` in the Cartesian communicator
+`comm`. If no `rank` is provided, it returns the coordinates of the current process.
+
+Returns an integer array of the 0-based coordinates. The inverse of [`Cart_rank`](@ref).
 
 # External links
 $(_doc_external("MPI_Cart_coords"))
 """
-function Cart_coords!(comm::Comm, rank::Integer, coords::MPIBuffertype{Cint})
+function Cart_coords(comm::Comm, rank::Integer=Comm_rank(comm))
     maxdims = Cartdim_get(comm)
+    coords = Vector{Cint}(undef, maxdims)
     # int MPI_Cart_coords(MPI_Comm comm, int rank, int maxdims, int coords[])
     @mpichk ccall((:MPI_Cart_coords, libmpi), Cint,
                   (MPI_Comm, Cint, Cint, Ptr{Cint}),
                   comm, rank, maxdims, coords)
-end
-
-"""
-    Cart_coords(comm::Comm)
-
-Determine coordinates of local process in Cartesian communicator.
-
-See also [`Cart_coords!`](@ref).
-"""
-function Cart_coords(comm::Comm)
-    maxdims = Cartdim_get(comm)
-    ccoords = Vector{Cint}(undef, maxdims)
-    rank    = Comm_rank(comm)
-    Cart_coords!(comm, rank, ccoords)
-    Int.(ccoords)
+    return Int.(coords)
 end
 
 """
@@ -174,7 +176,10 @@ be kept in the generated subgrid.
 # External links
 $(_doc_external("MPI_Cart_sub"))
 """
-function Cart_sub(comm::Comm, remain_dims::MPIBuffertype{Cint})
+function Cart_sub(comm::Comm, remain_dims)
+    if !(remain_dims isa Array{Cint})
+        remain_dims = Cint[dim for dim in remain_dims]
+    end
     comm_sub = Comm()
     # int MPI_Cart_sub(MPI_Comm comm, const int remain_dims[], MPI_Comm *comm_new)
     @mpichk ccall((:MPI_Cart_sub, libmpi), Cint,
@@ -185,8 +190,3 @@ function Cart_sub(comm::Comm, remain_dims::MPIBuffertype{Cint})
     end
     comm_sub
 end
-
-function Cart_sub(comm::Comm, remain_dims)
-    cremain_dims = [Cint(dim) for dim in remain_dims]
-    Cart_sub(comm, cremain_dims)
-end

test/test_cart_coords.jl

@@ -7,13 +7,11 @@ nnodes = MPI.Comm_size(comm)
 ndims = 3
 reorder = 1
 periods = [0,1,0]
-dims = [0,0,0]
-MPI.Dims_create!(nnodes, dims)
-comm_cart = MPI.Cart_create(comm, dims, periods, reorder)
+dims = MPI.Dims_create(nnodes, [0,0,0])
+comm_cart = MPI.Cart_create(comm, dims; periodic=periods, reorder=reorder)
 
 rank = MPI.Comm_rank(comm)
-ccoords = Cint[-1,-1,-1]
-MPI.Cart_coords!(comm_cart, rank, ccoords)
+ccoords = MPI.Cart_coords(comm_cart, rank)
 @test all(ccoords .>= 0)
 @test all(ccoords .< dims)
 

test/test_cart_create.jl

@@ -5,19 +5,14 @@ MPI.Init()
 comm = MPI.COMM_WORLD
 nnodes = MPI.Comm_size(comm)
 ndims = 3
-reorder = 1
-periods = [0 1 0]
-dims = [0 0 0]
-MPI.Dims_create!(nnodes, dims)
+dims = MPI.Dims_create(nnodes, [0,0,0])
 
 @test prod(dims) == nnodes
 
-cperiods = Cint.(periods[:])
-cdims = Cint.(dims[:])
-comm_cart = MPI.Cart_create(comm, ndims, cdims, cperiods, reorder)
+comm_cart = MPI.Cart_create(comm, dims; periodic=[0, 1, 0], reorder=1)
 @test MPI.Comm_size(comm_cart) == nnodes
 
-comm_cart2 = MPI.Cart_create(comm, dims, periods, reorder)
+comm_cart2 = MPI.Cart_create(comm, dims; periodic=(false, true, false), reorder=true)
 @test MPI.Comm_size(comm_cart2) == nnodes
 
 comm_sub1 = MPI.Cart_sub(comm_cart, (false, true, true))

test/test_cart_get.jl

@@ -12,7 +12,7 @@ periods = Cint[0, 1,      0]
 dims    = Cint[1, 1, nnodes]
 
 # create
-comm_cart = MPI.Cart_create(comm, maxdims, dims, periods, false)
+comm_cart = MPI.Cart_create(comm, dims; periodic=periods, reorder=false)
 
 # check number of dimensions
 @test MPI.Cartdim_get(comm_cart) == maxdims

test/test_cart_rank.jl

@@ -7,19 +7,15 @@ nnodes = MPI.Comm_size(comm)
 ndims = 3
 reorder = 1
 periods = [0,1,0]
-dims = [0,0,0]
-MPI.Dims_create!(nnodes, dims)
-comm_cart = MPI.Cart_create(comm, dims, periods, reorder)
-
-coords_check = Cint[-1, -1, -1]
+dims = MPI.Dims_create(nnodes, [0,0,0])
+comm_cart = MPI.Cart_create(comm, dims; periodic=periods, reorder=reorder)
 
 for k = 1:dims[3], j = 1:dims[2], i = 1:dims[1]
     cart_coords = [i, j, k] .- 1  # zero-based location
     cart_rank = MPI.Cart_rank(comm_cart, cart_coords)
-    @test cart_rank >= 0
-    @test cart_rank < nnodes
+    @test 0 <= cart_rank < nnodes
 
-    MPI.Cart_coords!(comm_cart, cart_rank, coords_check)
+    coords_check = MPI.Cart_coords(comm_cart, cart_rank)
     @test all(cart_coords .== coords_check)
 end
 

test/test_cart_shift.jl

@@ -7,9 +7,8 @@ nnodes = MPI.Comm_size(comm)
 ndims = 3
 reorder = 1
 periods = [0,1,0]
-dims = [0,0,0]
-MPI.Dims_create!(nnodes, dims)
-comm_cart = MPI.Cart_create(comm, dims, periods, reorder)
+dims = MPI.Dims_create(nnodes, [0,0,0])
+comm_cart = MPI.Cart_create(comm, dims; periodic=periods, reorder=reorder)
 coords = MPI.Cart_coords(comm_cart)
 disp = 1
 

test/test_dims_create.jl

@@ -6,18 +6,9 @@ comm = MPI.COMM_WORLD
 nnodes = MPI.Comm_size(comm)
 ndims = 3
 
-cdims = Cint[0,0,0]
-MPI.Dims_create!(nnodes, ndims, cdims)
-@test prod(cdims) == nnodes
-
-cdims = Cint[1,0,1]
-MPI.Dims_create!(nnodes, ndims, cdims)
-@test cdims == Cint[1,nnodes,1]
-
-for dims in ([0,0,0], [0 0 0])
-    MPI.Dims_create!(nnodes, dims)
-    @test prod(dims) == nnodes
-end
+@test prod(MPI.Dims_create(nnodes, (0,0,0))) == nnodes
+@test prod(MPI.Dims_create(nnodes, [0,0,0])) == nnodes
+@test MPI.Dims_create(nnodes, (1,0,1)) == [1, nnodes, 1]
 
 MPI.Finalize()
 @test MPI.Finalized()

test/test_sendrecv.jl

@@ -107,7 +107,7 @@ comm_size = MPI.Comm_size(comm)
 a = Float64[comm_rank, comm_rank, comm_rank]
 
 # construct cartesian communicator with 1D topology
-comm_cart = MPI.Cart_create(comm, 1, Cint[comm_size], Cint[1], false)
+comm_cart = MPI.Cart_create(comm, (comm_size,); periodic=(true,), reorder=false)
 
 # get source and dest ranks using Cart_shift
 src_rank, dest_rank = MPI.Cart_shift(comm_cart, 0, -1)