From 008fa8c5cc3e769540187dd0939c4901a3771268 Mon Sep 17 00:00:00 2001 From: Jeff Squyres Date: Sun, 31 Aug 2008 19:31:10 +0000 Subject: [PATCH] Fixes trac:1236, #1237. * Various changes to enable 0-dimensional cartesian communicators: * Set various mtc_* members to NULL when there are 0 dimensions (and don't bother trying to memcpy these arrays when duplicating the communicator -- because they're NULL) * adjust topo_base_cart_sub to correctly handle 0 dimensions (simplified it a bit) * adjust a few error codes to return ERR_OUT_OF_RESOURCE * adjust error checking of CART_CREATE, CART_RANK * Allow MPI_GRAPH_CREATE to accept 0 == nnodes. * Bump reported MPI version in mpi.h to 2.1 This commit was SVN r19461. The following Trac tickets were found above: Ticket 1236 --> https://svn.open-mpi.org/trac/ompi/ticket/1236 --- NEWS | 3 ++ ompi/communicator/comm.c | 66 ++++++++++++++--------- ompi/include/mpi.h.in | 2 +- ompi/mca/topo/base/topo_base_cart_sub.c | 70 +++++++++++-------------- ompi/mpi/c/cart_create.c | 8 +-- ompi/mpi/c/cart_rank.c | 7 ++- ompi/mpi/c/graph_create.c | 14 ++++- 7 files changed, 97 insertions(+), 73 deletions(-) diff --git a/NEWS b/NEWS index 0e1d10e5c6..2ea70bdce5 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,9 @@ Trunk (not on release branches yet) 1.3 (not released yet) --- +- Increased MPI_SUBVERSION value in mpi.h to 1 (i.e., MPI 2.1) +- Changed behavior of MPI_GRAPH_CREATE, MPI_TOPO_CREATE, and several + other topology functions per MPI-2.1. - Fix the type of the C++ constant MPI::IN_PLACE. - Various enhancements to the openib BTL: - Added btl_openib_if_[in|ex]clude MCA parameters for diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c index 438dacda1c..0adab96fc8 100644 --- a/ompi/communicator/comm.c +++ b/ompi/communicator/comm.c @@ -10,7 +10,7 @@ * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. * Copyright (c) 2007-2008 University of Houston. All rights reserved. - * Copyright (c) 2007 Cisco, Inc. All rights reserved. + * Copyright (c) 2007-2008 Cisco, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -1387,21 +1387,24 @@ int ompi_topo_create (ompi_communicator_t *old_comm, new_comm->c_flags |= cart_or_graph; new_comm->c_topo_comm->mtc_ndims_or_nnodes = ndims_or_nnodes; - new_comm->c_topo_comm->mtc_dims_or_index = NULL; new_comm->c_topo_comm->mtc_periods_or_edges = NULL; new_comm->c_topo_comm->mtc_reorder = reorder; - new_comm->c_topo_comm->mtc_coords = NULL; - new_comm->c_topo_comm->mtc_dims_or_index = (int *)malloc (sizeof(int) * ndims_or_nnodes); - if (NULL == new_comm->c_topo_comm->mtc_dims_or_index) { - ompi_comm_free (&new_comm); - *comm_topo = new_comm; - return OMPI_ERROR; + /* MPI-2.1 allows 0-dimension cartesian communicators, so prevent + a 0-byte malloc -- leave dims_or_index as NULL */ + if (!(OMPI_COMM_CART == cart_or_graph && 0 == ndims_or_nnodes)) { + new_comm->c_topo_comm->mtc_dims_or_index = + (int *) malloc(sizeof(int) * ndims_or_nnodes); + if (NULL == new_comm->c_topo_comm->mtc_dims_or_index) { + ompi_comm_free (&new_comm); + *comm_topo = new_comm; + return OMPI_ERROR; + } + memcpy (new_comm->c_topo_comm->mtc_dims_or_index, + dims_or_index, ndims_or_nnodes * sizeof(int)); } - memcpy (new_comm->c_topo_comm->mtc_dims_or_index, - dims_or_index, ndims_or_nnodes * sizeof(int)); /* Now the topology component has been selected, let the component * re-arrange the proc ranks if need be. This is a down-call into @@ -1445,21 +1448,24 @@ int ompi_topo_create (ompi_communicator_t *old_comm, * structure. The base component functions are free to change * it as they deem fit */ - new_comm->c_topo_comm->mtc_periods_or_edges = - (int*) malloc (sizeof(int) * ndims_or_nnodes); - if (NULL == new_comm->c_topo_comm->mtc_periods_or_edges) { - ompi_comm_free (&new_comm); - *comm_topo = new_comm; - return OMPI_ERROR; - } - memcpy (new_comm->c_topo_comm->mtc_periods_or_edges, - periods_or_edges, ndims_or_nnodes * sizeof(int)); + if (ndims_or_nnodes > 0) { + new_comm->c_topo_comm->mtc_periods_or_edges = + (int*) malloc(sizeof(int) * ndims_or_nnodes); + if (NULL == new_comm->c_topo_comm->mtc_periods_or_edges) { + ompi_comm_free (&new_comm); + *comm_topo = new_comm; + return OMPI_ERR_OUT_OF_RESOURCE; + } + memcpy (new_comm->c_topo_comm->mtc_periods_or_edges, + periods_or_edges, ndims_or_nnodes * sizeof(int)); - new_comm->c_topo_comm->mtc_coords = (int *)malloc (sizeof(int) * ndims_or_nnodes); - if (NULL == new_comm->c_topo_comm->mtc_coords) { - ompi_comm_free (&new_comm); - *comm_topo = new_comm; - return OMPI_ERROR; + new_comm->c_topo_comm->mtc_coords = + (int *) malloc(sizeof(int) * ndims_or_nnodes); + if (NULL == new_comm->c_topo_comm->mtc_coords) { + ompi_comm_free (&new_comm); + *comm_topo = new_comm; + return OMPI_ERR_OUT_OF_RESOURCE; + } } if (OMPI_SUCCESS != @@ -1645,14 +1651,22 @@ static int ompi_comm_copy_topo (ompi_communicator_t *oldcomm, /* pointers for the rest of the information have been set up.... simply allocate enough space and copy all the - information from the previous one */ + information from the previous one. Remember that MPI-2.1 + allows for 0-dimensional Cartesian communicators. */ newt->mtc_ndims_or_nnodes = oldt->mtc_ndims_or_nnodes; newt->mtc_reorder = oldt->mtc_reorder; + if (OMPI_COMM_IS_CART(oldcomm) && 0 == oldt->mtc_ndims_or_nnodes) { + newt->mtc_dims_or_index = NULL; + newt->mtc_periods_or_edges = NULL; + newt->mtc_coords = NULL; + return MPI_SUCCESS; + } + newt->mtc_dims_or_index = (int *)malloc(sizeof(int) * newt->mtc_ndims_or_nnodes); if (NULL == newt->mtc_dims_or_index) { - return OMPI_ERROR; + return OMPI_ERR_OUT_OF_RESOURCE; } memcpy (newt->mtc_dims_or_index, oldt->mtc_dims_or_index, newt->mtc_ndims_or_nnodes * sizeof(int)); diff --git a/ompi/include/mpi.h.in b/ompi/include/mpi.h.in index c86a72d370..218e6e3dee 100644 --- a/ompi/include/mpi.h.in +++ b/ompi/include/mpi.h.in @@ -176,7 +176,7 @@ * MPI version */ #define MPI_VERSION 2 -#define MPI_SUBVERSION 0 +#define MPI_SUBVERSION 1 /* * To accomodate programs written for MPI implementations that use a diff --git a/ompi/mca/topo/base/topo_base_cart_sub.c b/ompi/mca/topo/base/topo_base_cart_sub.c index bd5ea3f0c1..eb734409ed 100644 --- a/ompi/mca/topo/base/topo_base_cart_sub.c +++ b/ompi/mca/topo/base/topo_base_cart_sub.c @@ -50,7 +50,6 @@ int mca_topo_base_cart_sub (MPI_Comm comm, int rank; int ndim; int dim; - bool allfalse; int i; int *d; int *c; @@ -65,7 +64,6 @@ int mca_topo_base_cart_sub (MPI_Comm comm, colour = key = 0; colfactor = keyfactor = 1; ndim = 0; - allfalse = false; i = comm->c_topo_comm->mtc_ndims_or_nnodes - 1; d = comm->c_topo_comm->mtc_dims_or_index + i; @@ -83,15 +81,11 @@ int mca_topo_base_cart_sub (MPI_Comm comm, keyfactor *= dim; } } - /* - * Special case: if all of remain_dims were false, we need to make - * a cartesian communicator with just ourselves in it (you can't - * have a communicator unless you're in it). - */ - if (ndim == 0) { - colour = ompi_comm_rank (comm); - ndim = 1; - allfalse = true; + /* Special case: if all of remain_dims were false, we need to make + a 0-dimension cartesian communicator with just ourselves in it + (you can't have a communicator unless you're in it). */ + if (0 == ndim) { + colour = ompi_comm_rank (comm); } /* * Split the communicator. @@ -107,33 +101,33 @@ int mca_topo_base_cart_sub (MPI_Comm comm, temp_comm->c_topo_comm->mtc_ndims_or_nnodes = ndim; - if (!allfalse) { - p = temp_comm->c_topo_comm->mtc_dims_or_index; - d = comm->c_topo_comm->mtc_dims_or_index; - r = remain_dims; - for (i = 0; i < comm->c_topo_comm->mtc_ndims_or_nnodes; ++i, ++d, ++r) { - if (*r) { - *p++ = *d; - } - } - } else { - temp_comm->c_topo_comm->mtc_dims_or_index[0] = 1; - } - /* - * Compute the caller's coordinates. - */ - rank = ompi_comm_rank (temp_comm); - if (MPI_SUCCESS != errcode) { - OBJ_RELEASE(temp_comm); - return errcode; - } - errcode = temp_comm->c_topo->topo_cart_coords (temp_comm, rank, - ndim, temp_comm->c_topo_comm->mtc_coords); - if (MPI_SUCCESS != errcode) { - OBJ_RELEASE(temp_comm); - return errcode; - } - } + if (ndim >= 1) { + p = temp_comm->c_topo_comm->mtc_dims_or_index; + d = comm->c_topo_comm->mtc_dims_or_index; + r = remain_dims; + for (i = 0; i < comm->c_topo_comm->mtc_ndims_or_nnodes; + ++i, ++d, ++r) { + if (*r) { + *p++ = *d; + } + } + + /* + * Compute the caller's coordinates, if ndims>0 + */ + rank = ompi_comm_rank (temp_comm); + if (MPI_SUCCESS != errcode) { + OBJ_RELEASE(temp_comm); + return errcode; + } + errcode = temp_comm->c_topo->topo_cart_coords (temp_comm, rank, + ndim, temp_comm->c_topo_comm->mtc_coords); + if (MPI_SUCCESS != errcode) { + OBJ_RELEASE(temp_comm); + return errcode; + } + } + } *new_comm = temp_comm; return MPI_SUCCESS; diff --git a/ompi/mpi/c/cart_create.c b/ompi/mpi/c/cart_create.c index ca41ea7b0e..9aaf776d7a 100644 --- a/ompi/mpi/c/cart_create.c +++ b/ompi/mpi/c/cart_create.c @@ -9,7 +9,7 @@ * University of Stuttgart. All rights reserved. * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. - * Copyright (c) 2007 Cisco Systems, Inc. All rights reserved. + * Copyright (c) 2007-2008 Cisco Systems, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -56,11 +56,11 @@ int MPI_Cart_create(MPI_Comm old_comm, int ndims, int *dims, return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_COMM, FUNC_NAME); } - if (1 > ndims) { + if (ndims < 0) { return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_ARG, FUNC_NAME); - } - if (NULL == dims || NULL == periods || NULL == comm_cart) { + } else if (ndims >= 1 && + (NULL == dims || NULL == periods || NULL == comm_cart)) { return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_ARG, FUNC_NAME); } diff --git a/ompi/mpi/c/cart_rank.c b/ompi/mpi/c/cart_rank.c index ab8be632e0..8e82724d67 100644 --- a/ompi/mpi/c/cart_rank.c +++ b/ompi/mpi/c/cart_rank.c @@ -9,7 +9,7 @@ * University of Stuttgart. All rights reserved. * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. - * Copyright (c) 2007 Cisco Systems, Inc. All rights reserved. + * Copyright (c) 2007-2008 Cisco Systems, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -58,7 +58,10 @@ int MPI_Cart_rank(MPI_Comm comm, int *coords, int *rank) return OMPI_ERRHANDLER_INVOKE (comm, MPI_ERR_TOPOLOGY, FUNC_NAME); } - if ((NULL == coords) || (NULL == rank)){ + /* Per MPI-2.1, coords is only relevant if the dimension of + the cartesian comm is >0 */ + if ((NULL == coords && comm->c_topo_comm->mtc_ndims_or_nnodes >= 1) || + (NULL == rank)){ return OMPI_ERRHANDLER_INVOKE (comm, MPI_ERR_ARG, FUNC_NAME); } diff --git a/ompi/mpi/c/graph_create.c b/ompi/mpi/c/graph_create.c index f59b9ceb4e..af8346182e 100644 --- a/ompi/mpi/c/graph_create.c +++ b/ompi/mpi/c/graph_create.c @@ -9,7 +9,7 @@ * University of Stuttgart. All rights reserved. * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. - * Copyright (c) 2007 Cisco, Inc. All rights reserved. + * Copyright (c) 2007-2008 Cisco, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -57,7 +57,10 @@ int MPI_Graph_create(MPI_Comm old_comm, int nnodes, int *index, return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_COMM, FUNC_NAME); } - if ( (1 > nnodes) || (NULL == index) || (NULL == edges) ) { + if (nnodes < 0) { + return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_ARG, + FUNC_NAME); + } else if (nnodes >= 1 && ((NULL == index) || (NULL == edges))) { return OMPI_ERRHANDLER_INVOKE (old_comm, MPI_ERR_ARG, FUNC_NAME); } @@ -73,6 +76,13 @@ int MPI_Graph_create(MPI_Comm old_comm, int nnodes, int *index, } } + /* MPI-2.1 7.5.3 states that if nnodes == 0, all processes should + get MPI_COMM_NULL */ + if (0 == nnodes) { + *comm_graph = MPI_COMM_NULL; + return MPI_SUCCESS; + } + /* * Now we have to check if the topo module exists or not. This has been * removed from initialization since most of the MPI calls do not use