fixing comments from review of PR#1648

there should be no functional change

opm/autodiff/StandardWell.hpp

@@ -371,7 +371,7 @@ namespace Opm
 
         // turn on crossflow to avoid singular well equations
         // when the well is banned from cross-flow and the BHP is not properly initialized,
-        // we turn on crossflow to avoid singular well equations. It can result in worng-signed
+        // we turn on crossflow to avoid singular well equations. It can result in wrong-signed
         // well rates, it can cause problem for THP calculation
         // TODO: looking for better alternative to avoid wrong-signed well rates
         bool openCrossFlowAvoidSingularity(const Simulator& ebos_simulator) const;

opm/autodiff/StandardWell_impl.hpp

@@ -1114,7 +1114,7 @@ namespace Opm
         case THP: {
             assert(this->isOperable() );
 
-            // when a well can not work under THP target, it change to work under BHP target
+            // when a well can not work under THP target, it switches to BHP control
             if (this->operability_status_.isOperableUnderTHPLimit() ) {
                 updateWellStateWithTHPTargetIPR(ebos_simulator, well_state);
             } else { // go to BHP limit
@@ -1295,12 +1295,6 @@ namespace Opm
     StandardWell<TypeTag>::
     checkWellOperability(const Simulator& ebos_simulator)
     {
-        // TODO: this function is probably can split another function out so that
-        // wellTestingPhysical can share some code with this function
-        // on solution is that this function will be called updateWellOperability
-        // and the actual checking part become another function checkWellOperability
-        // Let us wait until finishing the wellTestingPhysical first.
-
         // focusing on PRODUCER for now
         if (well_type_ == INJECTOR) {
             return;
@@ -1319,8 +1313,6 @@ namespace Opm
         // checking the BHP limit related
         checkOperabilityUnderBHPLimitProducer(ebos_simulator);
 
-        // TODO: if the BHP limit does not work anyway, we do not need to do the following
-        // We do it now for studying purpose.
         // checking whether the well can operate under the THP constraints.
         if (this->wellHasTHPConstraints()) {
             checkOperabilityUnderTHPLimitProducer(ebos_simulator);
@@ -1349,8 +1341,10 @@ namespace Opm
     checkOperabilityUnderBHPLimitProducer(const Simulator& ebos_simulator)
     {
         const double bhp_limit = mostStrictBhpFromBhpLimits();
+        // Crude but works: default is one atmosphere.
         // TODO: a better way to detect whether the BHP is defaulted or not
-        if ( bhp_limit > 1.5e5 || !this->wellHasTHPConstraints() ) {
+        const bool bhp_limit_not_defaulted = bhp_limit > 1.5 * unit::barsa;
+        if ( bhp_limit_not_defaulted || !this->wellHasTHPConstraints() ) {
             // if the BHP limit is not defaulted or the well does not have a THP limit
             // we need to check the BHP limit
 
@@ -1367,7 +1361,6 @@ namespace Opm
                 // option 1: calculate well rates based on the BHP limit.
                 // option 2: stick with the above IPR curve
                 // we use IPR here
-                const double bhp_limit = mostStrictBhpFromBhpLimits();
                 std::vector<double> well_rates_bhp_limit;
                 computeWellRatesWithBhp(ebos_simulator, bhp_limit, well_rates_bhp_limit);
 
@@ -1379,13 +1372,13 @@ namespace Opm
                 }
             }
         } else {
-            // defaulted BHP and there is THP constraints
+            // defaulted BHP and there is a THP constraint
-            // default BHP limit is about 1.03 bar.
+            // default BHP limit is about 1 atm.
-            // when applied the hydrostatic pressure correction,
+            // when applied the hydrostatic pressure correction dp,
-            // most likely we get a negative bhp value to search in the VFP table,
+            // most likely we get a negative value (bhp + dp)to search in the VFP table,
-            // which is not desirable
+            // which is not desirable.
-            // we assume we can operate under thi BHP limit and will violate the THP limit
+            // we assume we can operate under defaulted BHP limit and will violate the THP limit
-            // when operating under this BHP limit
+            // when operating under defaulted BHP limit.
             this->operability_status_.operable_under_only_bhp_limit = true;
             this->operability_status_.obey_thp_limit_under_bhp_limit = false;
         }
@@ -1400,24 +1393,30 @@ namespace Opm
     StandardWell<TypeTag>::
     checkOperabilityUnderTHPLimitProducer(const Simulator& ebos_simulator)
     {
-        const double  bhp_limit = mostStrictBhpFromBhpLimits();
+        const double obtain_bhp =  calculateBHPWithTHPTargetIPR();
-        const double thp_limit = this->getTHPConstraint();
-        const double thp_control_index = this->getTHPControlIndex();
-        const  int table_id = well_controls_iget_vfp(well_controls_, thp_control_index);
-        const double alq = well_controls_iget_alq(well_controls_, thp_control_index);
 
-        const double vfp_ref_depth = vfp_properties_->getProd()->getTable(table_id)->getDatumDepth();
+        if (obtain_bhp > 0.) {
+            this->operability_status_.can_obtain_bhp_with_thp_limit = true;
 
-        // the density of the top perforation
+            const double  bhp_limit = mostStrictBhpFromBhpLimits();
-        const double rho = perf_densities_[0];
+            this->operability_status_.obey_bhp_limit_with_thp_limit = (obtain_bhp >= bhp_limit);
-
-        const double dp = (vfp_ref_depth - ref_depth_) * rho * gravity_;
-
-        vfp_properties_->getProd()->operabilityCheckingUnderTHP(ipr_a_, ipr_b_, bhp_limit,
-                                           table_id, thp_limit, alq, dp,
-                                           this->operability_status_.obtain_solution_with_thp_limit,
-                                           this->operability_status_.obey_bhp_limit_with_thp_limit );
 
+            const double thp_limit = this->getTHPConstraint();
+            if (obtain_bhp < thp_limit) {
+                const std::string msg = " obtained bhp " + std::to_string(unit::convert::to(obtain_bhp, unit::barsa))
+                                        + " bars is SMALLER than thp limit "
+                                        + std::to_string(unit::convert::to(thp_limit, unit::barsa))
+                                        + " bars as a producer for well " + name();
+                OpmLog::debug(msg);
+            }
+        } else {
+            this->operability_status_.can_obtain_bhp_with_thp_limit = false;
+            const double thp_limit = this->getTHPConstraint();
+            OpmLog::debug(" COULD NOT find bhp value under thp_limit "
+                          + std::to_string(unit::convert::to(thp_limit, unit::barsa))
+                          + " bars for well " + name() + ", the well might need to be closed ");
+            this->operability_status_.obey_bhp_limit_with_thp_limit = false;
+        }
     }
 
 
@@ -1445,7 +1444,7 @@ namespace Opm
 
             // for now, if there is one perforation can produce/inject in the correct
             // direction, we consider this well can still produce/inject.
-            // TODO: it can be more complicated than this to cause worng-signed rates
+            // TODO: it can be more complicated than this to cause wrong-signed rates
             if ( (drawdown < 0. && well_type_ == INJECTOR) ||
                  (drawdown > 0. && well_type_ == PRODUCER) )  {
                 all_drawdown_wrong_direction = false;
@@ -1505,6 +1504,8 @@ namespace Opm
 
         const double bhp = calculateBHPWithTHPTargetIPR();
 
+        assert(bhp > 0.0);
+
         well_state.bhp()[index_of_well_] = bhp;
 
         // TODO: explicit quantities are always tricky for this type of situation
@@ -1557,9 +1558,6 @@ namespace Opm
         const double obtain_bhp = vfp_properties_->getProd()->calculateBhpWithTHPTarget(ipr_a_, ipr_b_,
                                              bhp_limit, thp_table_id, thp_target, alq, dp);
 
-        // we should have made sure that this well should be operable under THP limit now
-        assert(obtain_bhp > 0.);
-
         return obtain_bhp;
     }
 

opm/autodiff/VFPProdProperties.cpp

@@ -229,10 +229,10 @@ calculateBhpWithTHPTarget(const std::vector<double>& ipr_a,
                                                                     detail::RateBhpPair{flo_bhp_limit, bhp_limit} };
 
     double obtain_bhp = 0.;
-    const bool obtain_solution_with_thp_limit = detail::findIntersectionForBhp(ratebhp_samples, ratebhp_twopoints_ipr, obtain_bhp);
+    const bool can_obtain_bhp_with_thp_limit = detail::findIntersectionForBhp(ratebhp_samples, ratebhp_twopoints_ipr, obtain_bhp);
 
-    // \Note: assuming not that negative BHP does not make sense
+    // \Note: assuming that negative BHP does not make sense
-    if (obtain_solution_with_thp_limit && obtain_bhp > 0.) {
+    if (can_obtain_bhp_with_thp_limit && obtain_bhp > 0.) {
         // getting too high bhp that might cause negative rates (rates in the undesired direction)
         if (obtain_bhp >= bhp_safe_limit) {
             const std::string msg (" We are getting a too high BHP value from the THP constraint, which may "
@@ -252,39 +252,4 @@ calculateBhpWithTHPTarget(const std::vector<double>& ipr_a,
 }
 
 
-
-
-void VFPProdProperties::
-operabilityCheckingUnderTHP(const std::vector<double>& ipr_a,
-                            const std::vector<double>& ipr_b,
-                            const double bhp_limit,
-                            const double thp_table_id,
-                            const double thp_limit,
-                            const double alq,
-                            const double dp,
-                            bool& obtain_solution_with_thp_limit,
-                            bool& obey_bhp_limit_with_thp_limit) const
-{
-    const double obtain_bhp = calculateBhpWithTHPTarget(ipr_a, ipr_b, bhp_limit, thp_table_id, thp_limit, alq, dp);
-
-    if (obtain_bhp > 0.) {
-        obtain_solution_with_thp_limit = true;
-
-        obey_bhp_limit_with_thp_limit = (obtain_bhp >= bhp_limit);
-
-        if (obtain_bhp < thp_limit) {
-            const std::string msg = " obtained bhp " + std::to_string(obtain_bhp / 1.e5) +
-                                    " is SMALLER than thp limit " + std::to_string(thp_limit / 1.e5) + " as a producer ";
-            OpmLog::debug(msg);
-        }
-
-    } else {
-        obtain_solution_with_thp_limit = false;
-        OpmLog::debug(" COULD NOT find bhp value under thp_limit " + std::to_string(thp_limit / 1.e5) +
-                      ", the well might need to be closed ");
-        obey_bhp_limit_with_thp_limit = true;
-    }
-}
-
-
 }

opm/autodiff/VFPProdProperties.hpp

@@ -184,16 +184,6 @@ public:
                                const double alq,
                                const double dp) const;
 
-    void operabilityCheckingUnderTHP(const std::vector<double>& ipr_a,
-                                     const std::vector<double>& ipr_b,
-                                     const double bhp_limit,
-                                     const double thp_table_id,
-                                     const double thp_limit,
-                                     const double alq,
-                                     const double dp,
-                                     bool& obtain_solution_with_thp_limit,
-                                     bool& voilate_bhp_limit_with_thp_limit) const;
-
 protected:
     // calculate a group bhp values with a group of flo rate values
     std::vector<double> bhpwithflo(const std::vector<double>& flos,

opm/autodiff/WellInterface.hpp

@@ -401,13 +401,13 @@ namespace Opm
         }
 
         bool isOperableUnderTHPLimit() const {
-            return obtain_solution_with_thp_limit && obey_bhp_limit_with_thp_limit;
+            return can_obtain_bhp_with_thp_limit && obey_bhp_limit_with_thp_limit;
         }
 
         void reset() {
             operable_under_only_bhp_limit = true;
             obey_thp_limit_under_bhp_limit = true;
-            obtain_solution_with_thp_limit = true;
+            can_obtain_bhp_with_thp_limit = true;
             obey_bhp_limit_with_thp_limit = true;
             // TODO: the following one might need to be treated differently
             existing_drawdown_correct_direction = true;
@@ -421,20 +421,17 @@ namespace Opm
         // the thp limit when operated under bhp limit
         bool obey_thp_limit_under_bhp_limit = true;
         // whether the well operate under the thp limit only
-        bool obtain_solution_with_thp_limit = true;
+        bool can_obtain_bhp_with_thp_limit = true;
         // whether the well obey bhp limit when operated under thp limit
         bool obey_bhp_limit_with_thp_limit = true;
 
         // there is some drawdown with correct sign/direction
         // if all the drawdown are with wrong sign/direction, it means producer can not produce
         // and injector can not inject.
-        // TODO: even not all the drawdown are with wrong sign, it is still possible that
+        // TODO: even there exist some drawdown with correct direction, it is still possible that
-        // producer can not produce and injector can not inject if the crossflow is allowed
+        // producer can not produce and injector can not inject if the crossflow is allowed, since
+        // the well rate is the sum of the rates from all the perofrations
         bool existing_drawdown_correct_direction = true;
-
-        // could not get converged, maybe at the end of the time step, after chopping for some steps.
-        // TODO: the best way is that this well can not get converged during local iterations.
-        // bool could_not_get_converged = false;
     };
 
 }

opm/autodiff/WellInterface_impl.hpp

@@ -459,9 +459,10 @@ namespace Opm
                     w, np, well_type_, wc, ctrl_index)) {
 
                 // if the well can not work under THP / BHP control, we should not switch to THP / BHP control
-                // TODO: does this mean the well is not operable anymore, while it is within the non-linear iteration?
+                const bool cannot_switch_to_bhp = well_controls_iget_type(wc, ctrl_index) == BHP && !operability_status_.isOperableUnderBHPLimit();
-                if ( !( well_controls_iget_type(wc, ctrl_index) == BHP && !operability_status_.isOperableUnderBHPLimit() ) &&
+                const bool cannot_switch_to_thp = well_controls_iget_type(wc, ctrl_index) == THP && !operability_status_.isOperableUnderTHPLimit();
-                     !( well_controls_iget_type(wc, ctrl_index) == THP && !operability_status_.isOperableUnderTHPLimit() ) ) {
+                const bool cannot_switch = cannot_switch_to_bhp || cannot_switch_to_thp;
+                if ( !cannot_switch ) {
 
                     // ctrl_index will be the index of the broken constraint after the loop.
                     break;
@@ -735,8 +736,8 @@ namespace Opm
             return;
         }
 
-        // if we understand correctly, only under prediction mode, we need to do well testing
+        // Based on current understanding, only under prediction mode, we need to shut well due to various
-        // TODO: which remains to be corrected
+        // reasons or limits. With more knowlage or testing cases later, this might need to be corrected.
         if (!underPredictionMode() ) {
             return;
         }