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(