Skip to content
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.

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

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

Merged
merged 2 commits into from
Jul 21, 2017

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Jul 12, 2017

This commit adds a helper function to get the inbound and outbound
neighbor count and updates the neighbor_allgatherv bindings to use the
correct count when checking the input parameters.

Fixes #2324

Signed-off-by: Nathan Hjelm [email protected]
(cherry picked from commit 3c0e94a)
Signed-off-by: Nathan Hjelm [email protected]

@hjelmn hjelmn added the bug label Jul 12, 2017
@hjelmn hjelmn added this to the v3.0.0 milestone Jul 12, 2017
@hjelmn
Copy link
Member Author

hjelmn commented Jul 12, 2017

Punt to 3.0.1 if needed. Do not let this hold anything up.

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

Choose a reason for hiding this comment

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

you could use mca_topo_base_dist_graph_neighbors_count() instead

more generally speaking, is there any reason why you invoke these base functions directly instead of using the callbacks
(for example module->topo.dist_graph.dist_graph_neighbors_count())

@hppritcha
Copy link
Member

@hjelmn have you and @ggouaillardet come to a consensus on this? we'd like to get this in today or latest tomorrow morning in time for an rc2.

@hjelmn
Copy link
Member Author

hjelmn commented Jul 18, 2017

@hppritcha Yes. I need to add one more commit to this then it is good to go.

hjelmn and others added 2 commits July 18, 2017 12:09
This commit adds a helper function to get the inbound and outbound
neighbor count and updates the neighbor_allgatherv bindings to use the
correct count when checking the input parameters.

Fixes open-mpi#2324

Signed-off-by: Nathan Hjelm <[email protected]>
(cherry picked from commit 3c0e94a)
Signed-off-by: Nathan Hjelm <[email protected]>
This commit removes the communicator topo helper functions in favor
of functions in mca/topo/base.

Signed-off-by: Nathan Hjelm <[email protected]>
(cherry picked from commit 9b702fb)
Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn
Copy link
Member Author

hjelmn commented Jul 18, 2017

ok, should be good now.

@jjhursey
Copy link
Member

bot:ibm:pgi:retest
bot:ibm:xl:retest

@open-mpi open-mpi deleted a comment from ibm-ompi Jul 18, 2017
@open-mpi open-mpi deleted a comment from ibm-ompi Jul 18, 2017
@hppritcha
Copy link
Member

Sign off checker seems to be confused. Maybe because the commits are doubly signed.

@hppritcha
Copy link
Member

@ggouaillardet please recheck

Copy link
Contributor

@ggouaillardet ggouaillardet left a comment

Choose a reason for hiding this comment

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

my minor comment was not addressed, but that should not block this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants