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/basic: fix MPI_Alltoallw(MPI_IN_PLACE) gap handling #9330

Merged
merged 5 commits into from
Oct 7, 2021

Conversation

ggouaillardet
Copy link
Contributor

The temporary buffer must be shifted by the true_extent on a
per type basis (since the various datatypes might have different
true_extent).

Thanks Heiko Bauke for reporting this.

Refs. #9329

Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp

The temporary buffer must be shifted by the true_extent on a
per type basis (since the various datatypes might have different
true_extent).

Thanks Heiko Bauke for reporting this.

Refs. open-mpi#9329

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
@jjhursey
Copy link
Member

bot:ibm:retest

@open-mpi open-mpi deleted a comment from ibm-ompi Aug 30, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Aug 30, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Aug 30, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Aug 30, 2021
@jjhursey
Copy link
Member

bot:ibm:retest

This function can be used to compute the packed size of a datatype on a
target architecture.

Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
Dont copy the datatype into a buffer with the same extent, but instead
pack it and send it to the peer as packed.

Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
@bosilca
Copy link
Member

bosilca commented Aug 31, 2021

A more memory-aware approach. I still don't like the double loop in the call, but I haven't had enough time to figure out a better solution yet. Once we agree we should apply the same patch to all in_place functions.

Provide optimized variant for the homogeneous case.

Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
@bosilca
Copy link
Member

bosilca commented Oct 6, 2021

@jsquyres and @bwbarrett we need this in the 5.0

@jsquyres
Copy link
Member

jsquyres commented Oct 7, 2021

@bosilca Is there a reason you don't review/approve this PR? You're far more qualified to review it than me or @bwbarrett...

@jsquyres
Copy link
Member

jsquyres commented Oct 7, 2021

@bosilca Never mind; I see you added commits beyond @ggouaillardet's original commits. Let me at least prove that this PR fixes the code posted in the original #9329 issue.

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.

I have confirmed that this PR fixes the sample code in #9329 (which was ported to ompi-tests/ibm/collective/alltoallw_in_place2.c), and that with a cursory code review, it looks ok.

@jsquyres jsquyres merged commit a30056c into open-mpi:master Oct 7, 2021
@jsquyres
Copy link
Member

jsquyres commented Oct 7, 2021

@bosilca I tried to cherry pick these commits to v5.0.x and failed -- there's some conflicts. ☹️

Are there some fixes that should be over on v5.0.x that aren't there (such that these fixes would apply cleanly)?

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.

6 participants