From 2760d2b09e85c976349d0c77a4a73b3b54f7e044 Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:35:17 +0100 Subject: [PATCH 01/14] vcfvstencil: use if constexpr this quells some static analyzer issues --- opm/models/discretization/vcfv/vcfvstencil.hh | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/opm/models/discretization/vcfv/vcfvstencil.hh b/opm/models/discretization/vcfv/vcfvstencil.hh index 734bc8480..59a44caa8 100644 --- a/opm/models/discretization/vcfv/vcfvstencil.hh +++ b/opm/models/discretization/vcfv/vcfvstencil.hh @@ -839,7 +839,12 @@ public: numVertices = e.subEntities(/*codim=*/dim); numEdges = e.subEntities(/*codim=*/dim-1); - numFaces = (dim<3)?0:e.subEntities(/*codim=*/1); + if constexpr (dim == 3) { + numFaces = e.subEntities(/*codim=*/1); + } + else { + numFaces = 0; + } numBoundarySegments_ = 0; // TODO: really required here(?) @@ -913,13 +918,13 @@ public: // cases which don't apply. LocalPosition ipLocal_; DimVector diffVec; - if (dim==1) { + if constexpr (dim == 1) { subContVolFace[k].ipLocal_ = 0.5; subContVolFace[k].normal_ = 1.0; subContVolFace[k].area_ = 1.0; ipLocal_ = subContVolFace[k].ipLocal_; } - else if (dim==2) { + else if constexpr (dim == 2) { ipLocal_ = referenceElement.position(static_cast(k), dim-1) + elementLocal; ipLocal_ *= 0.5; subContVolFace[k].ipLocal_ = ipLocal_; @@ -937,7 +942,7 @@ public: subContVolFace[k].area_ = subContVolFace[k].normal_.two_norm(); subContVolFace[k].normal_ /= subContVolFace[k].area_; } - else if (dim==3) { + else if constexpr (dim == 3) { unsigned leftFace; unsigned rightFace; getFaceIndices(numVertices, k, leftFace, rightFace); @@ -973,17 +978,17 @@ public: unsigned bfIdx = numBoundarySegments_; ++numBoundarySegments_; - if (dim == 1) { + if constexpr (dim == 1) { boundaryFace_[bfIdx].ipLocal_ = referenceElement.position(static_cast(vertInElement), dim); boundaryFace_[bfIdx].area_ = 1.0; } - else if (dim == 2) { + else if constexpr (dim == 2) { boundaryFace_[bfIdx].ipLocal_ = referenceElement.position(static_cast(vertInElement), dim) + referenceElement.position(static_cast(face), 1); boundaryFace_[bfIdx].ipLocal_ *= 0.5; boundaryFace_[bfIdx].area_ = 0.5 * intersection.geometry().volume(); } - else if (dim == 3) { + else if constexpr (dim == 3) { unsigned leftEdge; unsigned rightEdge; getEdgeIndices(numVertices, face, vertInElement, leftEdge, rightEdge); @@ -1113,12 +1118,12 @@ private: #endif void fillSubContVolData_() { - if (dim == 1) { + if constexpr (dim == 1) { // 1D subContVol[0].volume_ = 0.5*elementVolume; subContVol[1].volume_ = 0.5*elementVolume; } - else if (dim == 2) { + else if constexpr (dim == 2) { switch (numVertices) { case 3: // 2D, triangle subContVol[0].volume_ = elementVolume/3; @@ -1152,7 +1157,7 @@ private: +", numVertices = "+std::to_string(numVertices)); } } - else if (dim == 3) { + else if constexpr (dim == 3) { switch (numVertices) { case 4: // 3D, tetrahedron for (unsigned k = 0; k < numVertices; k++) From 98497c52e99544bf0783acbafff18808c8f808df Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:42:08 +0100 Subject: [PATCH 02/14] fixed: broken range check --- opm/simulators/flow/CollectDataOnIORank_impl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opm/simulators/flow/CollectDataOnIORank_impl.hpp b/opm/simulators/flow/CollectDataOnIORank_impl.hpp index b91e79b1e..538c39bbf 100644 --- a/opm/simulators/flow/CollectDataOnIORank_impl.hpp +++ b/opm/simulators/flow/CollectDataOnIORank_impl.hpp @@ -1098,7 +1098,7 @@ localIdxToGlobalIdx(unsigned localIdx) const throw std::logic_error("index map is not created on this rank"); } - if (localIdx > this->localIdxToGlobalIdx_.size()) { + if (localIdx >= this->localIdxToGlobalIdx_.size()) { throw std::logic_error("local index is outside map range"); } From 9ed93eeeccd0b3b75ccf6486743de439eacadb8e Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:42:32 +0100 Subject: [PATCH 03/14] fixed: check correct variable (copy-pasta issue) --- opm/simulators/flow/EclWriter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opm/simulators/flow/EclWriter.hpp b/opm/simulators/flow/EclWriter.hpp index e74b90b1f..11e014b6c 100644 --- a/opm/simulators/flow/EclWriter.hpp +++ b/opm/simulators/flow/EclWriter.hpp @@ -309,7 +309,7 @@ public: if (this->sub_step_report_.max_linear_iterations != 0) { miscSummaryData["NLINSMAX"] = this->sub_step_report_.max_linear_iterations; } - if (this->simulation_report_.success.total_newton_iterations != 0) { + if (this->simulation_report_.success.total_linear_iterations != 0) { miscSummaryData["MSUMLINS"] = this->simulation_report_.success.total_linear_iterations; } if (this->simulation_report_.success.total_newton_iterations != 0) { From e108e4b8c1ce8126c20db8be33e5b48a88c77252 Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:43:09 +0100 Subject: [PATCH 04/14] use c++ casts --- opm/simulators/linalg/HyprePreconditioner.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opm/simulators/linalg/HyprePreconditioner.hpp b/opm/simulators/linalg/HyprePreconditioner.hpp index 85306f863..d1f9cfe30 100644 --- a/opm/simulators/linalg/HyprePreconditioner.hpp +++ b/opm/simulators/linalg/HyprePreconditioner.hpp @@ -319,7 +319,7 @@ private: } HYPRE_IJMatrixAssemble(A_hypre_); - HYPRE_IJMatrixGetObject(A_hypre_, (void**)&parcsr_A_); + HYPRE_IJMatrixGetObject(A_hypre_, reinterpret_cast(&parcsr_A_)); } /** @@ -350,8 +350,8 @@ private: HYPRE_IJVectorAssemble(x_hypre_); HYPRE_IJVectorAssemble(b_hypre_); - HYPRE_IJVectorGetObject(x_hypre_, (void**)&par_x_); - HYPRE_IJVectorGetObject(b_hypre_, (void**)&par_b_); + HYPRE_IJVectorGetObject(x_hypre_, reinterpret_cast(&par_x_)); + HYPRE_IJVectorGetObject(b_hypre_, reinterpret_cast(&par_b_)); } /** From 4d2b07432c07ac6e06e49ba6b6b28c23e48029cf Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:45:32 +0100 Subject: [PATCH 05/14] fixed: rethrow exceptions using bare throw instead of making copies --- opm/simulators/linalg/gpubridge/opencl/ChowPatelIlu.cpp | 2 +- .../linalg/gpubridge/opencl/openclSolverBackend.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opm/simulators/linalg/gpubridge/opencl/ChowPatelIlu.cpp b/opm/simulators/linalg/gpubridge/opencl/ChowPatelIlu.cpp index 87f11b646..67a39cb58 100644 --- a/opm/simulators/linalg/gpubridge/opencl/ChowPatelIlu.cpp +++ b/opm/simulators/linalg/gpubridge/opencl/ChowPatelIlu.cpp @@ -978,7 +978,7 @@ void ChowPatelIlu::gpu_decomposition( OPM_THROW(std::logic_error, oss.str()); } catch (const std::logic_error& error) { // rethrow exception by OPM_THROW in the try{} - throw error; + throw; } } diff --git a/opm/simulators/linalg/gpubridge/opencl/openclSolverBackend.cpp b/opm/simulators/linalg/gpubridge/opencl/openclSolverBackend.cpp index e3599b65c..8a16a815f 100644 --- a/opm/simulators/linalg/gpubridge/opencl/openclSolverBackend.cpp +++ b/opm/simulators/linalg/gpubridge/opencl/openclSolverBackend.cpp @@ -219,7 +219,7 @@ openclSolverBackend(int verbosity_, OPM_THROW(std::logic_error, oss.str()); } catch (const std::logic_error& error) { // rethrow exception by OPM_THROW in the try{}, without this, a segfault occurs - throw error; + throw; } } @@ -479,7 +479,7 @@ initialize(std::shared_ptr> matrix, OPM_THROW(std::logic_error, oss.str()); } catch (const std::logic_error& error) { // rethrow exception by OPM_THROW in the try{}, without this, a segfault occurs - throw error; + throw; } initialized = true; @@ -649,7 +649,7 @@ solve_system(WellContributions& wellContribs, GpuResult& res) OPM_THROW(std::logic_error, oss.str()); } catch (const std::logic_error& error) { // rethrow exception by OPM_THROW in the try{}, without this, a segfault occurs - throw error; + throw; } if (verbosity > 2) { From 8f293ba5e5c024c8d09a7ae2bdaaea5df357b98b Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:46:55 +0100 Subject: [PATCH 06/14] add some missing initializers --- .../multiphasebaseextensivequantities.hh | 4 +-- opm/models/common/transfluxmodule.hh | 8 ++--- .../common/baseauxiliarymodule.hh | 2 +- .../common/fvbaseextensivequantities.hh | 6 ++-- .../vcfv/p1fegradientcalculator.hh | 6 ++-- .../satfunc/GasPhaseConsistencyChecks.hpp | 10 +++--- .../satfunc/OilPhaseConsistencyChecks.hpp | 36 +++++++++---------- .../utils/satfunc/RelpermDiagnostics.hpp | 6 ++-- .../ThreePointHorizontalConsistencyChecks.hpp | 16 ++++----- .../satfunc/WaterPhaseConsistencyChecks.hpp | 10 +++--- 10 files changed, 52 insertions(+), 52 deletions(-) diff --git a/opm/models/common/multiphasebaseextensivequantities.hh b/opm/models/common/multiphasebaseextensivequantities.hh index b372cd129..0438d8b62 100644 --- a/opm/models/common/multiphasebaseextensivequantities.hh +++ b/opm/models/common/multiphasebaseextensivequantities.hh @@ -167,8 +167,8 @@ public: { return 1.0 - upstreamWeight(phaseIdx); } private: - short upstreamScvIdx_[numPhases]; - short downstreamScvIdx_[numPhases]; + short upstreamScvIdx_[numPhases]{}; + short downstreamScvIdx_[numPhases]{}; }; } // namespace Opm diff --git a/opm/models/common/transfluxmodule.hh b/opm/models/common/transfluxmodule.hh index 7f9797cef..2a72d8149 100644 --- a/opm/models/common/transfluxmodule.hh +++ b/opm/models/common/transfluxmodule.hh @@ -500,10 +500,10 @@ private: Evaluation pressureDifference_[numPhases]; // the local indices of the interior and exterior degrees of freedom - unsigned short interiorDofIdx_; - unsigned short exteriorDofIdx_; - short upIdx_[numPhases]; - short dnIdx_[numPhases]; + unsigned short interiorDofIdx_{}; + unsigned short exteriorDofIdx_{}; + short upIdx_[numPhases]{}; + short dnIdx_[numPhases]{}; }; } // namespace Opm diff --git a/opm/models/discretization/common/baseauxiliarymodule.hh b/opm/models/discretization/common/baseauxiliarymodule.hh index 61dc6624d..2cea8a54d 100644 --- a/opm/models/discretization/common/baseauxiliarymodule.hh +++ b/opm/models/discretization/common/baseauxiliarymodule.hh @@ -122,7 +122,7 @@ public: {}; private: - int dofOffset_; + int dofOffset_{}; }; } // namespace Opm diff --git a/opm/models/discretization/common/fvbaseextensivequantities.hh b/opm/models/discretization/common/fvbaseextensivequantities.hh index 35c97cbb6..0ba451e08 100644 --- a/opm/models/discretization/common/fvbaseextensivequantities.hh +++ b/opm/models/discretization/common/fvbaseextensivequantities.hh @@ -124,10 +124,10 @@ public: private: // local indices of the interior and the exterior sub-control-volumes - unsigned short interiorScvIdx_; - unsigned short exteriorScvIdx_; + unsigned short interiorScvIdx_{}; + unsigned short exteriorScvIdx_{}; - Scalar extrusionFactor_; + Scalar extrusionFactor_{}; }; } // namespace Opm diff --git a/opm/models/discretization/vcfv/p1fegradientcalculator.hh b/opm/models/discretization/vcfv/p1fegradientcalculator.hh index 4149fef08..a1731cf56 100644 --- a/opm/models/discretization/vcfv/p1fegradientcalculator.hh +++ b/opm/models/discretization/vcfv/p1fegradientcalculator.hh @@ -341,9 +341,9 @@ private: #if HAVE_DUNE_LOCALFUNCTIONS static LocalFiniteElementCache feCache_; - const LocalFiniteElement* localFiniteElement_; - std::vector> p1Value_[maxFap]; - DimVector p1Gradient_[maxFap][maxDof]; + const LocalFiniteElement* localFiniteElement_{nullptr}; + std::vector> p1Value_[maxFap]{}; + DimVector p1Gradient_[maxFap][maxDof]{}; #endif // HAVE_DUNE_LOCALFUNCTIONS }; diff --git a/opm/simulators/utils/satfunc/GasPhaseConsistencyChecks.hpp b/opm/simulators/utils/satfunc/GasPhaseConsistencyChecks.hpp index 7f015ff4d..ef325763c 100644 --- a/opm/simulators/utils/satfunc/GasPhaseConsistencyChecks.hpp +++ b/opm/simulators/utils/satfunc/GasPhaseConsistencyChecks.hpp @@ -70,7 +70,7 @@ namespace Opm::Satfunc::PhaseChecks::Gas { private: /// Minimum gas saturation. - Scalar sgl_; + Scalar sgl_{}; /// Run check against a set of saturation function end-points. /// @@ -124,7 +124,7 @@ namespace Opm::Satfunc::PhaseChecks::Gas { private: /// Maximum gas saturation. - Scalar sgu_; + Scalar sgu_{}; /// Run check against a set of saturation function end-points. /// @@ -182,13 +182,13 @@ namespace Opm::Satfunc::PhaseChecks::Gas { private: /// Minimum gas saturation. - Scalar sgl_; + Scalar sgl_{}; /// Critical gas saturation. - Scalar sgcr_; + Scalar sgcr_{}; /// Maximum gas saturation. - Scalar sgu_; + Scalar sgu_{}; /// Run check against a set of saturation function end-points. /// diff --git a/opm/simulators/utils/satfunc/OilPhaseConsistencyChecks.hpp b/opm/simulators/utils/satfunc/OilPhaseConsistencyChecks.hpp index 9c346d433..eeea3f775 100644 --- a/opm/simulators/utils/satfunc/OilPhaseConsistencyChecks.hpp +++ b/opm/simulators/utils/satfunc/OilPhaseConsistencyChecks.hpp @@ -70,7 +70,7 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Critical oil saturation in gas/oil system. - Scalar sogcr_; + Scalar sogcr_{}; /// Run check against a set of saturation function end-points. /// @@ -128,10 +128,10 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum (connate) water saturation in gas/oil system. - Scalar swl_; + Scalar swl_{}; /// Maximum gas saturation in gas/oil system. - Scalar sgu_; + Scalar sgu_{}; /// Run check against a set of saturation function end-points. /// @@ -192,13 +192,13 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum water saturation. - Scalar swl_; + Scalar swl_{}; /// Minimum gas saturation. - Scalar sgl_; + Scalar sgl_{}; /// Critical oil saturation in gas/oil system. - Scalar sogcr_; + Scalar sogcr_{}; /// Run check against a set of saturation function end-points. /// @@ -259,13 +259,13 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum water saturation. - Scalar swl_; + Scalar swl_{}; /// Critical gas saturation. - Scalar sgcr_; + Scalar sgcr_{}; /// Critical oil saturation in gas/oil system - Scalar sogcr_; + Scalar sogcr_{}; /// Run check against a set of saturation function end-points. /// @@ -321,7 +321,7 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Critical oil saturation in oil/water system. - Scalar sowcr_; + Scalar sowcr_{}; /// Run check against a set of saturation function end-points. /// @@ -379,10 +379,10 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum gas saturation. Typically zero. - Scalar sgl_; + Scalar sgl_{}; /// Minimum (connate) saturation. - Scalar swu_; + Scalar swu_{}; /// Run check against a set of saturation function end-points. /// @@ -443,13 +443,13 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum (connate) water saturation. - Scalar swl_; + Scalar swl_{}; /// Minimum gas saturation. Typically zero. - Scalar sgl_; + Scalar sgl_{}; /// Critical oil saturation in oil/water system. - Scalar sowcr_; + Scalar sowcr_{}; /// Run check against a set of saturation function end-points. /// @@ -510,13 +510,13 @@ namespace Opm::Satfunc::PhaseChecks::Oil { private: /// Minimum gas saturation. Typically zero. - Scalar sgl_; + Scalar sgl_{}; /// Critical water saturation. - Scalar swcr_; + Scalar swcr_{}; /// Critical oil saturation in oil/water system. - Scalar sowcr_; + Scalar sowcr_{}; /// Run check against a set of saturation function end-points. /// diff --git a/opm/simulators/utils/satfunc/RelpermDiagnostics.hpp b/opm/simulators/utils/satfunc/RelpermDiagnostics.hpp index 4ea30d145..efd3fe435 100644 --- a/opm/simulators/utils/satfunc/RelpermDiagnostics.hpp +++ b/opm/simulators/utils/satfunc/RelpermDiagnostics.hpp @@ -77,10 +77,10 @@ namespace Opm { NoFamily }; - SaturationFunctionFamily satFamily_; + SaturationFunctionFamily satFamily_{NoFamily}; - std::vector > unscaledEpsInfo_; - std::vector > scaledEpsInfo_; + std::vector > unscaledEpsInfo_{}; + std::vector > scaledEpsInfo_{}; ///Check the phase that used. diff --git a/opm/simulators/utils/satfunc/ThreePointHorizontalConsistencyChecks.hpp b/opm/simulators/utils/satfunc/ThreePointHorizontalConsistencyChecks.hpp index 1261bb6b8..355f9fb8e 100644 --- a/opm/simulators/utils/satfunc/ThreePointHorizontalConsistencyChecks.hpp +++ b/opm/simulators/utils/satfunc/ThreePointHorizontalConsistencyChecks.hpp @@ -82,16 +82,16 @@ namespace Opm::Satfunc::PhaseChecks::ThreePointHorizontal { private: /// Minimum (connate) water saturation. - Scalar swl_; + Scalar swl_{}; /// Critical oil saturation in two-phase gas/oil system. - Scalar sogcr_; + Scalar sogcr_{}; /// Critical gas saturation. - Scalar sgcr_; + Scalar sgcr_{}; /// Maximum gas saturation. - Scalar sgu_; + Scalar sgu_{}; /// Run check against a set of saturation function end-points. /// @@ -157,16 +157,16 @@ namespace Opm::Satfunc::PhaseChecks::ThreePointHorizontal { private: /// Minimum gas saturation. - Scalar sgl_; + Scalar sgl_{}; /// Critical oil saturation in two-phase oil/water system. - Scalar sowcr_; + Scalar sowcr_{}; /// Critical water saturation. - Scalar swcr_; + Scalar swcr_{}; /// Maximum water saturation. - Scalar swu_; + Scalar swu_{}; /// Run check against a set of saturation function end-points. /// diff --git a/opm/simulators/utils/satfunc/WaterPhaseConsistencyChecks.hpp b/opm/simulators/utils/satfunc/WaterPhaseConsistencyChecks.hpp index aab6db022..22a9b10dd 100644 --- a/opm/simulators/utils/satfunc/WaterPhaseConsistencyChecks.hpp +++ b/opm/simulators/utils/satfunc/WaterPhaseConsistencyChecks.hpp @@ -70,7 +70,7 @@ namespace Opm::Satfunc::PhaseChecks::Water { private: /// Minimum (connate) water saturation. - Scalar swl_; + Scalar swl_{}; /// Run check against a set of saturation function end-points. /// @@ -124,7 +124,7 @@ namespace Opm::Satfunc::PhaseChecks::Water { private: /// Maximum water saturation. - Scalar swu_; + Scalar swu_{}; /// Run check against a set of saturation function end-points. /// @@ -182,13 +182,13 @@ namespace Opm::Satfunc::PhaseChecks::Water { private: /// Minimum (connate) water saturation. - Scalar swl_; + Scalar swl_{}; /// Critical water saturation. - Scalar swcr_; + Scalar swcr_{}; /// Maximum water saturation. - Scalar swu_; + Scalar swu_{}; /// Run check against a set of saturation function end-points. /// From b8ce6629efd1d69b969b62841d8b8f4656715e5b Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:47:58 +0100 Subject: [PATCH 07/14] fix some header vs implementation parameter name inconsistencies --- opm/simulators/linalg/gpubridge/opencl/openclBISAI.cpp | 6 +++--- opm/simulators/wells/GasLiftSingleWellGeneric.hpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/opm/simulators/linalg/gpubridge/opencl/openclBISAI.cpp b/opm/simulators/linalg/gpubridge/opencl/openclBISAI.cpp index e8cff6a1a..8f38a1fa9 100644 --- a/opm/simulators/linalg/gpubridge/opencl/openclBISAI.cpp +++ b/opm/simulators/linalg/gpubridge/opencl/openclBISAI.cpp @@ -341,16 +341,16 @@ create_preconditioner(BlockedMatrix* mat) } template -void openclBISAI::apply(const cl::Buffer& x, cl::Buffer& y) +void openclBISAI::apply(const cl::Buffer& y, cl::Buffer& x) { const unsigned int bs = block_size; OpenclKernels::spmv(d_invLvals, d_rowIndices, d_colPointers, - x, d_invL_x, Nb, bs, true, true); // application of isaiL is a simple spmv with addition + y, d_invL_x, Nb, bs, true, true); // application of isaiL is a simple spmv with addition // (to compensate for the unitary diagonal that is not // included in isaiL, for simplicity) OpenclKernels::spmv(d_invUvals, d_rowIndices, d_colPointers, - d_invL_x, y, Nb, bs); // application of isaiU is a simple spmv + d_invL_x, x, Nb, bs); // application of isaiU is a simple spmv } #define INSTANTIATE_TYPE(T) \ diff --git a/opm/simulators/wells/GasLiftSingleWellGeneric.hpp b/opm/simulators/wells/GasLiftSingleWellGeneric.hpp index 74af05270..7f7471496 100644 --- a/opm/simulators/wells/GasLiftSingleWellGeneric.hpp +++ b/opm/simulators/wells/GasLiftSingleWellGeneric.hpp @@ -412,8 +412,8 @@ protected: Scalar delta_alq) const; LimitedRates - updateRatesToGroupLimits_(const BasicRates& rates, - const LimitedRates& new_rates, + updateRatesToGroupLimits_(const BasicRates& old_rates, + const LimitedRates& rates, const std::string& gr_name = "") const; void updateWellStateAlqFixedValue_(const GasLiftWell& well); From 4ff851af9c1813fe5f58383d620f527c9cd05d4b Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:48:30 +0100 Subject: [PATCH 08/14] WellState: replace some loops with standard algorithms also addresses some static analyzer issues --- opm/simulators/wells/WellState.cpp | 60 ++++++++++++++---------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/opm/simulators/wells/WellState.cpp b/opm/simulators/wells/WellState.cpp index 561546cc7..4f5a65de2 100644 --- a/opm/simulators/wells/WellState.cpp +++ b/opm/simulators/wells/WellState.cpp @@ -919,48 +919,44 @@ template void WellState::communicateGroupRates(const Parallel::Communication& comm) { // Compute the size of the data. - std::size_t sz = 0; - for (const auto& [_, owner_rates] : this->well_rates) { - (void)_; - const auto& [__, rates] = owner_rates; - (void)__; - sz += rates.size(); - } + std::size_t sz = std::accumulate(this->well_rates.begin(), this->well_rates.end(), std::size_t{0}, + [](const std::size_t acc, const auto& rates) + { return acc + rates.second.second.size(); }); sz += this->alq_state.pack_size(); // Make a vector and collect all data into it. std::vector data(sz); - std::size_t pos = 0; - for (const auto& [_, owner_rates] : this->well_rates) { - (void)_; - const auto& [owner, rates] = owner_rates; - for (const auto& value : rates) { - if (owner) - data[pos++] = value; - else - data[pos++] = 0; - } + auto pos = data.begin(); + std::for_each(this->well_rates.begin(), this->well_rates.end(), + [&pos](const auto& input) + { + const auto& [owner, rates] = input.second; + if (owner) { + std::copy(rates.begin(), rates.end(), pos); + } + pos += rates.size(); + }); + + if (pos != data.end()) { + pos += this->alq_state.pack_data(&(*pos)); } - if (!data.empty() && pos != sz) { - pos += this->alq_state.pack_data(&data[pos]); - } - assert(pos == sz); + assert(pos == data.end()); // Communicate it with a single sum() call. comm.sum(data.data(), data.size()); - pos = 0; - for (auto& [_, owner_rates] : this->well_rates) { - (void)_; - auto& [__, rates] = owner_rates; - (void)__; - for (auto& value : rates) - value = data[pos++]; + pos = data.begin(); + std::for_each(this->well_rates.begin(), this->well_rates.end(), + [&pos](auto& input) + { + auto& rates = input.second.second; + std::copy(pos, pos + rates.size(), rates.begin()); + pos += rates.size(); + }); + if (pos != data.end()) { + pos += this->alq_state.unpack_data(&(*pos)); } - if (!data.empty() && pos != sz) { - pos += this->alq_state.unpack_data(&data[pos]); - } - assert(pos == sz); + assert(pos == data.end()); } template From de986943f0d7a8eba2d677a14eb7a69de5e1dc6e Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:50:48 +0100 Subject: [PATCH 09/14] remove some unused bools these loops are inf-loops-with-break and the bools are never used --- opm/simulators/wells/GasLiftSingleWellGeneric.cpp | 12 ++++-------- opm/simulators/wells/WellBhpThpCalculator.cpp | 3 +-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/opm/simulators/wells/GasLiftSingleWellGeneric.cpp b/opm/simulators/wells/GasLiftSingleWellGeneric.cpp index c7f760b31..51efb9711 100644 --- a/opm/simulators/wells/GasLiftSingleWellGeneric.cpp +++ b/opm/simulators/wells/GasLiftSingleWellGeneric.cpp @@ -1048,11 +1048,10 @@ GasLiftSingleWellGeneric:: increaseALQtoPositiveOilRate_(Scalar alq, const LimitedRates& orig_rates) const { - bool stop_iteration = false; Scalar temp_alq = alq; // use the copy constructor to only copy the rates BasicRates rates = orig_rates; - while (!stop_iteration) { + while (true) { temp_alq += this->increment_; if (temp_alq > this->max_alq_) break; @@ -1078,10 +1077,9 @@ increaseALQtoMinALQ_(const Scalar orig_alq, assert(min_alq >= 0); assert(orig_alq < min_alq); assert(min_alq < this->max_alq_); - bool stop_iteration = false; Scalar alq = orig_alq; LimitedRates rates = orig_rates; - while (!stop_iteration) { + while (true) { Scalar temp_alq = alq + this->increment_; alq = temp_alq; @@ -1175,11 +1173,10 @@ GasLiftSingleWellGeneric:: reduceALQtoGroupAlqLimits_(const Scalar orig_alq, const LimitedRates& orig_rates) const { - bool stop_this_iteration = false; Scalar alq = orig_alq; BasicRates rates {orig_rates}; Scalar temp_alq = orig_alq; - while (!stop_this_iteration) { + while (true) { if (temp_alq == 0) break; temp_alq -= this->increment_; @@ -1268,8 +1265,7 @@ reduceALQtoWellTarget_(const Scalar orig_alq, Scalar alq = orig_alq; Scalar temp_alq = alq; std::optional new_rates; - bool stop_iteration = false; - while (!stop_iteration) { + while (true) { if (temp_alq == 0) break; temp_alq -= this->increment_; diff --git a/opm/simulators/wells/WellBhpThpCalculator.cpp b/opm/simulators/wells/WellBhpThpCalculator.cpp index 8407c0908..80c0a243d 100644 --- a/opm/simulators/wells/WellBhpThpCalculator.cpp +++ b/opm/simulators/wells/WellBhpThpCalculator.cpp @@ -171,10 +171,9 @@ findThpFromBhpIteratively(const std::function max_iterations) { break; } From fe3c687eaa2de41ddd9b88d31e0f868b7da114cf Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:52:32 +0100 Subject: [PATCH 10/14] remove some duplicate if conditionals --- opm/models/ptflash/flashintensivequantities.hh | 16 +++++----------- .../flow/SimulatorFullyImplicitBlackoil.hpp | 3 --- .../wells/BlackoilWellModelGeneric.cpp | 2 -- opm/simulators/wells/BlackoilWellModel_impl.hpp | 5 ++--- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/opm/models/ptflash/flashintensivequantities.hh b/opm/models/ptflash/flashintensivequantities.hh index 868844876..7e71ef5a3 100644 --- a/opm/models/ptflash/flashintensivequantities.hh +++ b/opm/models/ptflash/flashintensivequantities.hh @@ -231,17 +231,11 @@ public: fluidState_.setCompressFactor(1, Z_V); // Print saturation - if (flashVerbosity >= 5) { - std::cout << "So = " << So < Date: Thu, 23 Jan 2025 10:53:29 +0100 Subject: [PATCH 11/14] remove some always true/false conditionals quell static analyzer issues --- opm/models/blackoil/blackoildispersionmodule.hh | 7 +++++-- opm/models/pvs/pvsmodel.hh | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/opm/models/blackoil/blackoildispersionmodule.hh b/opm/models/blackoil/blackoildispersionmodule.hh index 346b7f415..c524b7535 100644 --- a/opm/models/blackoil/blackoildispersionmodule.hh +++ b/opm/models/blackoil/blackoildispersionmodule.hh @@ -194,14 +194,17 @@ public: } // Adding dispersion in the gas phase leads to - // convergence issues and unphysical results. + // convergence issues and unphysical results. // We disable dispersion in the gas phase for now + // See comment below if (FluidSystem::gasPhaseIdx == phaseIdx) { continue; } // no dispersion in gas for blackoil models unless gas can contain evaporated water or oil - if ((!FluidSystem::enableVaporizedWater() && !FluidSystem::enableVaporizedOil()) && FluidSystem::gasPhaseIdx == phaseIdx) { + // phase check disabled due to if above, reenable when removing unconditional gas phase disablement + if ((!FluidSystem::enableVaporizedWater() && !FluidSystem::enableVaporizedOil()) + /*&& FluidSystem::gasPhaseIdx == phaseIdx*/) { continue; } diff --git a/opm/models/pvs/pvsmodel.hh b/opm/models/pvs/pvsmodel.hh index 8b93d9f20..8bdec1300 100644 --- a/opm/models/pvs/pvsmodel.hh +++ b/opm/models/pvs/pvsmodel.hh @@ -581,13 +581,13 @@ public: using FsToolbox = Opm::MathToolbox; for (unsigned phaseIdx = 0; phaseIdx < numPhases; ++phaseIdx) { - bool oldPhasePresent = (oldPhasePresence& (1 << phaseIdx)) > 0; + bool oldPhasePresent = (oldPhasePresence & (1 << phaseIdx)) > 0; bool newPhasePresent = newPv.phaseIsPresent(phaseIdx); if (oldPhasePresent == newPhasePresent) continue; const auto& pos = elemCtx.pos(dofIdx, /*timeIdx=*/0); - if (oldPhasePresent && !newPhasePresent) { + if (oldPhasePresent) { std::cout << "'" << FluidSystem::phaseName(phaseIdx) << "' phase disappears at position " << pos << ". saturation=" << fs.saturation(phaseIdx) From 6d2fe4334af69ab4f9517e25eeabb22e96501e5f Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:54:00 +0100 Subject: [PATCH 12/14] remove unnecessary arithmetic --- examples/problems/co2injectionproblem.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/problems/co2injectionproblem.hh b/examples/problems/co2injectionproblem.hh index d2cc86c77..5dd893340 100644 --- a/examples/problems/co2injectionproblem.hh +++ b/examples/problems/co2injectionproblem.hh @@ -571,7 +571,7 @@ private: const auto& matParams = this->materialLawParams(context, spaceIdx, timeIdx); MaterialLaw::capillaryPressures(pC, matParams, fs); - fs.setPressure(liquidPhaseIdx, pl + (pC[liquidPhaseIdx] - pC[liquidPhaseIdx])); + fs.setPressure(liquidPhaseIdx, pl); fs.setPressure(gasPhaseIdx, pl + (pC[gasPhaseIdx] - pC[liquidPhaseIdx])); ////// From 043f743ccaf736d9ed238ae74bb71e08a9f69cc8 Mon Sep 17 00:00:00 2001 From: Arne Morten Kvarving Date: Thu, 23 Jan 2025 10:54:36 +0100 Subject: [PATCH 13/14] use an explicit cast to better communicate intent and mark some variables const while at it --- opm/simulators/flow/Transmissibility_impl.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/opm/simulators/flow/Transmissibility_impl.hpp b/opm/simulators/flow/Transmissibility_impl.hpp index 8d38778ec..60c1925c6 100644 --- a/opm/simulators/flow/Transmissibility_impl.hpp +++ b/opm/simulators/flow/Transmissibility_impl.hpp @@ -61,10 +61,10 @@ namespace details { std::uint64_t isId(std::uint32_t elemIdx1, std::uint32_t elemIdx2) { - std::uint32_t elemAIdx = std::min(elemIdx1, elemIdx2); - std::uint64_t elemBIdx = std::max(elemIdx1, elemIdx2); + const std::uint32_t elemAIdx = std::min(elemIdx1, elemIdx2); + const std::uint64_t elemBIdx = std::max(elemIdx1, elemIdx2); - return (elemBIdx< isIdReverse(const std::uint64_t& id) @@ -72,15 +72,15 @@ namespace details { // Assigning an unsigned integer to a narrower type discards the most significant bits. // See "The C programming language", section A.6.2. // NOTE that the ordering of element A and B may have changed - std::uint32_t elemAIdx = id; - std::uint32_t elemBIdx = (id - elemAIdx) >> elemIdxShift; + const std::uint32_t elemAIdx = static_cast(id); + const std::uint32_t elemBIdx = (id - elemAIdx) >> elemIdxShift; return std::make_pair(elemAIdx, elemBIdx); } std::uint64_t directionalIsId(std::uint32_t elemIdx1, std::uint32_t elemIdx2) { - return (std::uint64_t(elemIdx1)< Date: Thu, 23 Jan 2025 10:55:25 +0100 Subject: [PATCH 14/14] reduce scope of variables remove unused variables break some long lines while at it --- .../wells/MultisegmentWell_impl.hpp | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/opm/simulators/wells/MultisegmentWell_impl.hpp b/opm/simulators/wells/MultisegmentWell_impl.hpp index f9a012652..da033094e 100644 --- a/opm/simulators/wells/MultisegmentWell_impl.hpp +++ b/opm/simulators/wells/MultisegmentWell_impl.hpp @@ -1656,32 +1656,28 @@ namespace Opm // if we fail to solve eqs, we reset status/operability before leaving const auto well_status_orig = this->wellStatus_; const auto operability_orig = this->operability_status_; - auto well_status_cur = well_status_orig; // don't allow opening wells that are stopped from schedule or has a stopped well state const bool allow_open = this->well_ecl_.getStatus() == WellStatus::OPEN && well_state.well(this->index_of_well_).status == WellStatus::OPEN; // don't allow switcing for wells under zero rate target or requested fixed status and control const bool allow_switching = !this->wellUnderZeroRateTarget(simulator, well_state, deferred_logger) && (!fixed_control || !fixed_status) && allow_open; - bool changed = false; bool final_check = false; // well needs to be set operable or else solving/updating of re-opened wells is skipped this->operability_status_.resetOperability(); this->operability_status_.solvable = true; for (; it < max_iter_number; ++it, ++debug_cost_counter_) { - its_since_last_switch++; + ++its_since_last_switch; if (allow_switching && its_since_last_switch >= min_its_after_switch){ const Scalar wqTotal = this->primary_variables_.getWQTotal().value(); - changed = this->updateWellControlAndStatusLocalIteration(simulator, well_state, group_state, - inj_controls, prod_controls, wqTotal, - deferred_logger, fixed_control, fixed_status); - if (changed){ + bool changed = this->updateWellControlAndStatusLocalIteration(simulator, well_state, group_state, + inj_controls, prod_controls, wqTotal, + deferred_logger, fixed_control, + fixed_status); + if (changed) { its_since_last_switch = 0; - switch_count++; - if (well_status_cur != this->wellStatus_) { - well_status_cur = this->wellStatus_; - } + ++switch_count; } if (!changed && final_check) { break; @@ -1690,7 +1686,8 @@ namespace Opm } } - assembleWellEqWithoutIteration(simulator, dt, inj_controls, prod_controls, well_state, group_state, deferred_logger); + assembleWellEqWithoutIteration(simulator, dt, inj_controls, prod_controls, + well_state, group_state, deferred_logger); const BVectorWell dx_well = this->linSys_.solve();