Add global comm by hgangwar · Pull Request #341 · SCOREC/pcms

hgangwar · 2026-07-13T16:30:59Z

Moving PR #228 to here for a clean PR.

This PR introduces redev::AdiosGlobalComm to provide a global communication interface between the coupler and the applications using the existing ADIOS channel.
Usage:

auto GDI = app->AddData<pcms::GO>("global_comm", comm);

This creates a global communicator that can send and receive data, for example:

app->BeginSendPhase();
GDI->Send(mean, "mean");
app->EndSendPhase();

app->BeginReceivePhase();
GDI->Receive(mean, "mean");
app->EndReceivePhase();

The test_GDI test case verifies global communication and data transfer.

jacobmerson · 2026-07-20T21:20:54Z

@hgangwar did you update this based on the redev merge? Note, you will need to make sure you bump the redev version in the config files for the CI.

jacobmerson · 2026-07-21T14:20:35Z

@hgangwar I think we discussed having this owned by the application so we have a consistent API between fields and "global data"

AddField -> AddData
SendField -> SendData
ReceiveField -> ReceiveData

jacobmerson

See previous comment about making the API match between fields and global data.

jacobmerson

Sorry I did not read carefully, you did add the global data communicator to the application. However for consistency:

SendData should be a function on the application that takes a string
Send should be a function on the GDI pointer.

jacobmerson · 2026-07-21T15:44:20Z

Also, look at how the FieldHandle works. I.e., we return a cheap to copy handle of the field which stores a string which is used to actually perform the send/receive. That way we are not handing a pointer back to the user.

jacobmerson

Don't have time to finish this review right now...but I want to go over again why we need to pass the data directly into SendData I understand at the redev level this is needed for the communictor because otherwise we will be constructing a lot of adios2 objects. However, it's less clear why that's needed here. It is possible that it's the correct choice, just the mismatch to how the fields are handled is giving me pause.

You may want to discuss with Sichao or Cameron or Fuad. Show them the general API and see which direction seems more natural to them.

jacobmerson · 2026-07-24T22:52:18Z

