MPI-4: Add support for MPI Sessions #9097

hppritcha · 2021-06-24T16:29:29Z

The PR contains changes to Open MPI to support the MPI 4 Sessions API (see chapter 11 sections 3 and 4, and chapter 7 sections 3.2 and 4.2, and parts of chapter 9 concerning errorhandlers). The MPI 4 standard is available at https://www.mpi-forum.org/docs/mpi-4.0/mpi40-report.pdf.

A set of sessions tests has been added to the OMPI public tests repo:https://github.com/open-mpi/ompi-tests-public.
A set of issues concerning this PR are at https://github.com/hpc/ompi/issues

Major changes associated with this PR are:

replacement of a single ompi/opal core initialize/finalize with a notion of instances and use of destructor callbacks to clean up rather than relying on an instance finalize procedure tightly coupled with the instance initialization procedure
use of an extended CID (exCID) concept for handling Communicator IDs (this is conditional depending on whether a PML supports this). The OB1 and CM/OFI PML's have been modified as part of this PR to support exCIDs.
support for calling functions included in Table 11.1 of the MPI 4 standard before calling MPI_Init and friends and after calling MPI_Finalize and friends.

A paper describing the prototype (at least at an early stage) is available at https://ieeexplore.ieee.org/abstract/document/8891002

There will be extensive squashing of commits prior to merging so it would probably be best to focus on the changes introduced by this PR as a whole rather than reviewing individual commits.

examples/ring_c.c

bosilca · 2021-06-26T21:07:18Z

ompi/attribute/attribute.c

@@ -406,13 +411,24 @@ typedef struct attribute_value_t {
    int av_sequence;
 } attribute_value_t;

