process set query: if pmix_query_info_nb returns error

[hppritcha]

don't wait for call back to be run as it will not be per section 5.4.4 of PMIx 4.1 standard.

related to #10749

Signed-off-by: Howard Pritchard [email protected]

[hppritcha]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[dalcinl]

@hppritcha Could you please rebase this PR to latest main?

[dalcinl]

@hppritcha When running in singleton init mode, MPI_Group_from_session_pset now fails with error class MPI_ERR_OTHER. Perhaps MPI_ERR_UNSUPPORTED_OPERATION would be more appropriate and in line with other recent changes from you?

[hppritcha]

@dalcinl rebased

[hppritcha]

i'll check out improving the error class returned in the singleton case

[bwbarrett]

bot:aws:retest

[dalcinl]

@hppritcha I'm not sure what to say. mpi4py's singleton, np=1, and np=2 are passing. However, for np=3, I got a failure in a test case that basically calls MPI_Comm_dup and MPI_Comm_free, here you have the test logs.

I don't think these failures are related to this PR, I also run my mpi4py@update/MPI4-openmpi branch against ompi@main, and I got the same failure again, although this time for np=5. So this looks like during the last week a new regression slipped in. Any idea who we can ask for help to figure out what could be de problem without having to manually bisect the last week worth of commits in the main branch? Maybe @jsquyres has a global picture of recent changes that may have affected MPI_Comm_dup + MPI_Comm_free?

[hppritcha]

bot:aws:retest

[hppritcha]

@dalcinl I'm not able to reproduce what I think you're testing with this simple 'c' code:

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

int main(int argc, char *argv[])
{
    int rank, size, len;
    char version[MPI_MAX_LIBRARY_VERSION_STRING];
    MPI_Comm my_self_dup;

    MPI_Init(&argc, &argv);
    MPI_Comm_rank(MPI_COMM_WORLD, &rank);
    MPI_Comm_size(MPI_COMM_WORLD, &size);
    MPI_Get_library_version(version, &len);
    printf("Hello, world, I am %d of %d, (%s, %d)\n", rank, size, version, len);
    MPI_Comm_dup(MPI_COMM_SELF, &my_self_dup);
    printf("Hello, dupped comm self\n");
    MPI_Comm_free(&my_self_dup);
    if (my_self_dup != MPI_COMM_NULL) {
	    fprintf(stderr, "hmm my_self_dup mt set to NULL\n");
    }
    MPI_Finalize();

    return 0;
}

is this effectively what your testCloneFree is doing?

[dalcinl]

Yes, but it could be that previous tests have a side effect that end up breaking the dup+free. Remember that mpi4py tests run within a single invocation of mpiexec. And note the failure is perhaps not easy to reproduce, it happened with np=3 for your branch, but np=5 for main. So something shady may be going on.

[dalcinl]

Oh, hold on. My test uses the world comm.

[hppritcha]

I modified the test to use MPI_COMM_WORLD and still cannot reproduce the problem that you are hitting.

[dalcinl]

I modified the test to use MPI_COMM_WORLD and still cannot reproduce the problem that you are hitting.

I'll have to bisect all the changes by hand using GitHub Actions. Not fun, but I don't see other way around.

[dalcinl]

@hppritcha I believe I got confused. The issue actually happens with MPI_Comm_create_from_group using the main branch (see the [logs]), I paste the output below:

======================================================================
ERROR: testCreateFromGroup (test_comm.TestCommSelf)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/mpi4py-testing/mpi4py-testing/mpi4py/test/test_comm.py", line 160, in testCreateFromGroup
    comm = MPI.Intracomm.Create_from_group(group)
  File "mpi4py/MPI/Comm.pyx", line 2048, in mpi4py.MPI.Intracomm.Create_from_group
mpi4py.MPI.Exception: MPI_ERR_UNKNOWN: unknown error

Looks like I'll have to disable this test in the mean time.

[dalcinl]

@hppritcha After disabling tests for MPI_Comm_create_from_group, all tests are green with this PR. This is good to merge from my side.

However, we now have this new issue of MPI_Comm_create_from_group being broken. Do you guys have any regression tests to ensure this new MPI-4 routine keeps working? mpi4py have the most basic testing for it, we got these tests passing, but now they are broken. Or perhaps I got confused and my tests never really worked with ompi@main (or worked just by accident).

Should I open a new issue for MPI_Comm_create_from_group? @jjhursey @awlauria Are you guys aware of any changes that may have affected this routine?

[hppritcha]

I am confused. I tried this little C test and it seems to work:

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

int main(int argc, char *argv[])
{
    int rank, size, len;
    char version[MPI_MAX_LIBRARY_VERSION_STRING];
    MPI_Group self_group;
    MPI_Comm comm_self;

    MPI_Init(&argc, &argv);
    MPI_Comm_rank(MPI_COMM_WORLD, &rank);
    MPI_Comm_size(MPI_COMM_WORLD, &size);
    MPI_Get_library_version(version, &len);
    printf("Hello, world, I am %d of %d, (%s, %d)\n", rank, size, version, len);
    MPI_Comm_group(MPI_COMM_SELF,&self_group);
    MPI_Comm_create_from_group(self_group, "my_self_group", MPI_INFO_NULL, MPI_ERRORS_RETURN, &comm_self);
    MPI_Group_free(&self_group);
    MPI_Comm_free(&comm_self);
    MPI_Finalize();

    return 0;
}

I have a PR open on our private test repo with some sessions tests. I have to say there's less overhead with generating these tests using python though!

[hppritcha]

hmm..looks like the python test is tripping up on the comm compare. investigating that.

[dalcinl]

looks like the python test is tripping up on the comm compare

Good catch! This mean that the way I wrote the test is not proper. I fixed it. Next I fired a test run to double-check my changes and verify your claim.

[hppritcha]

something else seems to be going on with this self with multiple ranks test. If each rank uses a unique tag argument in the call to MPI_Comm_create_from_group then I don't get a problem with the comm compare operation which i added to the code snippet above. Not sure if this may be some pmix related behavior or not.

[rhc54]

Not sure if this may be some pmix related behavior or not

Can you clarify this a bit for me? I'm not entirely sure I follow the above discussion, so it isn't clear to me what pmix behavior you are asking about.

[hppritcha]

okay it turns out that this python test is hitting an edge case concerning the use of the tag argument to MPI_Comm_create_from_group and the only difference being the list of underlying pmix_proc's being supplied to PMIx_Group_construct. The MPI-4 standard is a little ambiguous about the extent to which the tag argument needs to be unique. I will generate a patch to fix this.

The behavior is intermittent because the error is actually coming from the PMIx_Group_destruct call, and depending on timing an error may not be returned, esp. for lower process counts.

[hppritcha]

@dalcinl could you open a new issue to track this specific problem - supplying a non-unique tag to concurrent calls to MPI_Comm_create_from_group with non-overlapping group arguments?

[hppritcha]

@gpaulsen please review this Pr. there are now numerous threads going on here that don't have anything to do with what this Pr is suppose to address.

[rhc54]

The behavior is intermittent because the error is actually coming from the PMIx_Group_destruct call, and depending on timing an error may not be returned, esp. for lower process counts.

@hppritcha I modified PMIx to always sort the proc arrays being passed to avoid ordering issues, but I may have missed some places. Is that the issue here? If so, I'm wondering if I just need to add the ordering code to the construct and destruct APIs.

[hppritcha]

I doubt it has to do with ordering of the proc array. The way dalcini's test case works - using the group from MPI_COMM_SELF to create a communicator congruent to MPI_COMM_SELF, and using the same tag argument for MPI_Comm_create_from_group, ends up with us making calls to PMIx_Group_construct with the same grp argument string but just different proc arrays (of size 1).

I don't think this is legal based on the wording in PMIx standard.

[rhc54]

I don't think this is legal based on the wording in PMIx standard.

Definitely not, I'm afraid.

[dalcinl]

When running in singleton init mode, MPI_Group_from_session_pset now fails with error class MPI_ERR_OTHER. Perhaps MPI_ERR_UNSUPPORTED_OPERATION would be more appropriate and in line with other recent changes from you?

i'll check out improving the error class returned in the singleton case

@hppritcha Could you look into this?

[hppritcha]

I looked in to this more and I think the current error code being returned is more accurate owing to this verbiage about MPI_Group_from_session_pset in the MPI 4 standard:

The function MPI_GROUP_FROM_SESSION_PSET creates a group newgroup using the
provided session handle and process set. The process set name must be one returned from
an invocation of MPI_SESSION_GET_NTH_PSET using the supplied session handle.