From f34f221be864ab3b13fe5d7564c8a7d302c3eb78 Mon Sep 17 00:00:00 2001 From: Joakim Hove Date: Thu, 29 Oct 2020 15:39:46 +0100 Subject: [PATCH 1/2] Properly initialize UDA values for group production and injection --- .../EclipseState/Schedule/Group/Group.hpp | 9 +++-- .../EclipseState/Schedule/Group/Group.cpp | 37 ++++++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/opm/parser/eclipse/EclipseState/Schedule/Group/Group.hpp b/opm/parser/eclipse/EclipseState/Schedule/Group/Group.hpp index 8ac1337e0..e5c40585f 100644 --- a/opm/parser/eclipse/EclipseState/Schedule/Group/Group.hpp +++ b/opm/parser/eclipse/EclipseState/Schedule/Group/Group.hpp @@ -116,6 +116,9 @@ static GuideRateTarget GuideRateTargetFromString( const std::string& stringValue struct GroupInjectionProperties { + GroupInjectionProperties(); + GroupInjectionProperties(Phase phase, const UnitSystem& unit_system); + Phase phase = Phase::WATER; InjectionCMode cmode = InjectionCMode::NONE; UDAValue surface_max_rate; @@ -162,10 +165,8 @@ struct InjectionControls { }; struct GroupProductionProperties { - GroupProductionProperties() = default; - GroupProductionProperties(const std::string& gname) : - name(gname) - {} + GroupProductionProperties(); + GroupProductionProperties(const UnitSystem& unit_system, const std::string& gname); std::string name; ProductionCMode gconprod_cmode = ProductionCMode::NONE; diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/Group/Group.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/Group/Group.cpp index 81ad3fe8a..40e105058 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/Group/Group.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/Group/Group.cpp @@ -34,6 +34,10 @@ Group::Group() { } + + + + Group::Group(const std::string& name, std::size_t insert_index_arg, std::size_t init_step_arg, double udq_undefined_arg, const UnitSystem& unit_system_arg) : m_name(name), m_insert_index(insert_index_arg), @@ -44,7 +48,7 @@ Group::Group(const std::string& name, std::size_t insert_index_arg, std::size_t gefac(1), transfer_gefac(true), vfp_table(0), - production_properties(name) + production_properties(unit_system, name) { // All groups are initially created as children of the "FIELD" group. if (name != "FIELD") @@ -176,6 +180,23 @@ bool Group::updateProduction(const GroupProductionProperties& production) { return update; } +Group::GroupInjectionProperties::GroupInjectionProperties() : + GroupInjectionProperties(Phase::WATER, UnitSystem(UnitSystem::UnitType::UNIT_TYPE_METRIC)) +{} + +Group::GroupInjectionProperties::GroupInjectionProperties(Phase phase_arg, const UnitSystem& unit_system) : + phase(phase_arg), + target_reinj_fraction(unit_system.getDimension(UnitSystem::measure::identity)), + target_void_fraction(unit_system.getDimension(UnitSystem::measure::identity)) +{ + if (phase == Phase::WATER) { + this->surface_max_rate = UDAValue(unit_system.getDimension(UnitSystem::measure::liquid_surface_rate)); + this->resv_max_rate = UDAValue(unit_system.getDimension(UnitSystem::measure::rate)); + } else { + this->surface_max_rate = UDAValue(unit_system.getDimension(UnitSystem::measure::gas_surface_rate)); + this->resv_max_rate = UDAValue(unit_system.getDimension(UnitSystem::measure::rate)); + } +} Group::GroupInjectionProperties Group::GroupInjectionProperties::serializeObject() { @@ -213,10 +234,22 @@ bool Group::GroupInjectionProperties::operator!=(const GroupInjectionProperties& return !(*this == other); } +Group::GroupProductionProperties::GroupProductionProperties() : + GroupProductionProperties(UnitSystem(UnitSystem::UnitType::UNIT_TYPE_METRIC), "") +{} + +Group::GroupProductionProperties::GroupProductionProperties(const UnitSystem& unit_system, const std::string& gname) : + name(gname), + oil_target(unit_system.getDimension(UnitSystem::measure::liquid_surface_rate)), + water_target(unit_system.getDimension(UnitSystem::measure::liquid_surface_rate)), + gas_target(unit_system.getDimension(UnitSystem::measure::gas_surface_rate)), + liquid_target(unit_system.getDimension(UnitSystem::measure::liquid_surface_rate)) +{ +} Group::GroupProductionProperties Group::GroupProductionProperties::serializeObject() { - Group::GroupProductionProperties result("Group123"); + Group::GroupProductionProperties result(UnitSystem(UnitSystem::UnitType::UNIT_TYPE_METRIC), "Group123"); result.name = "Group123"; result.gconprod_cmode = ProductionCMode::PRBL; result.active_cmode = ProductionCMode::PRBL; From 9bec2673ad4d817bd952b10e1454eda4326e32a4 Mon Sep 17 00:00:00 2001 From: Joakim Hove Date: Thu, 29 Oct 2020 15:52:26 +0100 Subject: [PATCH 2/2] Explicitly delete UDAValue::operator=() --- opm/parser/eclipse/Deck/UDAValue.hpp | 16 ++++++-- src/opm/io/eclipse/rst/well.cpp | 17 ++++---- src/opm/parser/eclipse/Deck/UDAValue.cpp | 6 +-- .../EclipseState/Schedule/KeywordHandlers.cpp | 2 +- .../EclipseState/Schedule/Well/Well.cpp | 26 ++++++------ .../Schedule/Well/WellInjectionProperties.cpp | 2 +- .../Well/WellProductionProperties.cpp | 4 +- tests/parser/UDQTests.cpp | 4 +- tests/parser/WellTests.cpp | 40 +++++++++---------- 9 files changed, 64 insertions(+), 53 deletions(-) diff --git a/opm/parser/eclipse/Deck/UDAValue.hpp b/opm/parser/eclipse/Deck/UDAValue.hpp index efe7b610a..1ce32b583 100644 --- a/opm/parser/eclipse/Deck/UDAValue.hpp +++ b/opm/parser/eclipse/Deck/UDAValue.hpp @@ -38,6 +38,19 @@ public: UDAValue(double data, const Dimension& dim); UDAValue(const std::string& data, const Dimension& dim); + /* + The assignment operators have been explicitly deleted, that is to prevent + people from adding them at a later stage. It seems very tempting/natural + to implement these assignment operators, but the problem is that the + resulting UDA object will typically have the wrong dimension member, and + subtle dimension related bugs will arise. + */ + UDAValue& operator=(double value) = delete; + UDAValue& operator=(const std::string& value) = delete; + void update(double d); + void update(const std::string& s); + void update_value(const UDAValue& other); + static UDAValue serializeObject(); /* @@ -66,9 +79,6 @@ public: bool operator==(const UDAValue& other) const; bool operator!=(const UDAValue& other) const; - UDAValue& operator=(double value); - UDAValue& operator=(const std::string& value); - void update_value(const UDAValue& other); bool is_numeric() { return numeric_value; } diff --git a/src/opm/io/eclipse/rst/well.cpp b/src/opm/io/eclipse/rst/well.cpp index ba77fc051..402bae2e2 100644 --- a/src/opm/io/eclipse/rst/well.cpp +++ b/src/opm/io/eclipse/rst/well.cpp @@ -71,13 +71,16 @@ RstWell::RstWell(const ::Opm::UnitSystem& unit_system, completion_ordering( iwel[VI::IWell::CompOrd]), pvt_table( def_pvt_table), msw_pressure_drop_model( iwel[VI::IWell::MSW_PlossMod]), - orat_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::OilRateTarget]))), - wrat_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::WatRateTarget]))), - grat_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::GasRateTarget]))), - lrat_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::LiqRateTarget]))), - resv_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::ResVRateTarget]))), - thp_target( unit_system.to_si(M::identity, swel_value(swel[VI::SWell::THPTarget]))), - bhp_target_float( unit_system.to_si(M::identity, swel[VI::SWell::BHPTarget])), + // The values orat_target -> bhp_target_flow will be used in UDA values. The + // UDA values are responsible for unit conversion and raw values are + // internalized here. + orat_target( swel_value(swel[VI::SWell::OilRateTarget])), + wrat_target( swel_value(swel[VI::SWell::WatRateTarget])), + grat_target( swel_value(swel[VI::SWell::GasRateTarget])), + lrat_target( swel_value(swel[VI::SWell::LiqRateTarget])), + resv_target( swel_value(swel[VI::SWell::ResVRateTarget])), + thp_target( swel_value(swel[VI::SWell::THPTarget])), + bhp_target_float( swel[VI::SWell::BHPTarget]), hist_lrat_target( unit_system.to_si(M::liquid_surface_rate, swel[VI::SWell::HistLiqRateTarget])), hist_grat_target( unit_system.to_si(M::gas_surface_rate, swel[VI::SWell::HistGasRateTarget])), hist_bhp_target( unit_system.to_si(M::pressure, swel[VI::SWell::HistBHPTarget])), diff --git a/src/opm/parser/eclipse/Deck/UDAValue.cpp b/src/opm/parser/eclipse/Deck/UDAValue.cpp index 961e995bb..c6cd4b754 100644 --- a/src/opm/parser/eclipse/Deck/UDAValue.cpp +++ b/src/opm/parser/eclipse/Deck/UDAValue.cpp @@ -115,16 +115,14 @@ double UDAValue::getSI() const { } -UDAValue& UDAValue::operator=(double value) { +void UDAValue::update(double value) { this->double_value = value; this->numeric_value = true; - return *this; } -UDAValue& UDAValue::operator=(const std::string& value) { +void UDAValue::update(const std::string& value) { this->string_value = value; this->numeric_value = false; - return *this; } diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp index 1c61eeacc..e3b8f6c06 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp @@ -383,7 +383,7 @@ namespace { { auto group_ptr = std::make_shared(this->getGroup(group_name, handlerContext.currentStep)); - Group::GroupProductionProperties production(group_name); + Group::GroupProductionProperties production(handlerContext.section.unitSystem(), group_name); production.gconprod_cmode = controlMode; production.active_cmode = controlMode; production.oil_target = oil_target; diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/Well/Well.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/Well/Well.cpp index df7f862b9..981fb92fb 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/Well/Well.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/Well/Well.cpp @@ -161,12 +161,12 @@ Well::Well(const RestartIO::RstWell& rst_well, auto p = std::make_shared(this->unit_system, wname); // Reverse of function ctrlMode() in AggregateWellData.cpp p->whistctl_cmode = def_whistctl_cmode; - p->BHPTarget = rst_well.bhp_target_float; - p->OilRate = rst_well.orat_target ; - p->WaterRate = rst_well.wrat_target ; - p->GasRate = rst_well.grat_target ; - p->LiquidRate = rst_well.lrat_target ; - p->ResVRate = rst_well.resv_target ; + p->BHPTarget.update(rst_well.bhp_target_float); + p->OilRate.update(rst_well.orat_target); + p->WaterRate.update(rst_well.wrat_target); + p->GasRate.update(rst_well.grat_target); + p->LiquidRate.update(rst_well.lrat_target) ; + p->ResVRate.update(rst_well.resv_target); p->VFPTableNumber = rst_well.vfp_table; p->ALQValue = rst_well.alq_value; p->predictionMode = this->prediction_mode; @@ -187,7 +187,7 @@ Well::Well(const RestartIO::RstWell& rst_well, p->addProductionControl( Well::ProducerCMode::RESV ); if (rst_well.thp_target != 0) { - p->THPTarget = rst_well.thp_target; + p->THPTarget.update(rst_well.thp_target); p->addProductionControl( Well::ProducerCMode::THP ); } @@ -270,10 +270,10 @@ Well::Well(const RestartIO::RstWell& rst_well, i->injectorType = rst_well.wtype.injector_type(); switch (i->injectorType) { case InjectorType::WATER: - i->surfaceInjectionRate = rst_well.wrat_target; + i->surfaceInjectionRate.update(rst_well.wrat_target); break; case InjectorType::GAS: - i->surfaceInjectionRate = rst_well.grat_target; + i->surfaceInjectionRate.update(rst_well.grat_target); break; default: throw std::invalid_argument("What ..."); @@ -284,17 +284,17 @@ Well::Well(const RestartIO::RstWell& rst_well, i->addInjectionControl(Well::InjectorCMode::RATE); if (std::abs(rst_well.resv_target) > 0.0f) { - i->reservoirInjectionRate = rst_well.resv_target; + i->reservoirInjectionRate.update(rst_well.resv_target); i->addInjectionControl(Well::InjectorCMode::RESV); } i->addInjectionControl(Well::InjectorCMode::BHP); - i->BHPTarget = rst_well.bhp_target_float; + i->BHPTarget.update(rst_well.bhp_target_float); if (this->isAvailableForGroupControl()) i->addInjectionControl(Well::InjectorCMode::GRUP); if (rst_well.thp_target != 0) { - i->THPTarget = rst_well.thp_target; + i->THPTarget.update(rst_well.thp_target); i->addInjectionControl(Well::InjectorCMode::THP); } @@ -463,7 +463,7 @@ bool Well::updateEconLimits(std::shared_ptr econ_limit void Well::switchToProducer() { auto p = std::make_shared(this->getInjectionProperties()); - p->BHPTarget = 0; + p->BHPTarget.update(0); p->dropInjectionControl( Opm::Well::InjectorCMode::BHP ); this->updateInjection( p ); this->wtype.update(true); diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellInjectionProperties.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellInjectionProperties.cpp index a43c7835a..39b377f38 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellInjectionProperties.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellInjectionProperties.cpp @@ -181,7 +181,7 @@ namespace Opm { if (!record.getItem("RATE").defaultApplied(0)) { double injectionRate = record.getItem("RATE").get(0); - this->surfaceInjectionRate = injectionRate; + this->surfaceInjectionRate.update(injectionRate); } if ( record.getItem( "BHP" ).hasValue(0) ) this->BHPH = record.getItem("BHP").getSIDouble(0); diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellProductionProperties.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellProductionProperties.cpp index aa00b8908..a6500e80f 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellProductionProperties.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/Well/WellProductionProperties.cpp @@ -207,8 +207,8 @@ void Well::WellProductionProperties::handleWCONHIST(const std::optionalinit_rates(record); this->init_vfp(alq_type, unit_system_arg, record); - this->LiquidRate = 0; - this->ResVRate = 0; + this->LiquidRate.update(0); + this->ResVRate.update(0); // when the well is switching to history matching producer from prediction mode // or switching from injector to producer diff --git a/tests/parser/UDQTests.cpp b/tests/parser/UDQTests.cpp index f5e62a065..783054562 100644 --- a/tests/parser/UDQTests.cpp +++ b/tests/parser/UDQTests.cpp @@ -1338,10 +1338,10 @@ BOOST_AUTO_TEST_CASE(UDA_VALUE) { BOOST_CHECK(!value0.is()); BOOST_CHECK_EQUAL( value0.get(), 0); BOOST_CHECK_THROW( value0.get(), std::invalid_argument); - value0 = 10; + value0.update(10); BOOST_CHECK_EQUAL( value0.get(), 10); BOOST_CHECK_THROW( value0.get(), std::invalid_argument); - value0 = "STRING"; + value0.update("STRING"); BOOST_CHECK_EQUAL( value0.get(), std::string("STRING")); BOOST_CHECK_THROW( value0.get(), std::invalid_argument); diff --git a/tests/parser/WellTests.cpp b/tests/parser/WellTests.cpp index a47a9ac18..5b13955fd 100644 --- a/tests/parser/WellTests.cpp +++ b/tests/parser/WellTests.cpp @@ -220,7 +220,7 @@ BOOST_AUTO_TEST_CASE(isProducerCorrectlySet) { /* Set a surface injection rate => Well becomes an Injector */ auto injectionProps1 = std::make_shared(well.getInjectionProperties()); - injectionProps1->surfaceInjectionRate = 100; + injectionProps1->surfaceInjectionRate.update(100); well.updateInjection(injectionProps1); BOOST_CHECK_EQUAL( true , well.isInjector()); BOOST_CHECK_EQUAL( false , well.isProducer()); @@ -233,7 +233,7 @@ BOOST_AUTO_TEST_CASE(isProducerCorrectlySet) { /* Set a reservoir injection rate => Well becomes an Injector */ auto injectionProps2 = std::make_shared(well.getInjectionProperties()); - injectionProps2->reservoirInjectionRate = 200; + injectionProps2->reservoirInjectionRate.update(200); well.updateInjection(injectionProps2); BOOST_CHECK_EQUAL( false , well.isProducer()); BOOST_CHECK_EQUAL( 200 , well.getInjectionProperties().reservoirInjectionRate.get()); @@ -247,9 +247,9 @@ BOOST_AUTO_TEST_CASE(isProducerCorrectlySet) { well.updateInjection(injectionProps3); auto properties = std::make_shared( well.getProductionProperties() ); - properties->OilRate = 100; - properties->GasRate = 200; - properties->WaterRate = 300; + properties->OilRate.update(100); + properties->GasRate.update(200); + properties->WaterRate.update(300); well.updateProduction(properties); BOOST_CHECK_EQUAL( false , well.isInjector()); @@ -286,14 +286,14 @@ BOOST_AUTO_TEST_CASE(XHPLimitDefault) { auto productionProps = std::make_shared(well.getProductionProperties()); - productionProps->BHPTarget = 100; + productionProps->BHPTarget.update(100); productionProps->addProductionControl(Opm::Well::ProducerCMode::BHP); well.updateProduction(productionProps); BOOST_CHECK_EQUAL( 100 , well.getProductionProperties().BHPTarget.get()); BOOST_CHECK_EQUAL( true, well.getProductionProperties().hasProductionControl( Opm::Well::ProducerCMode::BHP )); auto injProps = std::make_shared(well.getInjectionProperties()); - injProps->THPTarget = 200; + injProps->THPTarget.update(200); well.updateInjection(injProps); BOOST_CHECK_EQUAL( 200 , well.getInjectionProperties().THPTarget.get()); BOOST_CHECK( !well.getInjectionProperties().hasInjectionControl( Opm::Well::InjectorCMode::THP )); @@ -327,26 +327,26 @@ BOOST_AUTO_TEST_CASE(WellHaveProductionControlLimit) { BOOST_CHECK( !well.getProductionProperties().hasProductionControl( Opm::Well::ProducerCMode::RESV )); auto properties1 = std::make_shared(well.getProductionProperties()); - properties1->OilRate = 100; + properties1->OilRate.update(100); properties1->addProductionControl(Opm::Well::ProducerCMode::ORAT); well.updateProduction(properties1); BOOST_CHECK( well.getProductionProperties().hasProductionControl( Opm::Well::ProducerCMode::ORAT )); BOOST_CHECK( !well.getProductionProperties().hasProductionControl( Opm::Well::ProducerCMode::RESV )); auto properties2 = std::make_shared(well.getProductionProperties()); - properties2->ResVRate = 100; + properties2->ResVRate.update(100); properties2->addProductionControl(Opm::Well::ProducerCMode::RESV); well.updateProduction(properties2); BOOST_CHECK( well.getProductionProperties().hasProductionControl( Opm::Well::ProducerCMode::RESV )); auto properties3 = std::make_shared(well.getProductionProperties()); - properties3->OilRate = 100; - properties3->WaterRate = 100; - properties3->GasRate = 100; - properties3->LiquidRate = 100; - properties3->ResVRate = 100; - properties3->BHPTarget = 100; - properties3->THPTarget = 100; + properties3->OilRate.update(100); + properties3->WaterRate.update(100); + properties3->GasRate.update(100); + properties3->LiquidRate.update(100); + properties3->ResVRate.update(100); + properties3->BHPTarget.update(100); + properties3->THPTarget.update(100); properties3->addProductionControl(Opm::Well::ProducerCMode::ORAT); properties3->addProductionControl(Opm::Well::ProducerCMode::LRAT); properties3->addProductionControl(Opm::Well::ProducerCMode::BHP); @@ -373,22 +373,22 @@ BOOST_AUTO_TEST_CASE(WellHaveInjectionControlLimit) { BOOST_CHECK( !well.getInjectionProperties().hasInjectionControl( Opm::Well::InjectorCMode::RESV )); auto injProps1 = std::make_shared(well.getInjectionProperties()); - injProps1->surfaceInjectionRate = 100; + injProps1->surfaceInjectionRate.update(100); injProps1->addInjectionControl(Opm::Well::InjectorCMode::RATE); well.updateInjection(injProps1); BOOST_CHECK( well.getInjectionProperties().hasInjectionControl( Opm::Well::InjectorCMode::RATE )); BOOST_CHECK( !well.getInjectionProperties().hasInjectionControl( Opm::Well::InjectorCMode::RESV )); auto injProps2 = std::make_shared(well.getInjectionProperties()); - injProps2->reservoirInjectionRate = 100; + injProps2->reservoirInjectionRate.update(100); injProps2->addInjectionControl(Opm::Well::InjectorCMode::RESV); well.updateInjection(injProps2); BOOST_CHECK( well.getInjectionProperties().hasInjectionControl( Opm::Well::InjectorCMode::RESV )); auto injProps3 = std::make_shared(well.getInjectionProperties()); - injProps3->BHPTarget = 100; + injProps3->BHPTarget.update(100); injProps3->addInjectionControl(Opm::Well::InjectorCMode::BHP); - injProps3->THPTarget = 100; + injProps3->THPTarget.update(100); injProps3->addInjectionControl(Opm::Well::InjectorCMode::THP); well.updateInjection(injProps3);