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

Regression: MPI_Win_start()/complete() with MPI_GROUP_EMPTY #8677

Closed
dalcinl opened this issue Mar 22, 2021 · 11 comments · Fixed by #8779
Closed

Regression: MPI_Win_start()/complete() with MPI_GROUP_EMPTY #8677

dalcinl opened this issue Mar 22, 2021 · 11 comments · Fixed by #8779
Assignees

Comments

@dalcinl
Copy link
Contributor

dalcinl commented Mar 22, 2021

I don't consider myself an expert on OSC, but the following code used to work across MPI implementations. Now it is failing with current master.

Reproducer

from mpi4py import MPI  # Implies MPI_Init_thread() with MPI_THREAD_MULTIPLE

buf = MPI.Alloc_mem(1024)
win = MPI.Win.Create(buf, 1, MPI.INFO_NULL, MPI.COMM_SELF)
win.Start(MPI.GROUP_EMPTY)
win.Complete()
win.Free()
MPI.Free_mem(buf)

Output:

Traceback (most recent call last):
  File "/tmp/rma.py", line 5, in <module>
    win.Complete()
  File "mpi4py/MPI/Win.pyx", line 592, in mpi4py.MPI.Win.Complete
mpi4py.MPI.Exception: MPI_ERR_RMA_SYNC: error executing rma sync
@jsquyres
Copy link
Member

@hjelmn Does this code look correct?

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 22, 2021

Note that the following code runs to completion. If Post() with the empty group works, then I would argue that Start() with the empty group should also work.

from mpi4py import MPI
win = MPI.Win.Create(MPI.BOTTOM, 1, MPI.INFO_NULL, MPI.COMM_SELF)
win.Post(MPI.GROUP_EMPTY)
win.Wait()
win.Free()

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 22, 2021

@jsquyres I know the test looks a bit silly. Those are the silly things and corner cases I test for. I get what I deserve.

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 22, 2021

@hjelmn Does this code look correct?

An MPI expert always needs to double check with a colleague about OSC semantics 🤣.

@bosilca
Copy link
Member

bosilca commented Mar 22, 2021

All these version look legit.

@hjelmn
Copy link
Member

hjelmn commented Mar 22, 2021

While odd that should work as I understand the standard.

@hjelmn
Copy link
Member

hjelmn commented Mar 22, 2021

When fixed we should add both a C (if it is missing) and fortran version of this to ompi-tests to cover.

@jsquyres
Copy link
Member

An MPI expert always needs to double check with a colleague about OSC semantics 🤣.

I stay far, far away from all MPI one-sided things. 😃

@hjelmn
Copy link
Member

hjelmn commented Mar 22, 2021

Why? One-sided is fast communication without the slowness that is MPI two-sided semantics 😁 . I mean, seriously. The no-overtaking rule is explicit deleted. We kind of fixed that flaw using communicator info keys but it is still the default.

@jsquyres
Copy link
Member

This is how I think of MPI one-sided:

explain-network

@awlauria
Copy link
Contributor

awlauria commented Apr 1, 2021

I addressed this in #8756 for osc/rdma.

The sync type wasn't being set, causing the SYNC error.
Also there was a couple issues freeing an empty group, the attached basic test runs clean for me now.

#include<mpi.h>
#include<stdio.h>
#include<stdlib.h>

int main() {
    MPI_Win win;
    char *buf;
    int rank;

    MPI_Init(0,0);
    MPI_Comm_rank(MPI_COMM_WORLD, &rank);
    MPI_Group comm_group, group;

    if (rank == 0) {
        MPI_Win_create(buf,sizeof(int)*3,sizeof(int),
            MPI_INFO_NULL,MPI_COMM_WORLD,&win);
    }
    else {
        MPI_Win_create(NULL,0,sizeof(int), MPI_INFO_NULL,MPI_COMM_WORLD,&win);
        MPI_Win_start(MPI_GROUP_EMPTY,0,win);
        MPI_Win_complete(win);

    }
    /* Free window and groups */
    MPI_Win_free(&win);
    return 0;
}

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

Successfully merging a pull request may close this issue.

5 participants