Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

changed: store a pointer to the static unit system instead of a copy per well #4232

Merged
merged 1 commit into from
Oct 1, 2024
Merged

changed: store a pointer to the static unit system instead of a copy per well #4232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions opm/input/eclipse/Schedule/Schedule.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <opm/input/eclipse/Schedule/ScheduleState.hpp>
#include <opm/input/eclipse/Schedule/ScheduleStatic.hpp>
#include <opm/input/eclipse/Schedule/Well/PAvg.hpp>
#include <opm/input/eclipse/Schedule/Well/Well.hpp>
#include <opm/input/eclipse/Schedule/WriteRestartFileEvents.hpp>
#include <opm/input/eclipse/Units/UnitSystem.hpp>

Expand Down Expand Up @@ -335,6 +336,20 @@ namespace Opm
this->template pack_unpack_map<int, VFPInjTable>(serializer);
this->template pack_unpack_map<std::string, Group>(serializer);
this->template pack_unpack_map<std::string, Well>(serializer);

// If we are deserializing we need to setup the pointer to the
// unit system since this is process specific. This is safe
// because we set the same value in all well instances.
// We do some redundant assignments as these are shared_ptr's
// with multiple pointers to any given instance, but it is not
// significant so let's keep it simple.
if (!serializer.isSerializing()) {
for (auto& snapshot : snapshots) {
for (auto& well : snapshot.wells) {
well.second->updateUnitSystem(&m_static.m_unit_system);
}
}
Comment on lines +347 to +351
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably know this already, but in the interest of completeness I'll just remark that this is safe in this particular context since every call to Well::updateUnitSystem() passes the same pointer value. In general, however, this kind of loop structure is potentially harmful. Since the snapshots store shared_ptr<> objects, we risk making inconsistent changes to both earlier and later Well objects if we're not careful and that's why most update() functions return a bool to let the caller know whether or not any actual changes were made.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am aware. As it is the same pointer being put into every well this is non-problematic and I need to do this after deserialization since the pointer obviously is process specific.

}
}