+/*


Why is this necessary ? As far as I see there is still a single global object to handle the attributes ?

Looks like it was done to use opal-object reference counting. The last one to tear down attributes calls the destructor. At some point Open MPI should really stop using globals and and this change is a step in the right direction.

Im not arguing against avoiding globals (where and when it makes sense), but such a change deserve its own PR instead of being part of a very large one.

No argument there. Up to @hppritcha to decide whether to move it to another PR.

ompi/attribute/attribute.c

bosilca · 2021-06-26T21:18:13Z

ompi/communicator/comm.c

@@ -198,15 +207,45 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm,
        }

        newcomm->c_flags |= OMPI_COMM_INTER;
+        newcomm->c_index_vec = malloc(remote_size * sizeof(int));


This is one sneaky change, with drastic implications in terms of latency, scalability and memory usage. It need to be carefully assessed and discusses before being merged.

yes agreed. There's an issue to address this in a different way - hpc#65

bosilca · 2021-06-26T21:24:40Z

ompi/communicator/comm.c

@@ -2032,3 +2404,28 @@ static int ompi_comm_copy_topo(ompi_communicator_t *oldcomm,
    newcomm->c_flags |= newcomm->c_topo->type;
    return OMPI_SUCCESS;
 }
+
+char *ompi_comm_print_cid (const ompi_communicator_t *comm)


This thread-local 2 correct values function begs for documentation !

Indeed. I thought I documented this somewhere.... Guess not. Should add documentation to the header declaration.

bosilca · 2021-06-26T21:27:13Z

ompi/communicator/comm_cid.c

+        break;
+    }
+
+    PMIX_INFO_LOAD(&pinfo, PMIX_GROUP_ASSIGN_CONTEXT_ID, NULL, PMIX_BOOL);


How scalable is the construction of the PMIx group ? What process is calling this function ?

It uses the same PRRTE communication collective as the "modex" operation. All processes that are to be members of the group must call it. It basically is a data-less "modex" (aka barrier) that returns a unique context ID.

In other words, slightly less scalable than MPI collectives on the local costs (because of the need to construct and destruct the group), then delegated to a different software that uses some communication capabilities, to finally be delegated to a RM with or without efficient support for collectives. Hopefully this CID selection is only rarely used and most calls are outside the critical path. Right ?

bosilca · 2021-06-26T21:32:01Z

ompi/communicator/communicator.h


-OMPI_DECLSPEC extern opal_pointer_array_t ompi_mpi_communicators;
+OMPI_DECLSPEC extern opal_hash_table_t ompi_comm_hash;
+OMPI_DECLSPEC extern opal_pointer_array_t ompi_comm_array;


Why the name change ? The old name was clear enough.

@hjelmn any reason not to go back to ompi_mpi_communicators?

I can't remember why I changed the name. There probably was a reason at one point during the evolution of this branch. Most likely I was making the name consistent with ompi_comm_hash to make it clear that one is an array of communicators where the lookup is by index where the other one is a hash where the lookup is by exCID.

If we remove this name change, the PR is getting more self-contained (aka. smaller and easier to understand).

switched back to the old variable name

ibm-ompi · 2021-06-28T15:13:01Z

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

Gist: https://gist.github.com/ibm-ompi/88b480fb98dfd2bdac939f023e1f8fe6

ibm-ompi · 2021-06-28T15:13:22Z

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

Gist: https://gist.github.com/ibm-ompi/88b480fb98dfd2bdac939f023e1f8fe6

hppritcha · 2021-06-28T15:35:27Z

bot:ibm:retest

ibm-ompi · 2021-06-28T15:39:00Z

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

Gist: https://gist.github.com/ibm-ompi/88b480fb98dfd2bdac939f023e1f8fe6

ibm-ompi · 2021-06-28T15:40:06Z

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

Gist: https://gist.github.com/ibm-ompi/88b480fb98dfd2bdac939f023e1f8fe6

ibm-ompi · 2021-06-28T15:42:36Z

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

Gist: https://gist.github.com/ibm-ompi/88b480fb98dfd2bdac939f023e1f8fe6

hppritcha · 2021-06-28T17:26:22Z

bot:ibm:retest

ibm-ompi · 2021-07-01T19:35:05Z

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

Gist: https://gist.github.com/2175efaa13b971835186c294806b0001

ibm-ompi · 2021-07-01T19:37:38Z

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

Gist: https://gist.github.com/61a0a9029bfafb0c95f43f60957a65d5

ibm-ompi · 2021-07-01T19:39:36Z

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

Gist: https://gist.github.com/52e26384a669f512dad96ba55ceaa81f

hppritcha · 2021-07-02T02:18:32Z

hmm aws seems to have gone bonkers rerun that first
bot:aws:retest

hppritcha · 2021-07-02T02:29:36Z

I suspect the git commit checker is experiencing some kind of scalability melt down. will ignore going forward with this PR. @jsquyres

jsquyres · 2021-07-02T15:03:55Z

I suspect the git commit checker is experiencing some kind of scalability melt down. will ignore going forward with this PR. @jsquyres

It looks like the git commit checker is not running at all. I've seen this before -- rarely, but I have seen it. I haven't been able to figure out the root cause. Perhaps it doesn't run if there's a conflict on the PR...?

ibm-ompi · 2021-07-14T20:05:25Z

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

Gist: https://gist.github.com/c6fe74035df169e07a5eb531a01cf04d

ibm-ompi · 2021-07-14T20:08:04Z

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

Gist: https://gist.github.com/77e50f876685171598bd552c135adaf2

ibm-ompi · 2021-07-14T20:10:05Z

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

Gist: https://gist.github.com/358a6ff4065920a662e408d5032d02e7

hppritcha · 2021-08-17T18:41:41Z

Leaving all the commits as is for now. Will squash down to a much smaller set of commits once this PR is reviewd.

ibm-ompi · 2021-12-23T21:04:25Z

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

Gist: https://gist.github.com/5993530c5eb1eb4702834c6595aef513

dependencies array. If this is not done, then when using a non-ob1 PML, the stages of finalization have the SMSC framework closed before the btl framework is closed or del_procs invoked for the btl(s). When using the --enable-mca_dso this results in a segfault as the code section for the SMSC framework/components has been dlclosed. Moving the opal_smc_base_framework from the opal_init_frameworks to the ompi_framework_dependencies fixes this problem. Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit ec21585)

Fix a memory leak in ofi mtl add_comm method. Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit 3626d64)

when MPI_Session_create_errhandler blows up. Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit e514ce1)

more complex test cases for sessions and world model showed a problem when creating sessions before calling mpi_init. This patch addresses this problem. Related to PR open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit 4848109)

related to open-mpi#9097 test https://github.com/open-mpi/ompi-tests-public/blob/master/sessions/sessions_test16.c now passes. Signed-off-by: Howard Pritchard <howardp@lanl.gov>

related to open-mpi#9097 test https://github.com/open-mpi/ompi-tests-public/blob/master/sessions/sessions_test16.c now passes. Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit 01c2233)

related to open-mpi#9097 test https://github.com/open-mpi/ompi-tests-public/blob/master/sessions/sessions_test16.c now passes. Signed-off-by: Howard Pritchard <howardp@lanl.gov>

somehow managed to not get into the original sessions pr. also do some cleanup of the use mpi_f08 profiling functions. Related to open-mpi#10388 Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

comms. related to open-mpi#10449 related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

The sessions attribute functionality was never approved as part of the MPI-4 sessions feature. Remove a vestige of this from Open MPI. Related to open-mpi#10388 Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

comms. related to open-mpi#10449 related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit bfffe99)

The sessions attribute functionality was never approved as part of the MPI-4 sessions feature. Remove a vestige of this from Open MPI. Related to open-mpi#10388 Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit c488c60)

somehow managed to not get into the original sessions pr. also do some cleanup of the use mpi_f08 profiling functions. Related to open-mpi#10388 Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

somehow managed to not get into the original sessions pr. also do some cleanup of the use mpi_f08 profiling functions. Related to open-mpi#10388 Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit e5c3795)

hppritcha marked this pull request as draft June 24, 2021 16:30

hppritcha added MPI-4.0 Target: main labels Jun 24, 2021

bosilca reviewed Jun 26, 2021

View reviewed changes

hppritcha force-pushed the sessions_pr branch from d3d54f8 to be2d2dd Compare July 6, 2021 16:26

hppritcha force-pushed the sessions_pr branch from 5b945fb to 44df849 Compare August 4, 2021 21:29

hppritcha marked this pull request as ready for review August 17, 2021 18:41

hppritcha added the Target: v5.0.x label Nov 19, 2021

hppritcha force-pushed the sessions_pr branch from 47ebff4 to 7f8dcd4 Compare November 20, 2021 22:08

hppritcha force-pushed the sessions_pr branch 2 times, most recently from 0639ea7 to b229b61 Compare December 23, 2021 20:41

gpaulsen pushed a commit to hppritcha/ompi that referenced this pull request Mar 31, 2022

sessions: invoke error handlers

b665d0f

when MPI_Session_create_errhandler blows up. Related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit e514ce1)

hppritcha mentioned this pull request May 5, 2022

sessions: support MPI_TAG_UB #10359

Merged

dalcinl mentioned this pull request May 25, 2022

Performance issues when using oversubscription #10426

Closed

hppritcha mentioned this pull request Jun 2, 2022

sessions: add missing errhandler funcs #10442

Merged

hppritcha added a commit to hppritcha/ompi that referenced this pull request Jun 21, 2022

sessions: initialize instance field for world and self

bfffe99

comms. related to open-mpi#10449 related to open-mpi#9097 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

hppritcha mentioned this pull request Jun 21, 2022

sessions: initialize instance field for world and self #10495

Merged

hppritcha mentioned this pull request Jun 21, 2022

sessions: remove sessions attr del function def #10499

Merged

hppritcha mentioned this pull request Jun 22, 2022

sessions: initialize instance field for world and self #10502

Merged

hppritcha mentioned this pull request Jun 22, 2022

sessions: remove sessions attr del function def #10503

Merged

hppritcha mentioned this pull request Jul 11, 2022

sessions: add missing errhandler funcs #10556

Merged

jywangx mentioned this pull request Dec 8, 2022

Does ompi_comm_split_type actually pass the "info" to the newly created "newcomm"? #11181

Closed

lrbison mentioned this pull request Feb 2, 2023

Fixing VERSION file for v5.0.0rc10 #11365

Merged

jsquyres added the 😳 Backward compat break label Feb 2, 2023

jsquyres mentioned this pull request Feb 2, 2023

Move PREDEFINED_COMMUNICATOR_PAD back to 512 #11373

Closed

MPI-4: Add support for MPI Sessions #9097

MPI-4: Add support for MPI Sessions #9097

Conversation

hppritcha commented Jun 24, 2021 • edited by jsquyres Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bosilca Jun 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibm-ompi commented Jun 28, 2021

ibm-ompi commented Jun 28, 2021

hppritcha commented Jun 28, 2021

ibm-ompi commented Jun 28, 2021

ibm-ompi commented Jun 28, 2021

ibm-ompi commented Jun 28, 2021

hppritcha commented Jun 28, 2021

ibm-ompi commented Jul 1, 2021

ibm-ompi commented Jul 1, 2021

ibm-ompi commented Jul 1, 2021

hppritcha commented Jul 2, 2021

hppritcha commented Jul 2, 2021

jsquyres commented Jul 2, 2021

ibm-ompi commented Jul 14, 2021

ibm-ompi commented Jul 14, 2021

ibm-ompi commented Jul 14, 2021

hppritcha commented Aug 17, 2021

ibm-ompi commented Dec 23, 2021

hppritcha commented Jun 24, 2021 •

edited by jsquyres

Loading

bosilca Jun 30, 2021 •

edited

Loading