From e790972201000327aa1011a1cb743750542993fb Mon Sep 17 00:00:00 2001 From: jakobtorben Date: Thu, 10 Oct 2024 15:51:53 +0200 Subject: [PATCH] PR review changes --- opm/simulators/linalg/ISTLSolver.hpp | 13 ++++++------ opm/simulators/wells/BlackoilWellModel.hpp | 13 +++++++++--- .../wells/BlackoilWellModel_impl.hpp | 21 +++++++++++-------- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/opm/simulators/linalg/ISTLSolver.hpp b/opm/simulators/linalg/ISTLSolver.hpp index 93e1e1cda..053a6fbc6 100644 --- a/opm/simulators/linalg/ISTLSolver.hpp +++ b/opm/simulators/linalg/ISTLSolver.hpp @@ -352,8 +352,6 @@ std::unique_ptr blockJacobiAdjacency(const Grid& grid, // Outch! We need to be able to scale the linear system! Hence const_cast matrix_ = const_cast(&M); - is_nldd_local_solver_ = parameters_[activeSolverNum_].is_nldd_local_solver_; - useWellConn_ = Parameters::Get(); // setup sparsity pattern for jacobi matrix for preconditioner (only used for openclSolver) } else { @@ -452,11 +450,16 @@ std::unique_ptr blockJacobiAdjacency(const Grid& grid, const CommunicationType* comm() const { return comm_.get(); } - void setDomainIndex(int index) + void setDomainIndex(const int index) { domainIndex_ = index; } + bool isNlddLocalSolver() const + { + return parameters_[activeSolverNum_].is_nldd_local_solver_; + } + protected: #if HAVE_MPI using Comm = Dune::OwnerOverlapCopyCommunication; @@ -497,7 +500,7 @@ std::unique_ptr blockJacobiAdjacency(const Grid& grid, OPM_TIMEBLOCK(flexibleSolverPrepare); if (shouldCreateSolver()) { if (!useWellConn_) { - if (is_nldd_local_solver_) { + if (isNlddLocalSolver()) { auto wellOp = std::make_unique>(simulator_.problem().wellModel()); wellOp->setDomainIndex(domainIndex_); flexibleSolver_[activeSolverNum_].wellOperator_ = std::move(wellOp); @@ -665,8 +668,6 @@ std::unique_ptr blockJacobiAdjacency(const Grid& grid, std::vector overlapRows_; std::vector interiorRows_; - bool is_nldd_local_solver_; - int domainIndex_ = -1; bool useWellConn_; diff --git a/opm/simulators/wells/BlackoilWellModel.hpp b/opm/simulators/wells/BlackoilWellModel.hpp index 21fd4de3f..385f7cf60 100644 --- a/opm/simulators/wells/BlackoilWellModel.hpp +++ b/opm/simulators/wells/BlackoilWellModel.hpp @@ -302,7 +302,7 @@ template class WellContributions; // subtract B*inv(D)*C * x from A*x void apply(const BVector& x, BVector& Ax) const; - void applyDomain(const BVector& x, BVector& Ax, int domainIndex) const; + void applyDomain(const BVector& x, BVector& Ax, const int domainIndex) const; #if COMPILE_BDA_BRIDGE // accumulate the contributions of all Wells in the WellContributions object @@ -312,7 +312,7 @@ template class WellContributions; // apply well model with scaling of alpha void applyScaleAdd(const Scalar alpha, const BVector& x, BVector& Ax) const; - void applyScaleAddDomain(const Scalar alpha, const BVector& x, BVector& Ax, int domainIndex) const; + void applyScaleAddDomain(const Scalar alpha, const BVector& x, BVector& Ax, const int domainIndex) const; // Check if well equations is converged. ConvergenceReport getWellConvergence(const std::vector& B_avg, const bool checkWellGroupControls = false) const; @@ -364,7 +364,10 @@ template class WellContributions; void addWellPressureEquations(PressureMatrix& jacobian, const BVector& weights,const bool use_well_weights) const; - void addWellPressureEquationsDomain(PressureMatrix& jacobian, const BVector& weights,const bool use_well_weights, int domainIndex) const; + void addWellPressureEquationsDomain([[maybe_unused]] PressureMatrix& jacobian, + [[maybe_unused]] const BVector& weights, + [[maybe_unused]] const bool use_well_weights, + [[maybe_unused]] const int domainIndex) const; void addWellPressureEquationsStruct(PressureMatrix& jacobian) const; @@ -594,6 +597,10 @@ template class WellContributions; private: BlackoilWellModel(Simulator& simulator, const PhaseUsage& pu); + // These members are used to avoid reallocation in specific functions + // (e.g., apply, applyDomain) instead of using local variables. + // Their state is not relevant between function calls, so they can + // (and must) be mutable, as the functions using them are const. mutable BVector x_local_; mutable BVector Ax_local_; mutable BVector res_local_; diff --git a/opm/simulators/wells/BlackoilWellModel_impl.hpp b/opm/simulators/wells/BlackoilWellModel_impl.hpp index aa1448d5b..f5edf3dee 100644 --- a/opm/simulators/wells/BlackoilWellModel_impl.hpp +++ b/opm/simulators/wells/BlackoilWellModel_impl.hpp @@ -1760,7 +1760,7 @@ namespace Opm { template void BlackoilWellModel:: - applyDomain(const BVector& x, BVector& Ax, int domainIndex) const + applyDomain(const BVector& x, BVector& Ax, const int domainIndex) const { for (size_t well_index = 0; well_index < well_container_.size(); ++well_index) { auto& well = well_container_[well_index]; @@ -1849,7 +1849,7 @@ namespace Opm { template void BlackoilWellModel:: - applyScaleAddDomain(const Scalar alpha, const BVector& x, BVector& Ax, int domainIndex) const + applyScaleAddDomain(const Scalar alpha, const BVector& x, BVector& Ax, const int domainIndex) const { if (this->well_container_.empty()) { return; @@ -1879,11 +1879,11 @@ namespace Opm { template void BlackoilWellModel:: - addWellPressureEquations(PressureMatrix& jacobian, const BVector& weights,const bool use_well_weights) const + addWellPressureEquations(PressureMatrix& jacobian, const BVector& weights, const bool use_well_weights) const { - int nw = this->numLocalWellsEnd(); + int nw = this->numLocalWellsEnd(); int rdofs = local_num_cells_; - for ( int i = 0; i < nw; i++ ){ + for ( int i = 0; i < nw; i++ ) { int wdof = rdofs + i; jacobian[wdof][wdof] = 1.0;// better scaling ? } @@ -1896,20 +1896,23 @@ namespace Opm { template void BlackoilWellModel:: - addWellPressureEquationsDomain(PressureMatrix& jacobian, const BVector& weights,const bool use_well_weights, int domainIndex) const + addWellPressureEquationsDomain([[maybe_unused]] PressureMatrix& jacobian, + [[maybe_unused]] const BVector& weights, + [[maybe_unused]] const bool use_well_weights, + [[maybe_unused]] const int domainIndex) const { throw std::logic_error("CPRW is not yet implemented for NLDD subdomains"); // To fix this function, rdofs should be the size of the domain, and the nw should be the number of wells in the domain - // int nw = this->numLocalWellsEnd(); // should number of wells in the domain + // int nw = this->numLocalWellsEnd(); // should number of wells in the domain // int rdofs = local_num_cells_; // should be the size of the domain - // for ( int i = 0; i < nw; i++ ){ + // for ( int i = 0; i < nw; i++ ) { // int wdof = rdofs + i; // jacobian[wdof][wdof] = 1.0;// better scaling ? // } // for ( const auto& well : well_container_ ) { // if (well_domain_.at(well->name()) == domainIndex) { - // weights should be the size of the domain + // weights should be the size of the domain // well->addWellPressureEquations(jacobian, weights, pressureVarIndex, use_well_weights, this->wellState()); // } // }