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 #3865

Merged
merged 2 commits into from
Nov 7, 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 hjelmn@lanl.gov
(cherry picked from commit 3c0e94a)
Signed-off-by: Nathan Hjelm hjelmn@me.com

@hjelmn hjelmn added the bug label Jul 12, 2017
@hjelmn hjelmn added this to the v2.0.4 milestone Jul 12, 2017
@hjelmn hjelmn requested a review from ggouaillardet July 12, 2017 19:02
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 <hjelmn@lanl.gov>
(cherry picked from commit 3c0e94a)
Signed-off-by: Nathan Hjelm <hjelmn@me.com>
@ibm-ompi
Copy link

The IBM CI (GNU Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/347c18eb4b0e667faeb05fbd0f1144af

@hjelmn hjelmn force-pushed the v2.0.x_neighbor_fix branch from 96edde5 to e1764ac Compare July 12, 2017 19:28
@ibm-ompi
Copy link

The IBM CI (XL Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/3d6240b77cedd336f6a97f2d1a66d356

@jsquyres
Copy link
Member

jsquyres commented Aug 3, 2017

@ggouaillardet Can you review this PR? Thanks!

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.

@hjelmn do you also need to cherry-pick 9b702fb ?
you did this in #3863 that was merged in v3.0.x

@hppritcha
Copy link
Member

@hjelmn could you respond to @ggouaillardet comment about 9b702fb

@hppritcha
Copy link
Member

@jsquyres should we close this?

@jsquyres
Copy link
Member

I just pinged @hjelmn again. It looks like it's an actual bug fix...?

@hppritcha
Copy link
Member

@hjelmn let's try and fix this. Please check by COB this thursday.

@hppritcha
Copy link
Member

@ggouaillardet please review. we want to get this in for 2.0.4, which we would like to start roll out this week.

@ggouaillardet
Copy link
Contributor

@hppritcha i did already review this PR, and my review was

do you also need to cherry-pick 9b702fb ?
you did this in #3863 that was merged in v3.0.x

if the mentioned commit does not require being cherry-picked, then i am fine with this PR

This commit removes the communicator topo helper functions in favor
of functions in mca/topo/base.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
(cherry picked from commit 9b702fb)
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

@hppritcha pushed in the missing commit

@ibm-ompi
Copy link

ibm-ompi commented Nov 6, 2017

The IBM CI (GNU Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/8bce83bcb32a46bb4ef9b4bc977f7cab

@ompiteam-bot
Copy link

All Tests Passed!

@jsquyres
Copy link
Member

jsquyres commented Nov 6, 2017

IBM Jenkins hit a github API rate limit; Josh was going to let it timeout / get more API credits and then re-kick off the IBM Jenkins.

@ggouaillardet
Copy link
Contributor

fwiw, the github API rate limit is pretty low for unauthenticated requests (and i guess things get even worse if you are behind a large corporate proxy), but the github API rate limit seems very acceptable for authenticated requests.

@jsquyres
Copy link
Member

jsquyres commented Nov 7, 2017

bot:ibm:retest

@jjhursey
Copy link
Member

jjhursey commented Nov 7, 2017

There is something else wrong with the Jenkins server that's causing it to fail out. Hopefully I'll find time this week to fix it. Otherwise...

@jsquyres
Copy link
Member

jsquyres commented Nov 7, 2017

IBM's CI is temporarily down. Merging this PR.

@jsquyres jsquyres merged commit 4d97b2b into open-mpi:v2.0.x Nov 7, 2017
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.

8 participants