Skip to content

Fix MPI tally reductions for counts larger than INT_MAX - #4037

Merged
GuySten merged 5 commits into
openmc-dev:developfrom
paulromano:large-tally-reduce
Jul 31, 2026
Merged

Fix MPI tally reductions for counts larger than INT_MAX#4037
GuySten merged 5 commits into
openmc-dev:developfrom
paulromano:large-tally-reduce

Conversation

@paulromano

Copy link
Copy Markdown
Contributor

Description

This PR fixes silent tally corruption in MPI runs when a tally result buffer contains more than INT_MAX elements. The existing reductions passed size_t counts directly to MPI_Reduce, whose legacy C API accepts an int. Large counts could therefore be narrowed silently, leaving part of the tally unreduced. In my implementation, the large-count MPI_Reduce_c function from MPI-4 is used if available (checked via CMake capability detection). However, because MPI-4 support is not universal, it will fall back to correctly bounded MPI_Reduce chunks that are no more than INT_MAX elements.

Note that I was hesitant to add a test that allocates a > 2 GB buffer in CI, so at present there is no associated test.

Separately, while working on this, I ran into a compile-time error when building OpenMC with MPI against a LibMesh build that wasn't built against MPI; small fix for that is also included.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuySten

GuySten commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

There are more places in the code using MPI_Reduce:

rg MPI_Reduce
src/mesh.cpp
1157:  MPI_Reduce(
1162:    MPI_Reduce(&outside_, outside, 1, MPI_C_BOOL, MPI_LOR, 0, mpi::intracomm);
1630:  MPI_Reduce(
1635:    MPI_Reduce(&outside_, outside, 1, MPI_C_BOOL, MPI_LOR, 0, mpi::intracomm);

src/tallies/tally.cpp
1053:      MPI_Reduce(values.data(), values_reduced.data(), values.size(),
1077:  MPI_Reduce(gt_values.data(), gt_values_reduced.data(), N_GLOBAL_TALLIES,
1090:  MPI_Reduce(&simulation::total_weight, &weight_reduced, 1, MPI_DOUBLE, MPI_SUM,

src/output.cpp
246:  MPI_Reduce(temp.data(), model::overlap_check_count.data(),

src/simulation.cpp
230:    MPI_Reduce(&simulation::simulation_tracks_completed, &total_tracks, 1,

src/cmfd_solver.cpp
120:  MPI_Reduce(
124:  MPI_Reduce(&outside_, outside, 1, MPI_C_BOOL, MPI_LOR, 0, mpi::intracomm);

src/state_point.cpp
907:  MPI_Reduce(gt.data(), gt_reduced.data(), gt.size(), MPI_DOUBLE, MPI_SUM, 0,
956:      MPI_Reduce(MPI_IN_PLACE, values.data(), values.size(), MPI_DOUBLE,
986:      MPI_Reduce(values.data(), nullptr, values.size(), MPI_DOUBLE, MPI_SUM, 0,

@paulromano

Copy link
Copy Markdown
Contributor Author

I didn't touch the other reductions since they operate on small, fixed-size buffers just to limit the scope of this PR. I can update them though if you feel strongly.

@GuySten

GuySten commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

In some of the cases it is not clear to me that the reductions are over small size buffers.
Specifically the reductions in mesh.cpp and cmfd_solver.cpp.

@paulromano
paulromano requested a review from shikhar413 as a code owner July 30, 2026 23:53
@paulromano

Copy link
Copy Markdown
Contributor Author

@GuySten Good point; I've updated a few of more variable-length reductions to be safe

@GuySten GuySten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now.

@GuySten GuySten added Bugs Merging Soon PR will be merged in < 24 hrs if no further comments are made. labels Jul 31, 2026
@GuySten
GuySten merged commit 37c3a9a into openmc-dev:develop Jul 31, 2026
19 checks passed
@paulromano
paulromano deleted the large-tally-reduce branch July 31, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugs Merging Soon PR will be merged in < 24 hrs if no further comments are made.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants