NgenSimulation MPI interfacing adjustments - #986
Draft
PhilMiller wants to merge 5 commits into
Draft
Conversation
Under NGEN_WITH_MPI, add a constructor that takes an MPI_Comm and derives mpi_rank_ and mpi_num_procs_ from it. The communicator is duplicated with MPI_Comm_dup so the simulation holds a private communication context, stored in a new mpi_comm_ member and freed in the destructor. The existing rank/process-count constructor sets mpi_comm_ to MPI_COMM_NULL so the destructor leaves it alone. The new constructor is not yet used by any caller; wiring it into main() follows separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In MPI builds, main() now constructs the simulation via the communicator constructor, passing MPI_COMM_WORLD; non-MPI builds keep the explicit rank/process-count constructor. run_routing_bmi()'s MPI_Reduce now uses the simulation's private mpi_comm_ rather than MPI_COMM_WORLD directly. The simulation is reset before MPI_Finalize() so its duplicated communicator is freed while MPI is still active. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now that parallel::gather_strings and parallel::broadcast_strings take an MPI_Comm, call them with the simulation's private mpi_comm_, consistent with the MPI_Reduce in run_routing_bmi. All of the routing collectives now run on the simulation's own communicator rather than MPI_COMM_WORLD. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In non-MPI builds, NgenSimulation now takes no MPI-related constructor arguments: it has a single constructor that initializes mpi_rank_ to 0 and mpi_num_procs_ to 1, behaving as rank 0 of a single process. In MPI builds the constructors are unchanged: the MPI_Comm constructor plus the explicit rank/process-count constructor, the latter now documented as the no-communicator path (mpi_comm_ stays MPI_COMM_NULL) for single-process and test use that needs no MPI runtime. main()'s non-MPI branch constructs with the serial constructor; mpi_num_procs is marked [[maybe_unused]] there since it is otherwise only read in MPI-guarded code. The construction test selects the matching constructor per build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Nels, this is some of what I was talking about in terms of "MPI being a compile-time option blows up interface complexity" In going back over this, though, the solution may be to just pull the communicator parameter and member out entirely, and just pass it in to the routing routines where it's actually used. |
11 tasks
Contributor
Author
|
@hellkite500 This is an alternative to #987 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Short description explaining the high-level reason for the pull request]
Additions
Removals
Changes
Testing
Screenshots
Notes
Todos
Checklist
Testing checklist (automated report can be put here)
Target Environment support