Skip to content

mpi/neighbor_allgatherv: fix copy&paste error and add helpers #2334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 12, 2017
Merged

mpi/neighbor_allgatherv: fix copy&paste error and add helpers #2334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ompi/mca/topo/base/base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ OMPI_DECLSPEC int
mca_topo_base_dist_graph_neighbors_count(ompi_communicator_t *comm,
int *inneighbors, int *outneighbors, int *weighted);


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have a ompi_comm_neighbors_count subroutine that does the same think (at least at first glance)
that being said, your new subroutine only uses internal subroutine (vs MPI_*) so i like it better.

should we only keep your subroutine ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ompi_comm_neighbors_count using MPI_* interfaces interfere with MPI profiling. I'm in favor of the new routine. Moreover, the new routine should be used everywhere and the former should be removed.

int mca_topo_base_neighbor_count (ompi_communicator_t *comm, int *indegree, int *outdegree);

END_C_DECLS

#endif /* MCA_BASE_TOPO_H */
27 changes: 27 additions & 0 deletions ompi/mca/topo/base/topo_base_frame.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,33 @@ static int mca_topo_base_open(mca_base_open_flag_t flags)
return mca_base_framework_components_open(&ompi_topo_base_framework, flags);
}

int mca_topo_base_neighbor_count (ompi_communicator_t *comm, int *indegree, int *outdegree) {
if (!OMPI_COMM_IS_TOPO(comm)) {
return OMPI_ERR_BAD_PARAM;
}

if (OMPI_COMM_IS_CART(comm)) {
/* cartesian */
/* outdegree is always 2*ndims because we need to iterate over
empty buffers for MPI_PROC_NULL */
*outdegree = *indegree = 2 * comm->c_topo->mtc.cart->ndims;
} else if (OMPI_COMM_IS_GRAPH(comm)) {
/* graph */
int rank, nneighbors;

rank = ompi_comm_rank (comm);
mca_topo_base_graph_neighbors_count (comm, rank, &nneighbors);

*outdegree = *indegree = nneighbors;
} else if (OMPI_COMM_IS_DIST_GRAPH(comm)) {
/* graph */
*indegree = comm->c_topo->mtc.dist_graph->indegree;
*outdegree = comm->c_topo->mtc.dist_graph->outdegree;
}

return OMPI_SUCCESS;
}

MCA_BASE_FRAMEWORK_DECLARE(ompi, topo, "OMPI Topo", NULL,
mca_topo_base_open, mca_topo_base_close,
mca_topo_base_static_components, 0);
Expand Down
35 changes: 7 additions & 28 deletions ompi/mpi/c/neighbor_allgatherv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* All rights reserved.
* Copyright (c) 2010 University of Houston. All rights reserved.
* Copyright (c) 2012 Cisco Systems, Inc. All rights reserved.
* Copyright (c) 2012-2013 Los Alamos National Security, LLC. All rights
* Copyright (c) 2012-2016 Los Alamos National Security, LLC. All rights
* reserved.
* Copyright (c) 2015 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
Expand All @@ -32,6 +32,7 @@
#include "ompi/communicator/communicator.h"
#include "ompi/errhandler/errhandler.h"
#include "ompi/datatype/ompi_datatype.h"
#include "ompi/mca/topo/base/base.h"
#include "ompi/memchecker.h"
#include "ompi/mca/topo/topo.h"
#include "ompi/mca/topo/base/base.h"
Expand All @@ -50,20 +51,20 @@ int MPI_Neighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype sen
void *recvbuf, const int recvcounts[], const int displs[],
MPI_Datatype recvtype, MPI_Comm comm)
{
int i, size, err;
int in_size, out_size, err;

MEMCHECKER(
int rank;
ptrdiff_t ext;

rank = ompi_comm_rank(comm);
size = ompi_comm_size(comm);
mca_topo_base_neighbor_count (comm, &in_size, &out_size);
ompi_datatype_type_extent(recvtype, &ext);

memchecker_datatype(recvtype);
memchecker_comm (comm);
/* check whether the receive buffer is addressable. */
for (i = 0; i < size; i++) {
for (int i = 0; i < in_size; ++i) {
memchecker_call(&opal_memchecker_base_isaddressable,
(char *)(recvbuf)+displs[i]*ext,
recvcounts[i], recvtype);
Expand Down Expand Up @@ -107,8 +108,8 @@ int MPI_Neighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype sen
get the size of the remote group here for both intra- and
intercommunicators */

size = ompi_comm_remote_size(comm);
for (i = 0; i < size; ++i) {
mca_topo_base_neighbor_count (comm, &in_size, &out_size);
for (int i = 0; i < in_size; ++i) {
if (recvcounts[i] < 0) {
return OMPI_ERRHANDLER_INVOKE(comm, MPI_ERR_COUNT, FUNC_NAME);
}
Expand Down Expand Up @@ -141,27 +142,6 @@ int MPI_Neighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype sen
}
}

/* Do we need to do anything? Everyone had to give the same
signature, which means that everyone must have given a
sum(recvounts) > 0 if there's anything to do. */

if ( OMPI_COMM_IS_INTRA( comm) ) {
for (i = 0; i < ompi_comm_size(comm); ++i) {
if (0 != recvcounts[i]) {
break;
}
}
if (i >= ompi_comm_size(comm)) {
return MPI_SUCCESS;
}
}
/* There is no rule that can be applied for inter-communicators, since
recvcount(s)=0 only indicates that the processes in the other group
do not send anything, sendcount=0 only indicates that I do not send
anything. However, other processes in my group might very well send
something */


OPAL_CR_ENTER_LIBRARY();

/* Invoke the coll component to perform the back-end operation */
Expand All @@ -170,4 +150,3 @@ int MPI_Neighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype sen
recvtype, comm, comm->c_coll->coll_neighbor_allgatherv_module);
OMPI_ERRHANDLER_RETURN(err, comm, err, FUNC_NAME);
}