Adds distributed row gatherer #1589

MarcelKoch · 2024-04-04T10:49:19Z

This PR adds a distributed row gatherer. This operator essentially provides the communication required in our matrix apply.

Besides the normal apply (which is blocking), it also provides two asynchronous calls. One version has an additional workspace parameter which is used as send buffer. This version can be called multiple times without restrictions, if different workspaces are used for each call. The other version doesn't have a workspace parameter, and instead uses an internal buffer. As a consequence, this function can only be called a second time, if the request of the previous call has been waited on. Otherwise, this function will throw.

This is the second part of splitting up #1546.

It also introduces some intermediate changes, which could be extracted out beforehand:

~~a type-erased DenseCache~~
~~makingdetail::run easier to use~~ now part of Use index_map in distributed::matrix #1544

PR Stack:

MarcelKoch · 2024-04-22T08:29:49Z

One issue that I have is the constructor. It takes a collective_communicator and an index_map. The index_map already defines the communication pattern, so the collective_communicator has to match that.
One option might be to have a virtual function like

std::unique_ptr<collective_communicator> create_with_same_type(communicator, index_map);

If I can't come up with anything better, I guess I will use that.

pratikvn · 2024-04-26T09:50:57Z

Do we need to have the std::future setup for the release ? Can we remove that for now and just use a normal synchronous approach ? I think that is a significant change that maybe needs more thought and probably a separate PR.

pratikvn

Some comments regarding synchronizations.

core/distributed/row_gatherer.cpp

pratikvn

Really nice work! LGTM!

pratikvn · 2024-07-23T07:59:57Z

core/distributed/row_gatherer.cpp