+            redev::Mode mode = redev::Mode::Synchronous) const;
+
+  [[nodiscard]] std::vector<T> Receive(
+    std::string variable_name, std::size_t msg_size,


Shouldn't this API be consistent with Send i.e., you should pass in the appropriate array_view which gets filled up. otherwise, a buffer needs to be created on the heap for every call to recieve.

Made the changes, can you please take a look again?

jacobmerson · 2026-07-24T22:52:31Z

+  {
+  }
+
+  void Send(T* msg, std::string variable_name, std::size_t msg_size,


This should take an array_view instead of a pointer and a size.

Same as above.

jacobmerson · 2026-07-24T22:53:21Z

+  template <typename T>
+  DataHandle<T> AddData(std::string name, MPI_Comm mpi_comm);
+  template <typename T>
+  [[nodiscard]] GlobalDataInterface<T>& GetDataInterface(


Should this return the handle we discussed?

AddData is returning a DataHandle, this is for DataHandle to be able return GlobalDataInterface object ptr. This is non-essential for now, so I have removed it.

Copilot

Pull request overview

This PR adds a new “global data” communication path built on the existing Redev/ADIOS channel, enabling coupler/applications to exchange arbitrary scalar arrays via a named global communicator, and wires in a new multi-process test to validate the flow.

Changes:

Introduce pcms::GlobalCommunicator<T> / GlobalDataInterface<T> plus Application::AddData() and DataHandle<T> to send/receive global data during communication phases.
Add test_GDI and CTest wiring to run a 3-process (coupler + 2 apps) global-comm integration test.
Update CI workflows to pin a newer redev commit and install the new header.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/pcms/coupler/global_communicator.h`	Adds a thin wrapper over Redev global comm creation and send/recv operations.
`src/pcms/coupler/coupler.hpp`	Exposes the new global data interface on `Application` and adds a handle-based API for callers.
`src/pcms/coupler/CMakeLists.txt`	Installs the new global communicator header.
`test/test_GDI.cpp`	New 3-role integration test exercising global send/receive across coupler/apps.
`test/CMakeLists.txt`	Registers `test_GDI` and adjusts unit test source/link configuration.
`.github/workflows/self-hosted.yml`	Updates the pinned `redev` commit for self-hosted CI.
`.github/workflows/cmake-test.yml`	Updates the pinned `redev` commit for cmake-test CI.
`.github/workflows/clang-tidy.yml`	Updates the pinned `redev` commit for clang-tidy CI.

Comments suppressed due to low confidence (3)

test/CMakeLists.txt:124

NAME3 is also set to "app", which will overwrite NAME1's log output (both write to app.log). Use a unique name for this third process (e.g., client0).

          NAME3
          app
          EXE3
          ./test_GDI

test/test_GDI.cpp:49

mean_buffer/mean are typed as long, but this data handle is AddDatapcms::GO. Using pcms::GO for the buffer/view avoids template type mismatches on platforms where int64_t is not long.

  Kokkos::View<long*, pcms::HostMemorySpace> mean_buffer("mean_buffer", 1);

  pcms::Rank1View<long, pcms::HostMemorySpace> mean{mean_buffer.data(),
                                                    mean_buffer.extent(0)};

test/test_GDI.cpp:84

mean_buffer/mean are typed as long, but the global comm is instantiated as AddDatapcms::GO. Use pcms::GO here as well to keep the test type-correct across platforms.

  Kokkos::View<long*, pcms::HostMemorySpace> mean_buffer("mean_buffer", 1);

  pcms::Rank1View<long, pcms::HostMemorySpace> mean{mean_buffer.data(),
                                                    mean_buffer.extent(0)};

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+          NAME1
+          app
+          EXE1
+          ./test_GDI


      APPEND
      PCMS_UNIT_TEST_SOURCES
      test_error_handling.cpp
-      test_eqdsk.cpp
      test_uniform_grid.cpp
      test_field_evaluation.cpp


-    pcms_transfer
-  )
-
 target_include_directories(unit_tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})


+#include <redev.h>
+#include <pcms/utility/profile.h>


+#include <Omega_h_mesh.hpp>
+#include <Omega_h_file.hpp>
+#include "test_support.h"
+#include "pcms/coupler/coupler.hpp"
+#include <pcms/utility/types.h>
+


+      app->BeginReceivePhase();
+      GDI.Receive(mean, "mean");
+      app->EndReceivePhase();
+      printf("total Recieved mean:%ld\n", mean[0]);


+      delta_f->EndReceivePhase();
+      printf("delta Received mean:%ld\n", mean[0]);
+      mean[0] = mean[0] / 2;
+      const auto msg_size = mean.size();


+      total_f->BeginReceivePhase();
+      GDI_total.Receive(mean, "mean");
+      total_f->EndReceivePhase();
+      printf("delta Received mean:%ld\n", mean[0]);
+      mean[0] = mean[0] / 2;


+  void Send(Rank1View<T, pcms::HostMemorySpace> msg, std::string variable_name,
+            redev::Mode mode = redev::Mode::Synchronous) const;
+
+  void Receive(Rank1View<T, pcms::HostMemorySpace> msg,
+               std::string variable_name,


+  Kokkos::View<long*, pcms::HostMemorySpace> mean_buffer("mean_buffer", 1);
+
+  pcms::Rank1View<long, pcms::HostMemorySpace> mean{mean_buffer.data(),
+                                                    mean_buffer.extent(0)};


hgangwar added 2 commits July 13, 2026 12:10

add_global_comm_header

2f7318d

add_test_case

313df5e

hgangwar requested a deployment to asics July 13, 2026 16:31 — with GitHub Actions Waiting

hgangwar added 2 commits July 20, 2026 18:43

format_test

e8cd7cd

update_redev_ref

dcbc7f3

hgangwar requested a deployment to asics July 20, 2026 23:16 — with GitHub Actions Waiting

format

f197296

hgangwar requested a deployment to asics July 20, 2026 23:18 — with GitHub Actions Waiting

hgangwar added 2 commits July 20, 2026 19:29

fix_warnings

29984dc

clang-tidy-redev_update

22d089e

hgangwar requested a deployment to asics July 20, 2026 23:34 — with GitHub Actions Waiting

CI_update

3761c83

hgangwar temporarily deployed to asics July 20, 2026 23:52 — with GitHub Actions Inactive

hgangwar marked this pull request as ready for review July 21, 2026 00:05

jacobmerson requested changes Jul 21, 2026

View reviewed changes

jacobmerson reviewed Jul 21, 2026

View reviewed changes

add_DataHandle

e11b082

hgangwar requested a deployment to asics July 21, 2026 20:15 — with GitHub Actions Waiting

clang_format

21f0ce7

hgangwar requested a deployment to asics July 21, 2026 20:20 — with GitHub Actions Waiting

add_try_catch

25671dd

hgangwar requested a deployment to asics July 21, 2026 20:52 — with GitHub Actions Waiting

format

a9a7267

hgangwar requested a deployment to asics July 21, 2026 20:57 — with GitHub Actions Waiting

jacobmerson reviewed Jul 24, 2026

View reviewed changes

use_view_instead_of_ptr_in_GDI_api

b900e4a

hgangwar requested a deployment to asics July 27, 2026 16:38 — with GitHub Actions Waiting

format

e8b82ed

hgangwar requested a deployment to asics July 27, 2026 16:39 — with GitHub Actions Waiting

fix_test_GDI

6d2e846

hgangwar requested a deployment to asics July 27, 2026 17:46 — with GitHub Actions Waiting

tidy_fix

d192a2e

hgangwar requested a deployment to asics July 27, 2026 18:04 — with GitHub Actions Waiting

removed_GetDataInterface_from_DataHandle

0d5a622

hgangwar requested a deployment to asics July 27, 2026 19:42 — with GitHub Actions Waiting

hgangwar requested review from Fuad-HH and Copilot July 27, 2026 19:44

Copilot started reviewing on behalf of hgangwar July 28, 2026 14:51 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

		#include <redev.h>
		#include <pcms/utility/profile.h>

Uh oh!

Conversation

hgangwar commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobmerson commented Jul 20, 2026

Uh oh!

jacobmerson commented Jul 21, 2026

Uh oh!

jacobmerson left a comment

Choose a reason for hiding this comment

Uh oh!

jacobmerson left a comment

Choose a reason for hiding this comment

Uh oh!

jacobmerson commented Jul 21, 2026

Uh oh!

jacobmerson left a comment

Choose a reason for hiding this comment

Uh oh!

jacobmerson Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

hgangwar Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

jacobmerson Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

hgangwar Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

jacobmerson Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

hgangwar Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hgangwar commented Jul 13, 2026 •

edited

Loading