template <typename T, class Serializer>
Expand Down
32 changes: 18 additions & 14 deletions opm/input/eclipse/Schedule/Well/Well.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ Well::Well(const RestartIO::RstWell& rst_well,
allow_cross_flow(rst_well.allow_xflow == 1),
automatic_shutin(def_automatic_shutin),
pvt_table(rst_well.pvt_table),
unit_system(unit_system_arg),
unit_system(&unit_system_arg),
udq_undefined(udq_undefined_arg),
wtype(rst_well.wtype),
guide_rate(guideRate(rst_well)),
Expand All @@ -319,7 +319,7 @@ Well::Well(const RestartIO::RstWell& rst_well,
well_inj_mult(std::nullopt)
{
if (this->wtype.producer()) {
auto p = std::make_shared<WellProductionProperties>(this->unit_system, wname);
auto p = std::make_shared<WellProductionProperties>(*this->unit_system, wname);

p->whistctl_cmode = (rst_whistctl_cmode > 0)
? producer_cmode_from_int(rst_whistctl_cmode)
Expand Down Expand Up @@ -364,7 +364,7 @@ Well::Well(const RestartIO::RstWell& rst_well,
p->addProductionControl(Well::ProducerCMode::BHP);
if (! p->predictionMode) {
p->BHPTarget.update(0.0);
p->setBHPLimit(unit_system.to_si(UnitSystem::measure::pressure, rst_well.bhp_target_float));
p->setBHPLimit(unit_system->to_si(UnitSystem::measure::pressure, rst_well.bhp_target_float));
p->controlMode = producer_cmode_from_int(rst_well.hist_requested_control);
}
else if (this->isAvailableForGroupControl())
Expand All @@ -373,7 +373,7 @@ Well::Well(const RestartIO::RstWell& rst_well,
this->updateProduction(std::move(p));
}
else {
auto i = std::make_shared<WellInjectionProperties>(this->unit_system, wname);
auto i = std::make_shared<WellInjectionProperties>(*this->unit_system, wname);
i->VFPTableNumber = rst_well.vfp_table;
i->predictionMode = this->prediction_mode;

Expand Down Expand Up @@ -482,7 +482,7 @@ Well::Well(const std::string& wname_arg,
automatic_shutin(auto_shutin),
pvt_table(pvt_table_),
gas_inflow(inflow_eq),
unit_system(unit_system_arg),
unit_system(&unit_system_arg),
udq_undefined(udq_undefined_arg),
wtype(wtype_arg),
guide_rate({true, -1, Well::GuideRateTarget::UNDEFINED,ParserKeywords::WGRUPCON::SCALING_FACTOR::defaultValue}),
Expand All @@ -496,16 +496,16 @@ Well::Well(const std::string& wname_arg,
brine_properties(std::make_shared<WellBrineProperties>()),
tracer_properties(std::make_shared<WellTracerProperties>()),
connections(std::make_shared<WellConnections>(ordering_arg, headI, headJ)),
production(std::make_shared<WellProductionProperties>(unit_system, wname)),
injection(std::make_shared<WellInjectionProperties>(unit_system, wname)),
production(std::make_shared<WellProductionProperties>(*unit_system, wname)),
injection(std::make_shared<WellInjectionProperties>(*unit_system, wname)),
wvfpdp(std::make_shared<WVFPDP>()),
wvfpexp(std::make_shared<WVFPEXP>()),
wdfac(std::make_shared<WDFAC>()),
status(Status::SHUT),
well_inj_temperature(std::nullopt),
well_inj_mult(std::nullopt)
{
auto p = std::make_shared<WellProductionProperties>(this->unit_system, this->wname);
auto p = std::make_shared<WellProductionProperties>(*this->unit_system, this->wname);
p->whistctl_cmode = whistctl_cmode;
this->updateProduction(p);
}
Expand All @@ -520,7 +520,6 @@ Well Well::serializationTestObject()
result.headI = 3;
result.headJ = 4;
result.ref_depth = 5;
result.unit_system = UnitSystem::serializationTestObject();
result.udq_undefined = 6.0;
result.status = Status::AUTO;
result.drainage_radius = 7.0;
Expand Down Expand Up @@ -1131,11 +1130,11 @@ double Well::convertDeckPI(double deckPI) const {
// not provide that enumerator.
switch (this->getPreferredPhase()) {
case Phase::GAS:
return this->unit_system.to_si(M::gas_productivity_index, deckPI);
return this->unit_system->to_si(M::gas_productivity_index, deckPI);

case Phase::OIL:
case Phase::WATER:
return this->unit_system.to_si(M::liquid_productivity_index, deckPI);
return this->unit_system->to_si(M::liquid_productivity_index, deckPI);

default:
throw std::invalid_argument {
Expand Down Expand Up @@ -1703,7 +1702,7 @@ Well::ProductionControls Well::productionControls(const SummaryState& st) const

Well::InjectionControls Well::injectionControls(const SummaryState& st) const {
if (!this->isProducer()) {
auto controls = this->injection->controls(this->unit_system, st, this->udq_undefined);
auto controls = this->injection->controls(*this->unit_system, st, this->udq_undefined);
return controls;
} else
throw std::logic_error("Trying to get injection data from a producer");
Expand Down Expand Up @@ -1750,7 +1749,9 @@ void Well::setWellInjTemperature(const double temp) {

bool Well::cmp_structure(const Well& other) const {
if ((this->segments && !other.segments) ||
(!this->segments && other.segments))
(!this->segments && other.segments) ||
(this->unit_system && !other.unit_system) ||
(!this->unit_system && other.unit_system))
{
return false;
}
Expand All @@ -1759,6 +1760,10 @@ bool Well::cmp_structure(const Well& other) const {
return false;
}

if (this->unit_system && *this->unit_system != *other.unit_system) {
return false;
}

return (this->name() == other.name())
&& (this->groupName() == other.groupName())
&& (this->firstTimeStep() == other.firstTimeStep())
Expand All @@ -1768,7 +1773,6 @@ bool Well::cmp_structure(const Well& other) const {
&& (this->hasRefDepth() == other.hasRefDepth())
&& (!this->hasRefDepth() || (this->getRefDepth() == other.getRefDepth()))
&& (this->getPreferredPhase() == other.getPreferredPhase())
&& (this->unit_system == other.unit_system)
&& (this->udq_undefined == other.udq_undefined)
&& (this->getConnections() == other.getConnections())
&& (this->getDrainageRadius() == other.getDrainageRadius())
Expand Down
6 changes: 4 additions & 2 deletions opm/input/eclipse/Schedule/Well/Well.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ class Well {
std::vector<bool>& scalingApplicable);
const PAvg& pavg() const;

//! \brief Used by schedule deserialization.
void updateUnitSystem(const UnitSystem* usys) { unit_system = usys; }

template<class Serializer>
void serializeOp(Serializer& serializer)
{
Expand All @@ -585,7 +588,6 @@ class Well {
serializer(headJ);
serializer(ref_depth);
serializer(wpave_ref_depth);
serializer(unit_system);
serializer(udq_undefined);
serializer(status);
serializer(drainage_radius);
Expand Down Expand Up @@ -640,7 +642,7 @@ class Well {
bool automatic_shutin{false};
int pvt_table{};
GasInflowEquation gas_inflow = GasInflowEquation::STD; // Will NOT be loaded/assigned from restart file
UnitSystem unit_system;
const UnitSystem* unit_system{nullptr};
double udq_undefined{};
WellType wtype{};
WellGuideRate guide_rate{};
Expand Down