MPI4: implement mpi_isendrecv and variant #9667

hppritcha · 2021-11-16T00:47:06Z

Signed-off-by: Howard Pritchard howardp@lanl.gov

ggouaillardet · 2021-11-16T04:07:11Z

@hppritcha at first glance, that looks good to me.

I am just wondering if adding a ompi_status_public_t *statuses[OMPI_COMM_REQUEST_MAX_SUBREQ] field to ompi_comm_request_item_t is the best strategy (my main concern is memory overhead).
Could we simple not use this field and let the status attached to the subrequest?
Then at MPI_Wait() time, the status will be copied from the subrequest to the main request.

I did not check the full specification, but if for example irecv() works but isend() has an error, should we "return" the error status of isend() instead of the successful status of irecv()?

bosilca · 2021-11-16T07:10:59Z

Unfortunately what error to return is as well defined as for MPI_Sendrecv. But from a logical perspective it makes more sense to return the status of the receive part of the communication, as the status for a receive contains more information than the send. Plus, what really matters for the send is that the buffer can be modified.

ompi/mpi/c/isendrecv.c

ompi/mpi/c/isendrecv_replace.c

ompi/communicator/comm_request.c

ompi/mpi/c/isendrecv_replace.c

ompi/mpi/c/isendrecv.c

jsquyres

Similar to #9707, I think this PR is missing the mpi module bindings (both tkr and ignore-tkr).

ompi/mpi/c/profile/Makefile.am

ompi/mpi/fortran/mpif-h/isendrecv_f.c

ompi/mpi/fortran/use-mpi-f08/bindings/mpi-f-interfaces-bind.h

jsquyres

Looks good! Can you squash?

all points have been addressed

Signed-off-by: Howard Pritchard <howardp@lanl.gov>

kawashima-fj

@hppritcha I'm sorry I' late. I added additional comments.

kawashima-fj · 2022-01-26T15:22:05Z

ompi/runtime/ompi_spc.h

+    OMPI_SPC_ISENDRECV,
+    OMPI_SPC_ISENDRECV_REPLACE,


Comment of this enum (line 26-33) says:

/* INSTRUCTIONS FOR ADDING COUNTERS * 1.) Add a new counter name in the ompi_spc_counters_t enum before * OMPI_SPC_NUM_COUNTERS below. * 2.) Add corresponding counter name and descriptions to the * counter_names and counter_descriptions arrays in * ompi_spc.c NOTE: The names and descriptions * MUST be in the same array location as where you added the * counter name in step 1.

Though I don't know detail of SPC, according to this comment, you need to insert lines in the same position in ompi_spc_counters in ompi_spc.h and ompi_spc_events_desc in ompi_spc.c

@kawashima-fj thanks for noting this. i'll open a PR to patch this.

kawashima-fj · 2022-01-26T15:23:59Z

ompi/mpi/c/isendrecv_replace.c

+    OMPI_COPY_STATUS(&request->super.req_status,
+                     context->subreq[0]->req_status, false);


Probably you forgot to erase these two lines when you inserted line 91-97.

kawashima-fj · 2022-01-26T15:28:34Z

ompi/mpi/man/man3/MPI_Isendrecv_replace.3in

+.nf
+USE MPI
+! or the older form: INCLUDE 'mpif.h'
+MPI_SENDRECV_REPLACE(\fIBUF, COUNT, DATATYPE, DEST, SENDTAG, SOURCE,


Missing I before SENDRECV.

thanks I'll file a PR to fix this too.

see review comments for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

per review feedback for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

see review comments for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit 1babb92)

per review feedback for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit b2183d3)

hppritcha requested review from kawashima-fj and ggouaillardet November 16, 2021 02:16

bosilca reviewed Nov 16, 2021

View reviewed changes

ompi/mpi/c/isendrecv.c Outdated Show resolved Hide resolved

ompi/mpi/c/isendrecv.c Outdated Show resolved Hide resolved

ompi/mpi/c/isendrecv_replace.c Outdated Show resolved Hide resolved

hppritcha force-pushed the topic/add_isendrecv_variants branch from b0416f3 to 608abd5 Compare November 21, 2021 00:54

kawashima-fj previously requested changes Nov 27, 2021

View reviewed changes

hppritcha force-pushed the topic/add_isendrecv_variants branch from 608abd5 to 509b87e Compare November 29, 2021 17:51

hppritcha force-pushed the topic/add_isendrecv_variants branch from 509b87e to 8e976a1 Compare December 21, 2021 20:14

hppritcha added Target: main Target: v5.0.x MPI-4.0 labels Dec 21, 2021

hppritcha requested review from jsquyres and removed request for ggouaillardet January 11, 2022 16:31

jsquyres requested review from kawashima-fj and bosilca January 19, 2022 01:11

jsquyres requested changes Jan 19, 2022

View reviewed changes

hppritcha force-pushed the topic/add_isendrecv_variants branch from b4c3b89 to 669cd10 Compare January 19, 2022 19:42

bosilca approved these changes Jan 19, 2022

View reviewed changes

hppritcha requested review from jsquyres and removed request for kawashima-fj January 19, 2022 21:02

jsquyres approved these changes Jan 23, 2022

View reviewed changes

hppritcha requested review from kawashima-fj and removed request for kawashima-fj January 24, 2022 17:26

MPI4: implement mpi_isendrecv and variant

47c8063

Signed-off-by: Howard Pritchard <howardp@lanl.gov>

hppritcha force-pushed the topic/add_isendrecv_variants branch from 669cd10 to 47c8063 Compare January 25, 2022 16:08

hppritcha merged commit c7c24e4 into open-mpi:master Jan 25, 2022

kawashima-fj reviewed Jan 26, 2022

View reviewed changes

kawashima-fj mentioned this pull request Jan 26, 2022

MPI4: implement mpi_isendrecv and variant #9926

Merged

hppritcha added a commit to hppritcha/ompi that referenced this pull request Jan 26, 2022

spc: fix up an update for isendrecv, etc.

1babb92

see review comments for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

hppritcha mentioned this pull request Jan 26, 2022

spc: fix up an update for isendrecv, etc. #9930

Merged

hppritcha added a commit to hppritcha/ompi that referenced this pull request Jan 26, 2022

isendrecv_rplace: fix problem with req status

b2183d3

per review feedback for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov>

hppritcha mentioned this pull request Jan 26, 2022

isendrecv_rplace: fix problem with req status #9931

Merged

hppritcha added a commit to hppritcha/ompi that referenced this pull request Jan 31, 2022

spc: fix up an update for isendrecv, etc.

0a2a4f1

see review comments for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit 1babb92)

hppritcha mentioned this pull request Jan 31, 2022

spc: fix up an update for isendrecv, etc. #9948

Merged

hppritcha added a commit to hppritcha/ompi that referenced this pull request Jan 31, 2022

isendrecv_rplace: fix problem with req status

6bc571f

per review feedback for PR open-mpi#9667 Signed-off-by: Howard Pritchard <howardp@lanl.gov> (cherry picked from commit b2183d3)

hppritcha mentioned this pull request Jan 31, 2022

isendrecv_rplace: fix problem with req status #9949

Merged

		OMPI_COPY_STATUS(&request->super.req_status,
		context->subreq[0]->req_status, false);

MPI4: implement mpi_isendrecv and variant #9667

MPI4: implement mpi_isendrecv and variant #9667

Uh oh!

Conversation

hppritcha commented Nov 16, 2021

Uh oh!

ggouaillardet commented Nov 16, 2021

Uh oh!

bosilca commented Nov 16, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsquyres left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsquyres left a comment

Choose a reason for hiding this comment

Uh oh!

kawashima-fj left a comment

Choose a reason for hiding this comment

Uh oh!

kawashima-fj Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

hppritcha Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

kawashima-fj Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

kawashima-fj Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

hppritcha Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!