From 5fa821e24212c0c40ab63bbdca23dcf5fa4c9f89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20H=C3=A6gland?= Date: Sun, 30 Aug 2026 14:18:18 +0200 Subject: [PATCH] Continue the master run when a slave run ends before it A slave whose schedule ran out before the master's deadlocked the whole coupled run. The slave called MPI_Comm_disconnect(), which blocks until the master joins the collective close, while the master blocked in MPI_Recv() waiting for a next report date that the slave would never send. Neither side could proceed, and the remaining slaves were starved behind the stuck master. Two things were wrong. The message protocol had no way for a slave to say that its run had ended. And receiveTerminateAndDisconnect() disconnected without checking the signal value it had just received, so it mistook the master's routine "keep going" for a shutdown order and also logged a misleading "Received terminate signal from master process". A slave that runs out of report steps now answers on the next-report-date channel with a negative sentinel value. A genuine offset is measured from the slave's own start and is therefore never negative, so no new message tag is needed, and the master learns about the ended slave at exactly the point where it used to block. The master acknowledges, disconnects that one intercommunicator, reports Slave run RES-1 has ended. Master run will continue with no flow from this slave. and carries on without that slave. Add slaveHasEnded() and slaveIsCoupled() alongside the existing slaveIsActivated(). slaveIsActivated() keeps its literal meaning and stays monotonic, while slaveIsCoupled() means "activated and not ended" - the question every gate actually wants to ask before exchanging messages with a slave, or before letting its master groups contribute rates and guide rates. Switching those gates is most of the diff. Little more was needed, because the master already had a complete zero-flow path for a slave that has not activated yet: receiveProductionDataFromSlaves() and receiveInjectionDataFromSlaves() already substituted zeros, and excludeInactiveSlaveMasterGroupsFromDistribution_() already forced the effective group-controlled-wells count to zero so those master groups drop out of guide-rate distribution. An ended slave reuses all of it. Verified on a two-slave case with the first slave's last report step removed: the run now completes, and from the moment that slave ends the field oil production rate equals the remaining slave's group rate exactly. With the unmodified deck, results are bit-identical to a run without this change at every shared report point. Making the master stop instead of continuing, via item 8 of GECON, is a separate option that remains unimplemented. --- .../flow/SimulatorFullyImplicit_impl.hpp | 10 +- .../flow/rescoup/ReservoirCoupling.hpp | 18 ++++ .../flow/rescoup/ReservoirCouplingMaster.cpp | 71 +++++++++++++- .../flow/rescoup/ReservoirCouplingMaster.hpp | 48 +++++++++- .../ReservoirCouplingMasterReportStep.cpp | 14 ++- .../ReservoirCouplingMasterReportStep.hpp | 8 +- .../flow/rescoup/ReservoirCouplingSlave.cpp | 93 +++++++++++++------ .../flow/rescoup/ReservoirCouplingSlave.hpp | 24 +++-- .../rescoup/ReservoirCouplingTimeStepper.cpp | 33 ++++++- .../rescoup/ReservoirCouplingTimeStepper.hpp | 13 ++- .../AdaptiveTimeStepping_impl.hpp | 2 +- .../wells/BlackoilWellModelNetworkGeneric.cpp | 11 +-- .../wells/BlackoilWellModelRescoup_impl.hpp | 6 +- .../rescoup/RescoupConstraintsCalculator.cpp | 26 +++--- 14 files changed, 294 insertions(+), 83 deletions(-) diff --git a/opm/simulators/flow/SimulatorFullyImplicit_impl.hpp b/opm/simulators/flow/SimulatorFullyImplicit_impl.hpp index f250316ee54..26a08ea9cff 100644 --- a/opm/simulators/flow/SimulatorFullyImplicit_impl.hpp +++ b/opm/simulators/flow/SimulatorFullyImplicit_impl.hpp @@ -130,10 +130,16 @@ run(SimulatorTimer& timer) this->reservoirCouplingMaster_->sendTerminateAndDisconnect(); } else if (this->reservoirCouplingSlave_ && !this->reservoirCouplingSlave_->terminated()) { - // TODO: Implement GECON item 8: stop master process when a slave finishes + // We got here by running out of report steps of our own. If the master is still + // running, notifyEndOfRunAndDisconnect() tells it so, and the master then continues + // with no flow from this slave. + // + // TODO: Implement GECON item 8, which lets a master deck ask for the opposite: stop + // the master run when one of its slaves finishes, rather than continuing without it. + // // Only call if not already terminated via maybeReceiveTerminateSignalFromMaster() // (which happens when master finishes before slave reaches end of its loop) - this->reservoirCouplingSlave_->receiveTerminateAndDisconnect(); + this->reservoirCouplingSlave_->notifyEndOfRunAndDisconnect(); } #endif diff --git a/opm/simulators/flow/rescoup/ReservoirCoupling.hpp b/opm/simulators/flow/rescoup/ReservoirCoupling.hpp index 21e5fef4551..74ae82d9ebd 100644 --- a/opm/simulators/flow/rescoup/ReservoirCoupling.hpp +++ b/opm/simulators/flow/rescoup/ReservoirCoupling.hpp @@ -160,6 +160,24 @@ enum class MessageTag : int { SlaveStatus, }; +/// @brief Sentinel sent by a slave on the MessageTag::SlaveNextReportDate channel to tell the +/// master that the slave has reached the end of its own schedule and will not take any +/// further report step. +/// +/// @details A genuine next-report-date offset is measured from the slave's own simulation +/// start and is therefore never negative, so a negative value is unambiguous. Reusing the +/// next-report-date channel rather than adding a new message tag means the master learns +/// about the ended slave at the point where it is already blocked waiting for that slave, +/// which is what avoids a deadlock. See ReservoirCouplingSlave::notifyEndOfRunAndDisconnect() +/// and ReservoirCouplingTimeStepper::receiveNextReportDateFromSlaves(). +inline constexpr double slave_end_of_run_sentinel = -1.0; + +/// @brief Whether a next-report-date offset received from a slave is the end-of-run sentinel. +inline bool isSlaveEndOfRunSentinel(double next_report_time_offset) +{ + return next_report_time_offset < 0.0; +} + /// @brief Phase indices for reservoir coupling, we currently only support black-oil phases /// (oil, gas, and water). enum class Phase : std::size_t { diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingMaster.cpp b/opm/simulators/flow/rescoup/ReservoirCouplingMaster.cpp index d959833410d..987bd633dd1 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingMaster.cpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingMaster.cpp @@ -210,6 +210,45 @@ isMasterGroup(const std::string &group_name) const this->master_group_slave_names_.end(); } +template +void +ReservoirCouplingMaster:: +markSlaveEndedAndDisconnect(int index) +{ + if (this->slave_ended_.size() < this->numSlavesStarted()) { + this->slave_ended_.resize(this->numSlavesStarted(), 0); + } + if (this->slave_ended_[index] != 0) { + return; // Already recorded and disconnected + } + this->slave_ended_[index] = 1; + // Acknowledge the slave's end-of-run notice. The slave is blocked waiting for this + // signal before it joins the disconnect below, see + // ReservoirCouplingSlave::notifyEndOfRunAndDisconnect(). + if (this->comm_.rank() == 0) { + int terminate_signal = 1; + // NOTE: See comment about error handling at the top of this file. + MPI_Send( + &terminate_signal, + /*count=*/1, + /*datatype=*/MPI_INT, + /*dest_rank=*/0, + /*tag=*/static_cast(MessageTag::SlaveProcessTermination), + this->master_slave_comm_[index] + ); + } + // MPI_Comm_disconnect() is collective over the intercommunicator: every master rank must + // call it, and it only completes once the slave has called it too. Afterwards the handle + // is MPI_COMM_NULL, so every later loop over slaves must skip this one - which it does, + // because slaveIsCoupled() is now false for it. + MPI_Comm_disconnect(&this->master_slave_comm_[index]); + this->logger_.info(fmt::format( + "Slave run {} has ended.\n" + "Master run will continue with no flow from this slave.", + this->slave_names_[index] + )); +} + template void ReservoirCouplingMaster:: @@ -260,6 +299,9 @@ maybeReceiveActivationHandshakeFromSlaves(double current_time) if (this->slave_activation_status_.empty()) { this->slave_activation_status_.resize(this->numSlavesStarted(), false); } + if (this->slave_ended_.empty()) { + this->slave_ended_.resize(this->numSlavesStarted(), 0); + } if (this->comm_.rank() == 0) { auto current_date = this->schedule_.getStartTime() + current_time; @@ -268,6 +310,12 @@ maybeReceiveActivationHandshakeFromSlaves(double current_time) if (this->slaveIsActivated(i)) { continue; } + // A slave that ended without ever activating has no intercommunicator left to + // probe, and its activation date lies in the past, which would trip the assert + // below. + if (this->slaveHasEnded(i)) { + continue; + } // Check if slave should activate during this timestep double slave_activation_date = this->slave_activation_dates_[i]; // NOTE: The master will adjust its time stepping to ensure that its step will always @@ -323,11 +371,11 @@ numSlavesStarted() const template std::size_t ReservoirCouplingMaster:: -numActivatedSlaves() const +numCoupledSlaves() const { std::size_t count = 0; for (std::size_t i = 0; i < this->slave_activation_status_.size(); ++i) { - if (this->slave_activation_status_[i] != 0) { + if (this->slaveIsCoupled(static_cast(i))) { ++count; } } @@ -404,15 +452,19 @@ void ReservoirCouplingMaster:: sendDontTerminateSignalToSlaves() { - // Send "don't terminate" signal (value=0) to the activated slaves. + // Send "don't terminate" signal (value=0) to the coupled slaves. // This is called at the start of each iteration in the master's substep loop. - // We send only to activated slaves: only an activated slave runs the coupled substep + // We send only to coupled slaves: only an activated slave runs the coupled substep // loop and is blocked at the terminate-signal receive point. A slave that has not yet // activated does not consume this channel, so sending to it would queue stale signals // that the slave later reads out of step when it does activate (e.g. a slave whose // activation date coincides with the master's last report step when the master schedule // is truncated by an END keyword). The final terminate signal is still sent to all // started slaves in sendTerminateAndDisconnect(). + // NOTE: A slave that is about to end is still coupled at this point and does receive + // this signal - that is deliberate. It consumes the signal as the first step of + // ReservoirCouplingSlave::notifyEndOfRunAndDisconnect(), which is how the two sides stay + // in step while the slave reports that its run has ended. if (this->comm_.rank() == 0) { int terminate_signal = 0; // maybeReceiveActivationHandshakeFromSlaves() has resized the activation-status @@ -420,7 +472,7 @@ sendDontTerminateSignalToSlaves() // once, so the vector covers all started slaves here. assert(this->slave_activation_status_.size() == this->numSlavesStarted()); for (std::size_t i = 0; i < this->numSlavesStarted(); i++) { - if (!this->slaveIsActivated(i)) { + if (!this->slaveIsCoupled(i)) { continue; } // NOTE: See comment about error handling at the top of this file. @@ -506,9 +558,15 @@ sendTerminateAndDisconnect() // Step 1: Send terminate signal (value=1) to all spawned slaves (only from rank 0) // We send to all spawned slaves, not just activated ones, because even non-activated // slaves are running and waiting at the terminate signal receive point. + // Slaves that already ended are the exception: they have been acknowledged and + // disconnected in markSlaveEndedAndDisconnect(), and their MPI_Comm handle is now + // MPI_COMM_NULL. if (this->comm_.rank() == 0) { int terminate_signal = 1; for (std::size_t i = 0; i < this->numSlavesStarted(); i++) { + if (this->slaveHasEnded(i)) { + continue; + } this->logger_.info(fmt::format( "Sending terminate signal to slave process: {}", this->slave_names_[i])); // NOTE: See comment about error handling at the top of this file. @@ -524,6 +582,9 @@ sendTerminateAndDisconnect() } // Step 2: Disconnect intercommunicators (collective operation - all ranks must participate) for (std::size_t i = 0; i < this->numSlavesStarted(); i++) { + if (this->slaveHasEnded(i)) { + continue; // Already disconnected in markSlaveEndedAndDisconnect() + } MPI_Comm_disconnect(&this->master_slave_comm_[i]); if (this->comm_.rank() == 0) { this->logger_.info(fmt::format( diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingMaster.hpp b/opm/simulators/flow/rescoup/ReservoirCouplingMaster.hpp index b072785df25..3b605e41727 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingMaster.hpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingMaster.hpp @@ -157,7 +157,11 @@ class ReservoirCouplingMaster { std::size_t numSlaveGroups(unsigned int index); std::size_t numSlaves() const { return this->numSlavesStarted(); } std::size_t numSlavesStarted() const; - std::size_t numActivatedSlaves() const; + + /// @brief Number of slaves that currently take part in the coupling. + /// @details Counts the slaves for which slaveIsCoupled() holds, i.e. activated and not + /// yet ended. + std::size_t numCoupledSlaves() const; void rebuildSlaveIdxToMasterGroupsVector(); void receiveNextReportDateFromSlaves(); void receiveProductionDataFromSlaves(); @@ -216,8 +220,44 @@ class ReservoirCouplingMaster { void setSlaveActivationDate(int index, double date) { this->slave_activation_dates_[index] = date; } void setSlaveNextReportTimeOffset(int index, double offset); void setSlaveStartDate(int index, std::time_t date) { this->slave_start_dates_[index] = date; } + /// @brief Whether the slave has completed its activation handshake with the master. + /// @details Monotonic: once true it never becomes false again, not even when the slave + /// later reaches the end of its own schedule. Callers that ask "does this slave take + /// part in the coupling right now?" want slaveIsCoupled() instead. bool slaveIsActivated(int index) const { return this->slave_activation_status_[index] != 0; } + /// @brief Whether the slave has reached the end of its own schedule and left the coupling. + /// @details A slave whose deck runs out of report steps before the master's does notifies + /// the master, and the two disconnect their intercommunicator. From that point on the + /// master must not communicate with the slave, and must treat the slave's master groups + /// as producing and injecting nothing. + /// @param index Index of the slave process. + /// @return true once the slave has notified the master that its run has ended. + bool slaveHasEnded(int index) const + { + return (static_cast(index) < this->slave_ended_.size()) + && (this->slave_ended_[index] != 0); + } + + /// @brief Whether the slave currently takes part in the coupling. + /// @details True only for a slave that has activated and has not ended. This is the + /// predicate to use before any master-slave message exchange, and before letting a + /// master group contribute rates, potentials or guide rates: a slave that is not + /// coupled contributes zeros. + bool slaveIsCoupled(int index) const + { + return this->slaveIsActivated(index) && !this->slaveHasEnded(index); + } + + /// @brief Record that a slave has ended, and close its intercommunicator. + /// @details Must be called on **every** master rank, because MPI_Comm_disconnect() is + /// collective over the whole intercommunicator. Rank 0 additionally sends the + /// acknowledging terminate signal the slave is waiting for. After this call + /// slaveIsCoupled() returns false for the slave and no further message may be sent to + /// it or received from it. + /// @param index Index of the slave process that has ended. + void markSlaveEndedAndDisconnect(int index); + /// @brief Whether the master syncs with slaves at slave report-step boundaries /// (true) or at every master actual time step (false, default CLI flag value). /// @details Set from the --rescoup-sync-at-report-steps CLI flag in the @@ -287,6 +327,12 @@ class ReservoirCouplingMaster { // with MPI broadcast(). std::vector slave_activation_status_; + // Whether the slave has reached the end of its own schedule and disconnected. Same + // std::uint8_t-instead-of-bool reasoning as slave_activation_status_ above: the value is + // derived on every rank from the broadcast next-report-date vector, so it must be a plain + // array of bytes. + std::vector slave_ended_; + // A mapping from master group names to slave names std::map master_group_slave_names_; diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.cpp b/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.cpp index bdb29b2e80b..4de1f6ce6b2 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.cpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.cpp @@ -140,7 +140,7 @@ receiveInjectionDataFromSlaves() } std::vector injection_data(num_slave_groups); if (this->comm().rank() == 0) { - if (this->slaveIsActivated(i)) { + if (this->slaveIsCoupled(i)) { // NOTE: See comment about error handling at the top of this file. auto MPI_INJECTION_DATA_TYPE = Dune::MPITraits::getType(); MPI_Recv( @@ -158,8 +158,11 @@ receiveInjectionDataFromSlaves() )); } else { + // Either the slave has not activated yet, or it has reached the end of its own + // schedule and left the coupling. In both cases the master continues with no + // injection from this slave. this->logger().debug(fmt::format( - "Slave {} has not activated yet, skipping injection data", + "Slave {} is not coupled, skipping injection data", this->slaveName(i) )); injection_data.assign(num_slave_groups, SlaveGroupInjectionData{}); // Set to zero injection data @@ -193,7 +196,7 @@ receiveProductionDataFromSlaves() } std::vector production_data(num_slave_groups); if (this->comm().rank() == 0) { - if (this->slaveIsActivated(i)) { + if (this->slaveIsCoupled(i)) { // NOTE: See comment about error handling at the top of this file. auto MPI_PRODUCTION_DATA_TYPE = Dune::MPITraits::getType(); MPI_Recv( @@ -211,8 +214,11 @@ receiveProductionDataFromSlaves() )); } else { + // Either the slave has not activated yet, or it has reached the end of its own + // schedule and left the coupling. In both cases the master continues with no + // production from this slave. this->logger().debug(fmt::format( - "Slave {} has not activated yet, skipping production data", + "Slave {} is not coupled, skipping production data", this->slaveName(i) )); production_data.assign(num_slave_groups, SlaveGroupProductionData{}); // Set to zero production data diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.hpp b/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.hpp index db69b967ef8..66285b7327f 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.hpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingMasterReportStep.hpp @@ -260,10 +260,12 @@ class ReservoirCouplingMasterReportStep { /// @return ReservoirCouplingGroupRates struct with per-group rates. data::ReservoirCouplingGroupRates collectGroupRatesForSummary() const; - /// @brief Check if a specific slave process has been activated + /// @brief Check if a specific slave process currently takes part in the coupling + /// @details True only for a slave that has activated and has not ended. A slave that is + /// not coupled sends no data, and its groups contribute zero rates. /// @param index Index of the slave process - /// @return true if the slave is activated, false otherwise - bool slaveIsActivated(int index) const { return this->master_.slaveIsActivated(index); } + /// @return true if the slave is coupled, false otherwise + bool slaveIsCoupled(int index) const { return this->master_.slaveIsCoupled(index); } /// @brief Get the name of a specific slave process /// @param index Index of the slave process diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingSlave.cpp b/opm/simulators/flow/rescoup/ReservoirCouplingSlave.cpp index bac0a48cd9b..ac84a788f20 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingSlave.cpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingSlave.cpp @@ -255,6 +255,71 @@ maybeReceiveTerminateSignalFromMaster() return false; } +template +void +ReservoirCouplingSlave:: +notifyEndOfRunAndDisconnect() +{ + // Step 1: read the master's signal (only on rank 0, then broadcast). A non-zero value + // means the master has finished as well and there is nothing left to tell it. A zero + // means "keep going": the master has started another sync step and is at this moment + // blocked waiting for a next report date from us. + int terminate_signal = 0; + if (this->comm_.rank() == 0) { + // NOTE: See comment about error handling at the top of this file. + MPI_Recv( + &terminate_signal, + /*count=*/1, + /*datatype=*/MPI_INT, + /*source_rank=*/0, + /*tag=*/static_cast(MessageTag::SlaveProcessTermination), + this->slave_master_comm_, + MPI_STATUS_IGNORE + ); + } + this->comm_.broadcast(&terminate_signal, /*count=*/1, /*emitter_rank=*/0); + + if (terminate_signal == 0) { + // Step 2: the master is still running. We have no next report date to give it, so we + // answer with the end-of-run sentinel instead. That is what lets the master drop us + // from the coupling and carry on with no flow from this slave, rather than blocking + // on a report date that will never come. + this->logger_.info( + "Slave run has ended before the master run, notifying the master process"); + if (this->comm_.rank() == 0) { + double end_of_run = ReservoirCoupling::slave_end_of_run_sentinel; + // NOTE: See comment about error handling at the top of this file. + MPI_Send( + &end_of_run, + /*count=*/1, + /*datatype=*/MPI_DOUBLE, + /*dest_rank=*/0, + /*tag=*/static_cast(MessageTag::SlaveNextReportDate), + this->slave_master_comm_ + ); + // Step 3: wait for the master to acknowledge before joining the disconnect. The + // acknowledgement is what guarantees the master has already recorded us as ended + // and will therefore call MPI_Comm_disconnect() on its side. + MPI_Recv( + &terminate_signal, + /*count=*/1, + /*datatype=*/MPI_INT, + /*source_rank=*/0, + /*tag=*/static_cast(MessageTag::SlaveProcessTermination), + this->slave_master_comm_, + MPI_STATUS_IGNORE + ); + } + this->comm_.broadcast(&terminate_signal, /*count=*/1, /*emitter_rank=*/0); + } + this->logger_.info("Received terminate signal from master process"); + + // Disconnect the intercommunicator (collective operation - all ranks must participate) + MPI_Comm_disconnect(&this->slave_master_comm_); + this->terminated_ = true; + this->logger_.info("Disconnected intercommunicator with master process"); +} + template void ReservoirCouplingSlave:: @@ -335,34 +400,6 @@ receiveProductionGroupConstraintsFromMaster(std::size_t num_targets) this->report_step_data_->receiveProductionGroupConstraintsFromMaster(num_targets); } -template -void -ReservoirCouplingSlave:: -receiveTerminateAndDisconnect() -{ - // Receive terminate signal from master (only on rank 0, then broadcast) - int terminate_signal = 0; - if (this->comm_.rank() == 0) { - // NOTE: See comment about error handling at the top of this file. - MPI_Recv( - &terminate_signal, - /*count=*/1, - /*datatype=*/MPI_INT, - /*source_rank=*/0, - /*tag=*/static_cast(MessageTag::SlaveProcessTermination), - this->slave_master_comm_, - MPI_STATUS_IGNORE - ); - this->logger_.info("Received terminate signal from master process"); - } - this->comm_.broadcast(&terminate_signal, /*count=*/1, /*emitter_rank=*/0); - - // Disconnect the intercommunicator (collective operation - all ranks must participate) - MPI_Comm_disconnect(&this->slave_master_comm_); - this->terminated_ = true; - this->logger_.info("Disconnected intercommunicator with master process"); -} - template void ReservoirCouplingSlave:: diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingSlave.hpp b/opm/simulators/flow/rescoup/ReservoirCouplingSlave.hpp index b55e89851bd..c56483b72cf 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingSlave.hpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingSlave.hpp @@ -180,16 +180,26 @@ class ReservoirCouplingSlave { /// @return true if terminate signal received and disconnect completed, false to continue. bool maybeReceiveTerminateSignalFromMaster(); - /// @brief Receive terminate signal from master and disconnect the intercommunicator. + /// @brief Wind up the coupling after the slave has run out of report steps of its own. /// - /// This method must be called at the end of the simulation to cleanly shut down - /// the MPI intercommunicator created when the slave was spawned. It performs two steps: - /// 1. Receives a terminate signal from master (only on rank 0, then broadcast) - /// 2. Disconnects the intercommunicator (collective operation) + /// Called once, when the slave's own schedule is exhausted, to shut down the MPI + /// intercommunicator created when the slave was spawned. Two cases have to be told apart, + /// and the terminate signal from the master is what tells them apart: /// - /// Both master and slaves must call their respective disconnect methods for + /// 1. **The master finished too.** The signal is non-zero. Nothing more is expected of + /// us; disconnect and return. + /// 2. **The master is still running.** The signal is zero ("keep going"), sent because + /// the master has begun another sync step and expects a next report date from us. + /// There is none, so we answer with ReservoirCoupling::slave_end_of_run_sentinel on + /// the next-report-date channel. That tells the master our run has ended, and it + /// replies with a terminate signal before joining the disconnect. + /// + /// Both master and slave must call their respective disconnect methods for /// MPI_Comm_disconnect() to complete - it is a collective operation. - void receiveTerminateAndDisconnect(); + /// + /// @see ReservoirCouplingTimeStepper::receiveNextReportDateFromSlaves() and + /// ReservoirCouplingMaster::markSlaveEndedAndDisconnect() for the master's half. + void notifyEndOfRunAndDisconnect(); /// @brief True once the initial well solve for this sync step has run /// @details Delegates to ReservoirCouplingSlaveReportStep diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.cpp b/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.cpp index b796e68cfd8..021591262b5 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.cpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.cpp @@ -70,6 +70,11 @@ maybeChopSubStep(double suggested_timestep_original, double elapsed_time) const // slave process will report or start during the timestep [step_start_date, step_end_date] // where suggested_timestep = step_end_date - step_start_date for (std::size_t i = 0; i < num_slaves; i++) { + if (this->slaveHasEnded(i)) { + // The slave has no further report steps, so there is nothing left to synchronize + // with. Its last reported date is stale and must not chop the master's timestep. + continue; + } double slave_start_date = this->slaveStartDate(i); double slave_activation_date = this->slaveActivationDate(i); double slave_next_report_date{this->slave_next_report_time_offsets_[i] + slave_start_date}; @@ -110,11 +115,12 @@ receiveNextReportDateFromSlaves() if (this->comm().rank() == 0) { this->logger().debug("Receiving next report dates from slave processes"); for (unsigned int i = 0; i < num_slaves; i++) { - if (!this->slaveIsActivated(i)) { - // Set to zero to indicate that the slave has not activated yet + if (!this->slaveIsCoupled(i)) { + // Set to zero to indicate that the slave does not take part in the coupling: + // it has either not activated yet, or it has already ended. this->slave_next_report_time_offsets_[i] = 0.0; this->logger().debug(fmt::format( - "Slave {} has not activated yet, setting next report date to 0.0", + "Slave {} is not coupled, setting next report date to 0.0", this->slaveName(i))); continue; } @@ -130,6 +136,14 @@ receiveNextReportDateFromSlaves() MPI_STATUS_IGNORE ); this->slave_next_report_time_offsets_[i] = slave_next_report_time_offset; + if (ReservoirCoupling::isSlaveEndOfRunSentinel(slave_next_report_time_offset)) { + // The slave has run out of report steps of its own. It is now waiting for the + // acknowledgement and the disconnect that markSlaveEndedAndDisconnect() below + // performs. Do not log a report date for it - there is none. + this->logger().debug(fmt::format( + "Slave {} reported that its run has ended", this->slaveName(i))); + continue; + } this->logger().debug(fmt::format( "Received next report date from {}: {} (offset from slave start)", this->slaveName(i), ReservoirCoupling::formatDays(slave_next_report_time_offset) @@ -140,6 +154,15 @@ receiveNextReportDateFromSlaves() this->slave_next_report_time_offsets_.data(), /*count=*/num_slaves, /*emitter_rank=*/0 ); this->logger().debug("Broadcasted slave next report dates to all ranks"); + // Every rank derives "this slave has ended" from the broadcast sentinel, so no separate + // broadcast is needed. markSlaveEndedAndDisconnect() must run on all ranks because the + // MPI_Comm_disconnect() inside it is collective over the intercommunicator. + for (unsigned int i = 0; i < num_slaves; i++) { + if (ReservoirCoupling::isSlaveEndOfRunSentinel(this->slave_next_report_time_offsets_[i])) { + this->slave_next_report_time_offsets_[i] = 0.0; + this->master_.markSlaveEndedAndDisconnect(i); + } + } } template @@ -150,9 +173,9 @@ sendNextTimeStepToSlaves(double timestep) OPM_TIMEFUNCTION(); if (this->comm().rank() == 0) { for (unsigned int slave_idx = 0; slave_idx < this->numSlaves(); slave_idx++) { - if (!this->slaveIsActivated(slave_idx)) { + if (!this->slaveIsCoupled(slave_idx)) { this->logger().debug(fmt::format( - "Slave {} has not activated yet, skipping sending next time step", + "Slave {} is not coupled, skipping sending next time step", this->slaveName(slave_idx) )); continue; diff --git a/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.hpp b/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.hpp index 3afdc281391..815c4fe5ba4 100644 --- a/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.hpp +++ b/opm/simulators/flow/rescoup/ReservoirCouplingTimeStepper.hpp @@ -115,10 +115,17 @@ class ReservoirCouplingTimeStepper { /// @return Reference to the Schedule object containing timing and control information const Schedule& schedule() const { return this->master_.schedule(); } - /// @brief Check if a specific slave process has been activated + /// @brief Check if a specific slave process has reached the end of its own schedule /// @param index Index of the slave process - /// @return true if the slave is activated, false otherwise - bool slaveIsActivated(int index) const { return this->master_.slaveIsActivated(index); } + /// @return true if the slave has ended and disconnected, false otherwise + bool slaveHasEnded(int index) const { return this->master_.slaveHasEnded(index); } + + /// @brief Check if a specific slave process currently takes part in the coupling + /// @details True only for a slave that has activated and has not ended. Use this before + /// any message exchange with the slave. + /// @param index Index of the slave process + /// @return true if the slave is coupled, false otherwise + bool slaveIsCoupled(int index) const { return this->master_.slaveIsCoupled(index); } /// @brief Get the name of a specific slave process /// @param index Index of the slave process diff --git a/opm/simulators/timestepping/AdaptiveTimeStepping_impl.hpp b/opm/simulators/timestepping/AdaptiveTimeStepping_impl.hpp index b28d27dc466..bf0cb7c05ec 100644 --- a/opm/simulators/timestepping/AdaptiveTimeStepping_impl.hpp +++ b/opm/simulators/timestepping/AdaptiveTimeStepping_impl.hpp @@ -670,7 +670,7 @@ getRcMasterSyncStepLength_(double prev_step, // dt_ to current_step_length. } current_step_length = reservoirCouplingMaster_().maybeChopSubStep(current_step_length, current_time); - auto num_active = reservoirCouplingMaster_().numActivatedSlaves(); + auto num_active = reservoirCouplingMaster_().numCoupledSlaves(); OpmLog::info(fmt::format( "\nChoosing next sync time{} between master and {} active slave {}: {:.2f} days", sync_at_report_steps ? " (RSYNC)" : "", diff --git a/opm/simulators/wells/BlackoilWellModelNetworkGeneric.cpp b/opm/simulators/wells/BlackoilWellModelNetworkGeneric.cpp index 99af93df038..530b5cba732 100644 --- a/opm/simulators/wells/BlackoilWellModelNetworkGeneric.cpp +++ b/opm/simulators/wells/BlackoilWellModelNetworkGeneric.cpp @@ -92,12 +92,11 @@ updateActiveState(const int report_step) const auto& rescoup_master = rescoup_proxy.master(); const auto num_slaves = rescoup_master.numSlaves(); for (std::size_t s = 0; s < num_slaves && !network_active; ++s) { - // Only an activated slave supplies leaf rates this step; a master - // group whose slave is inactive must not keep the network active - // (it would be solved against missing/stale slave rates). This - // matches the slaveIsActivated gate used in the coupled-network - // iteration. - if (!rescoup_master.slaveIsActivated(s)) { + // Only a coupled slave supplies leaf rates this step; a master + // group whose slave has not activated yet, or has already ended, + // must not keep the network active (it would be solved against + // missing/stale slave rates). + if (!rescoup_master.slaveIsCoupled(s)) { continue; } for (const auto& master_group : rescoup_master.getMasterGroupNamesForSlave(s)) { diff --git a/opm/simulators/wells/BlackoilWellModelRescoup_impl.hpp b/opm/simulators/wells/BlackoilWellModelRescoup_impl.hpp index 1efada79b18..8697603a677 100644 --- a/opm/simulators/wells/BlackoilWellModelRescoup_impl.hpp +++ b/opm/simulators/wells/BlackoilWellModelRescoup_impl.hpp @@ -78,7 +78,7 @@ masterNetworkHasMasterGroupLeaves() const const auto& rcm = this->reservoirCouplingMaster(); const auto num_slaves = rcm.numSlaves(); for (std::size_t s = 0; s < num_slaves; ++s) { - if (!rcm.slaveIsActivated(s)) continue; + if (!rcm.slaveIsCoupled(s)) continue; if (this->masterNetworkHasMasterGroupLeavesForSlave_(s)) { return true; } @@ -293,7 +293,7 @@ sendCoupledNetworkActiveStatus() const auto num_slaves = rescoup_master.numSlaves(); bool any_connected = false; for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (rescoup_master.slaveIsActivated(slave_idx)) { + if (rescoup_master.slaveIsCoupled(slave_idx)) { const bool connected = this->masterNetworkHasMasterGroupLeavesForSlave_(slave_idx); any_connected = any_connected || connected; @@ -334,7 +334,7 @@ sendMasterGroupNodePressuresToSlaves(bool is_final) const auto& node_pressures = this->network_.nodePressures(); const auto num_slaves = rescoup_master.numSlaves(); for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (!rescoup_master.slaveIsActivated(slave_idx)) continue; + if (!rescoup_master.slaveIsCoupled(slave_idx)) continue; std::vector> pressures; const auto& master_groups = rescoup_master.getMasterGroupNamesForSlave(slave_idx); for (std::size_t i = 0; i < master_groups.size(); ++i) { diff --git a/opm/simulators/wells/rescoup/RescoupConstraintsCalculator.cpp b/opm/simulators/wells/rescoup/RescoupConstraintsCalculator.cpp index fb40d99657b..90eeda02d2c 100644 --- a/opm/simulators/wells/rescoup/RescoupConstraintsCalculator.cpp +++ b/opm/simulators/wells/rescoup/RescoupConstraintsCalculator.cpp @@ -228,7 +228,7 @@ calculateMasterGroupConstraintsAndSendToSlaves() std::vector> all_injection_targets(num_slaves); std::vector> all_production_constraints(num_slaves); for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (rescoup_master.slaveIsActivated(slave_idx)) { + if (rescoup_master.slaveIsCoupled(slave_idx)) { auto [inj, prod] = this->calculateSlaveGroupConstraints_(slave_idx, calculator); all_injection_targets[slave_idx] = std::move(inj); all_production_constraints[slave_idx] = std::move(prod); @@ -242,7 +242,7 @@ calculateMasterGroupConstraintsAndSendToSlaves() // Phase 3: send to slaves. The send functions are rank-0-only internally. for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (rescoup_master.slaveIsActivated(slave_idx)) { + if (rescoup_master.slaveIsCoupled(slave_idx)) { this->sendSlaveGroupConstraintsToSlave_( rescoup_master, slave_idx, all_injection_targets[slave_idx], @@ -279,7 +279,7 @@ recalculateInjectionTargetsAndSendToSlaves() }; const auto num_slaves = rescoup_master.numSlaves(); for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (!rescoup_master.slaveIsActivated(slave_idx)) { + if (!rescoup_master.slaveIsCoupled(slave_idx)) { continue; } auto injection_targets = this->calculateSlaveGroupInjectionTargets_(slave_idx, calculator); @@ -476,20 +476,16 @@ capAndRedistributeProductionTargets_( this->updateGCWAndTargetReductions_(); } -// Switch the master groups associated with currently-inactive slaves to +// Switch the master groups associated with currently-uncoupled slaves to // individual control so they are excluded from guide-rate distribution. -// An inactive slave contributes zero rate and zero potential, so +// An uncoupled slave contributes zero rate and zero potential, so // its master groups should not consume any share of the parent's target. // -// TODO: A slave run can finish before the master. If the slave run finishes -// before the master run, the master run will continue without any production -// or injection from the slave (unless GECON item 8 is "YES"). The current -// `slaveIsActivated` flag transitions false→true on the activation -// handshake but never back to false, so finished slaves are still -// treated as contributing. When finished-slave detection lands (a -// separate PR with the corresponding MPI-protocol changes), the test -// below should also cover finished slaves so they too are excluded from -// guide-rate distribution. +// A slave is uncoupled either because it has not activated yet, or because it +// has reached the end of its own schedule while the master run continues. +// +// TODO: Implement GECON item 8, which lets a master deck ask for the master run +// to stop when one of its slaves finishes, instead of continuing without it. template void RescoupConstraintsCalculator:: @@ -503,7 +499,7 @@ excludeInactiveSlaveMasterGroupsFromDistribution_() // rate mode used here as a marker. const Group::ProductionCMode individual_cmode = Group::ProductionCMode::ORAT; for (std::size_t slave_idx = 0; slave_idx < num_slaves; ++slave_idx) { - if (!rescoup_master.slaveIsActivated(slave_idx)) { + if (!rescoup_master.slaveIsCoupled(slave_idx)) { const auto& master_groups = rescoup_master.getMasterGroupNamesForSlave(slave_idx); for (const auto& group_name : master_groups) { this->group_state_helper_.groupState().production_control(