Fix P3 to read lookup tables on rank 0 and broadcast to other ranks by ndkeen · Pull Request #8547 · E3SM-Project/E3SM

ndkeen · 2026-07-07T20:21:57Z

Fix P3 microphysics initialization so only the root MPI rank reads the lookup table files from disk and broadcasts the data to the other ranks, instead of every rank independently reading the same files. This avoids unnecessary load
on shared/parallel filesystems and a possible slowdown or stall at scale, as reported in E3SM issues #6654 and #6833.

Fixes #6654 and #6833

[bfb]

ndkeen · 2026-07-07T20:24:00Z

Summary

Change Functions<S,D>::p3_init() so its second parameter is const ekat::Comm* comm instead of const bool masterproc.
In read_ice_lookup_tables() and io_impl() (used by read_computed_tables() / write_computed_tables() for the binary mu_r/vn/vm/revap tables), only the root rank of comm now opens and reads the lookup table files; the data is then broadcast to the rest of comm with ekat::Comm::broadcast().
If comm is null, behavior is unchanged from before: every calling process reads the files independently. This keeps the zero-argument p3_init() call sites (unit tests, p3_run_and_cmp, etc.) working with no changes needed.
P3Microphysics::initialize_impl() (the real EAMxx runtime path) now passes &this->get_comm() so it gets the new root-read-and-broadcast behavior.
p3_tables_setup (the offline table-regeneration tool) now builds a real ekat::Comm around MPI_COMM_WORLD, and calls MPI_Init/MPI_Finalize itself, since it is not built through the shared Catch2 test main() that normally provides this.
read_ice_lookup_tables() and io_impl() now compute do_io (which process(es) touch the filesystem) and do_print (which process(es) print the informational "Reading/Writing..." messages) separately, instead of conflating both
into a single masterproc flag. Previously, masterproc defaulted to true whenever comm was null (since there is no rank to be non-root of), so every zero-argument p3_init() call site (the vast majority of unit tests) starte
d unconditionally printing on every call. Now do_print is only true for a genuine multi-rank root, restoring the pre-existing quiet default when comm is null.
Added EKAT_REQUIRE_MSG checks in io_impl() after opening each binary table file, and after reading/writing it, so a missing or truncated mu_r/vn/vm/revap table file now fails loudly and immediately instead of silently p
ropagating bad/uninitialized data (which, after the read-on-root-and-broadcast change above, would now also propagate to every other rank).

Also fixed: a build break in PAM/CRM

components/eam/src/physics/crm/pam/pam_driver.cpp also calls P3F::p3_init(), passing a bool (am_i_root) as the second argument. That stopped compiling once the parameter type changed to const ekat::Comm* (a bool does not implicitly convert to a pointer type). PAM does not currently plumb an MPI communicator through its coupler, so this call site now passes comm=nullptr, which preserves its exact pre-existing behavior (every rank still reads the lookup tables independently). Plumbing a real communicator through PAM for the same broadcast optimization is left as a follow-up, if desired.

Copilot

Pull request overview

Update EAMxx P3 microphysics initialization to avoid per-rank lookup-table file reads by having the root MPI rank perform the filesystem I/O and broadcasting table data to the other ranks via ekat::Comm. This targets known scalability/filesystem-pressure issues reported in #6654 and #6833.

Changes:

Change p3::Functions::p3_init to accept an optional ekat::Comm* and use it to root-read + broadcast both ice and non-ice lookup tables.
Update the EAMxx P3 process initialization to pass its communicator into p3_init.
Update the offline P3 table-generation tool to construct an ekat::Comm and call the new p3_init signature.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
components/eamxx/src/physics/p3/tests/p3_tables_setup.cpp	Initialize MPI and pass an `ekat::Comm` to `p3_init` for table generation.
components/eamxx/src/physics/p3/p3_functions.hpp	Update `p3_init` API to accept an optional communicator and document root-read + broadcast behavior.
components/eamxx/src/physics/p3/impl/p3_init_impl.hpp	Implement root-rank file I/O and broadcast logic for P3 lookup tables.
components/eamxx/src/physics/p3/eamxx_p3_process_interface.cpp	Pass the atmosphere communicator into `p3_init` so only root reads tables in production runs.
components/eam/src/physics/crm/pam/pam_driver.cpp	Update call to new `p3_init` signature (currently still using `comm=nullptr` in PAM).

ndkeen · 2026-07-07T22:56:43Z

+  const bool masterproc = comm == nullptr || comm->am_i_root();
  if (masterproc) {
    std::cout << "Reading ice lookup tables in file: " << filename << std::endl;
  }