+namespace distributed {
+
+
+#if GINKGO_HAVE_OPENMPI_POST_4_1_X


I think this should be PRE and the other way around ? I dont think there is a macro like this defined.

pratikvn · 2024-07-23T08:01:34Z

core/distributed/row_gatherer.cpp

+    int is_inactive;
+    MPI_Status status;
+    GKO_ASSERT_NO_MPI_ERRORS(
+        MPI_Request_get_status(req_listener_, &is_inactive, &status));


Can we maybe move this MPI function into mpi.hpp and create a wrapper for it ?

pratikvn · 2024-07-23T08:08:09Z

include/ginkgo/core/distributed/row_gatherer.hpp

+
+    mutable array<char> send_workspace_;
+
+    mutable MPI_Request req_listener_{MPI_REQUEST_NULL};


This can be of type mpi::request ?

pratikvn · 2024-07-23T09:10:12Z

core/distributed/row_gatherer.cpp

+template <typename LocalIndexType>
+void RowGatherer<LocalIndexType>::apply_impl(const LinOp* alpha, const LinOp* b,
+                                             const LinOp* beta, LinOp* x) const
+    GKO_NOT_IMPLEMENTED;


I think you can also implement the advanced apply by replacing b_local->row_gather(idxs, buffer) by b_local->row_gather(alpha, idxs, beta, buffer) ?

- only allocate if necessary - synchronize correct executor Co-authored-by: Pratik Nayak <pratik.nayak@kit.edu>

MarcelKoch self-assigned this Apr 4, 2024

ginkgo-bot added reg:build This is related to the build system. reg:testing This is related to testing. mod:core This is related to the core module. type:matrix-format This is related to the Matrix formats labels Apr 4, 2024

MarcelKoch requested a review from pratikvn April 4, 2024 10:49

MarcelKoch force-pushed the distributed-row-gatherer branch from 6b4521b to ae60198 Compare April 4, 2024 11:00

MarcelKoch force-pushed the neighborhood-communicator branch from 6acf7c4 to 8aa6ab9 Compare April 4, 2024 11:00

MarcelKoch force-pushed the distributed-row-gatherer branch 2 times, most recently from 49557f1 to 4a79442 Compare April 5, 2024 08:18

MarcelKoch modified the milestone: Ginkgo 1.8.0 Apr 5, 2024

MarcelKoch force-pushed the neighborhood-communicator branch from 8aa6ab9 to 77398bd Compare April 17, 2024 16:28

MarcelKoch force-pushed the distributed-row-gatherer branch from 4a79442 to 172eb7d Compare April 17, 2024 16:28

MarcelKoch requested a review from upsj April 19, 2024 09:20

MarcelKoch mentioned this pull request Apr 19, 2024

Adds sparse communicator class #1546

Closed

7 tasks

MarcelKoch force-pushed the neighborhood-communicator branch from 77398bd to d278cad Compare April 19, 2024 14:39

MarcelKoch force-pushed the distributed-row-gatherer branch 2 times, most recently from 98fa10a to 79de4c3 Compare April 19, 2024 16:19

This was referenced Apr 22, 2024

Add segmented array type #1545

Merged

Distributed Index Map #1543

Merged

Adds Index map device kernels #1579

Merged

Use index_map in distributed::matrix #1544

Merged

MarcelKoch force-pushed the distributed-row-gatherer branch from 79de4c3 to b0e5c92 Compare April 22, 2024 11:11

MarcelKoch force-pushed the neighborhood-communicator branch from d278cad to d6112ef Compare April 22, 2024 11:11

MarcelKoch force-pushed the distributed-row-gatherer branch from b0e5c92 to 775854a Compare April 25, 2024 07:16

MarcelKoch force-pushed the neighborhood-communicator branch from d6112ef to 1582673 Compare April 25, 2024 07:16

MarcelKoch force-pushed the distributed-row-gatherer branch 2 times, most recently from 8ec2b02 to 58d06ed Compare July 10, 2024 13:03

MarcelKoch force-pushed the neighborhood-communicator branch from e0ef53e to 8d566aa Compare July 10, 2024 13:03

pratikvn reviewed Jul 15, 2024

View reviewed changes

core/distributed/row_gatherer.cpp Outdated Show resolved Hide resolved

core/distributed/row_gatherer.cpp Outdated Show resolved Hide resolved

core/distributed/row_gatherer.cpp Outdated Show resolved Hide resolved

MarcelKoch force-pushed the distributed-row-gatherer branch from 58d06ed to dec99fb Compare July 16, 2024 07:37

MarcelKoch force-pushed the neighborhood-communicator branch 2 times, most recently from c9d5643 to 8e3e932 Compare July 16, 2024 07:43

MarcelKoch force-pushed the distributed-row-gatherer branch from dec99fb to a58baba Compare July 16, 2024 07:43

MarcelKoch requested a review from pratikvn July 16, 2024 07:43

MarcelKoch force-pushed the neighborhood-communicator branch from 8e3e932 to 40cd2c0 Compare July 18, 2024 16:39

MarcelKoch force-pushed the distributed-row-gatherer branch 2 times, most recently from 98dcc4f to 5e970e9 Compare July 18, 2024 17:00

MarcelKoch force-pushed the neighborhood-communicator branch from 40cd2c0 to fe864bb Compare July 18, 2024 17:00

MarcelKoch force-pushed the distributed-row-gatherer branch from 5e970e9 to 0a8e28c Compare July 19, 2024 08:18

MarcelKoch force-pushed the neighborhood-communicator branch from fe864bb to db7f6ed Compare July 19, 2024 08:18

pratikvn approved these changes Jul 23, 2024

View reviewed changes

MarcelKoch force-pushed the distributed-row-gatherer branch from 0a8e28c to bfc5233 Compare August 9, 2024 13:40

MarcelKoch force-pushed the neighborhood-communicator branch from db7f6ed to 0ad4ee8 Compare August 9, 2024 13:40

MarcelKoch and others added 8 commits August 16, 2024 17:21

[dist-rg] adds distributed row-gatherer

f75b61f

[dist-rg] handle copy to host buffer

01d9c56

[dist-rg] use mpi request instead of future

49dfd97

[dist-rg] adds distributed row-gatherer tests

3f550af

[dist-mat] use row-gatherer

03d350d

[pgm] use row-gatherer from matrix

67e8321

[core] enable array conversion between ints

31374e9

[dist-rg] review update:

8697971

- only allocate if necessary - synchronize correct executor Co-authored-by: Pratik Nayak <pratik.nayak@kit.edu>

MarcelKoch force-pushed the distributed-row-gatherer branch from bfc5233 to 8697971 Compare August 16, 2024 15:21

MarcelKoch force-pushed the neighborhood-communicator branch from 0ad4ee8 to 1f49b91 Compare August 16, 2024 15:21

MarcelKoch requested review from upsj and removed request for upsj August 27, 2024 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds distributed row gatherer #1589

Adds distributed row gatherer #1589

MarcelKoch commented Apr 4, 2024 •

edited

Loading

MarcelKoch commented Apr 22, 2024

pratikvn commented Apr 26, 2024

pratikvn left a comment

pratikvn left a comment

pratikvn Jul 23, 2024

pratikvn Jul 23, 2024

pratikvn Jul 23, 2024

pratikvn Jul 23, 2024


		mutable array<char> send_workspace_;

		mutable MPI_Request req_listener_{MPI_REQUEST_NULL};

		namespace distributed {


		#if GINKGO_HAVE_OPENMPI_POST_4_1_X

Adds distributed row gatherer #1589

Are you sure you want to change the base?

Adds distributed row gatherer #1589

Conversation

MarcelKoch commented Apr 4, 2024 • edited Loading

MarcelKoch commented Apr 22, 2024

pratikvn commented Apr 26, 2024

pratikvn left a comment

Choose a reason for hiding this comment

pratikvn left a comment

Choose a reason for hiding this comment

pratikvn Jul 23, 2024

Choose a reason for hiding this comment

pratikvn Jul 23, 2024

Choose a reason for hiding this comment

pratikvn Jul 23, 2024

Choose a reason for hiding this comment

pratikvn Jul 23, 2024

Choose a reason for hiding this comment

MarcelKoch commented Apr 4, 2024 •

edited

Loading