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

coll/han: fix bug in reduce in place #11800

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

wzamazon
Copy link
Contributor

@wzamazon wzamazon commented Jul 5, 2023

Piror to this patch, the reduce code try to applied arithematic operations on a sendbuf, even when it
is MPI_IN_PLACE. this patch addressed the issue.

@wzamazon
Copy link
Contributor Author

wzamazon commented Jul 5, 2023

Fix #11799

@bosilca please review

Copy link
Contributor

@wenduwan wenduwan left a comment

Choose a reason for hiding this comment

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

Thank you!

@wzamazon wzamazon force-pushed the coll_han_fix_reduce_in_place branch from ce4d807 to 01f552b Compare July 6, 2023 14:26
@jsquyres
Copy link
Member

jsquyres commented Jul 8, 2023

@B-a-S Looks like a failure in the NVIDIA CI environment:

Error from server (NotFound): pods "ompi-ci-1" not found

Can you please investigate?

@wzamazon wzamazon force-pushed the coll_han_fix_reduce_in_place branch 2 times, most recently from da87fdc to dc3b21a Compare July 10, 2023 19:34
ompi/mca/coll/han/coll_han_reduce.c Outdated Show resolved Hide resolved
@wzamazon wzamazon force-pushed the coll_han_fix_reduce_in_place branch from dc3b21a to 93bfdd5 Compare July 10, 2023 20:36
@wzamazon wzamazon requested a review from bwbarrett July 10, 2023 20:39
@jsquyres
Copy link
Member

CI infra issue has been resolved.

@bosilca Can you review?

@wzamazon wzamazon force-pushed the coll_han_fix_reduce_in_place branch from 93bfdd5 to 9ba0637 Compare July 10, 2023 22:11
Piror to this patch, the reduce code try to applied
arithematic operations on a sendbuf, even when it
is MPI_IN_PLACE. this patch addressed the issue.

Signed-off-by: Wei Zhang <wzam@amazon.com>
@wzamazon wzamazon force-pushed the coll_han_fix_reduce_in_place branch from 9ba0637 to d520921 Compare July 12, 2023 13:06
@wzamazon
Copy link
Contributor Author

all CI passed. Ready for merge!

@jsquyres
Copy link
Member

@wzamazon It says you force-pushed an hour ago. What did you push after the PR was approved?

@wzamazon
Copy link
Contributor Author

@wzamazon It says you force-pushed an hour ago. What did you push after the PR was approved?

I rebased to latest main branch (using the github's rebase branch option)

@jsquyres
Copy link
Member

I rebased to latest main branch (using the github's rebase branch option)

Ok. In our community, there's generally no need to do that unless a specific reason for why it's necessary. Otherwise, it just brings up the question "what changed?" 😄

@jsquyres jsquyres merged commit 63a5d03 into open-mpi:main Jul 12, 2023
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