-  std::ifstream in(filename);
-
-  // read header
-  std::string version, version_val;
-  in >> version >> version_val;
-  EKAT_REQUIRE_MSG(version == "VERSION", "Bad " << filename << ", expected VERSION X.Y.Z header");
-  EKAT_REQUIRE_MSG(version_val == p3_version, "Bad " << filename << ", expected version " << p3_version << ", but got " << version_val);
-
-  // read tables
-  double dum_s; int dum_i; // dum_s needs to be double to stream correctly
-  for (int jj = 0; jj < densize; ++jj) {
-    for (int ii = 0; ii < rimsize; ++ii) {
-      for (int i = 0; i < isize; ++i) {
-        in >> dum_i >> dum_i;
-        int j_idx = 0;
-        for (int j = 0; j < 15; ++j) {
-          in >> dum_s;
-          if (j > 1 && j != 10) {
-            ice_table_vals_h(jj, ii, i, j_idx++) = dum_s;
+  // Only the root rank (or every calling process, if comm is null, e.g. in
+  // stand-alone tools/unit tests that don't provide a communicator) touches
+  // the filesystem. The resulting data is then broadcast to the other ranks
+  // in comm below. This avoids every single MPI rank independently reading
+  // the exact same (small, read-only) text file at initialization, which can
+  // put unnecessary stress on shared/parallel filesystems and, in the worst
+  // case, cause a slowdown or stall (see E3SM issue #6654 / #6833).
+  if (masterproc) {
+    std::ifstream in(filename);


Split the old conflated masterproc flag into do_io (comm == nullptr || comm->am_i_root(), gates filesystem access) and do_print (comm != nullptr && comm->am_i_root(), gates std::cout). Applied in read_ice_lookup_tables(), io_impl(), and in p3_init() where the value passed to compute_tables() is now do_print instead of the old masterproc (that function does no I/O, only prints). This restores the pre-existing quiet default for the comm == nullptr case, while still performing the actual root-read/broadcast optimization

ndkeen · 2026-07-07T22:57:12Z

+  if (masterproc) {
+    stream_t mu_r_file(mu_r_filename.c_str(), std::ios::binary);
+    stream_t revap_file(revap_filename.c_str(), std::ios::binary);
+    stream_t vn_file(vn_filename.c_str(), std::ios::binary);
+    stream_t vm_file(vm_filename, std::ios::binary);
+
+    // Read/write files
+    action(mu_r_file, mu_r_table_vals_h.data(), mu_r_table_vals.size());
+    action(revap_file, revap_table_vals_h.data(), revap_table_vals.size());
+    action(vn_file, vn_table_vals_h.data(), vn_table_vals.size());
+    action(vm_file, vm_table_vals_h.data(), vm_table_vals.size());
+  }


Added EKAT_REQUIRE_MSG checks in io_impl() right after opening each binary table stream (mu_r/revap/vn/vm), and again after the read/write action() call, so a missing/truncated file now fails loudly with a clear message instead of silently propagating bad data (which matters more now that root's read gets broadcast to every rank).

ndkeen · 2026-07-07T22:56:55Z

+  const bool masterproc = comm == nullptr || comm->am_i_root();
  if (masterproc) {
    std::cout << (IsRead ? "Reading" : "Writing") << " lookup (non-ice) tables in dir " << dir << std::endl;
  }


Split the old conflated masterproc flag into do_io (comm == nullptr || comm->am_i_root(), gates filesystem access) and do_print (comm != nullptr && comm->am_i_root(), gates std::cout). Applied in read_ice_lookup_tables(), io_impl(), and in p3_init() where the value passed to compute_tables() is now do_print instead of the old masterproc (that function does no I/O, only prints). This restores the pre-existing quiet default for the comm == nullptr case, while still performing the actual root-read/broadcast optimization

ndkeen · 2026-07-07T22:57:41Z

+      // PAM does not currently plumb an MPI communicator through the
+      // coupler, so pass comm=nullptr here: every rank reads the lookup
+      // table files independently, matching the pre-existing behavior.
+      P3F::p3_init(/*write_tables=*/false, /*comm=*/nullptr);


Probably best to leave this as-is for this PR as it's in PAM

ndkeen · 2026-07-07T23:01:00Z

Pushed a commit to address comments regarding verbosity regression and missing error checks.
Quick test with ne4/ne30 and then a 8-node test with ne120. I can run developer/integration suites as well. To look at performance differences, might need to run a fairly large problem (many nodes). While I expect it to be faster, it is still smaller part of total init cost -- the primary benefit here is to avoid taxing the filesystem unnecessarily.

ndkeen · 2026-07-09T17:35:51Z

OK I was about to merge to next, but I see Conrad is now assigned.

i tested with e3sm_eamxx_v1 on pm-gpu -- same results as we see on cdash.
As well as e3sm_developer on pm-cpu.

Fix P3 to read lookup tables on rank 0 and broadcast to other ranks

bbf9786

ndkeen added BFB PR leaves answers BFB Performance EAMxx C++ based E3SM atmosphere model (aka SCREAM) labels Jul 7, 2026

ndkeen requested review from bartgol and jgfouca July 7, 2026 20:24

jgfouca approved these changes Jul 7, 2026

View reviewed changes

jgfouca requested a review from Copilot July 7, 2026 20:54

Copilot started reviewing on behalf of jgfouca July 7, 2026 20:55 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

address copilot comments

ab8df44

ndkeen requested a review from jgfouca July 7, 2026 23:14

rljacob assigned tcclevenger Jul 8, 2026

jgfouca approved these changes Jul 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix P3 to read lookup tables on rank 0 and broadcast to other ranks#8547

Fix P3 to read lookup tables on rank 0 and broadcast to other ranks#8547
ndkeen wants to merge 2 commits into
masterfrom
ndk/eamxx/P3-lookup-file-read

ndkeen commented Jul 7, 2026

Uh oh!

ndkeen commented Jul 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

ndkeen Jul 7, 2026

Uh oh!

ndkeen Jul 7, 2026

Uh oh!

ndkeen Jul 7, 2026

Uh oh!

ndkeen Jul 7, 2026

Uh oh!

ndkeen commented Jul 7, 2026 •

edited

Loading

Uh oh!

ndkeen commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ndkeen commented Jul 7, 2026

Uh oh!

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

Uh oh!

Summary

Also fixed: a build break in PAM/CRM

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ndkeen Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ndkeen Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ndkeen Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ndkeen Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

ndkeen commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ndkeen commented Jul 7, 2026 •

edited

Loading

ndkeen commented Jul 7, 2026 •

edited

Loading