Add satellite production/injection update API for reservoir coupling#5101
Add satellite production/injection update API for reservoir coupling#5101bska merged 3 commits intoOPM:masterfrom
Conversation
Add Schedule::updateSatelliteProduction() and Schedule::updateSatelliteInjection() for reservoir coupling. These methods allow opm-simulators to inject slave production and injection rates into the Schedule's GSatProd and GroupSatelliteInjection structures at runtime. The existing satellite_rate machinery in Summary.cpp then automatically computes all rate-based summary vectors (FOPR, GOPR, FGOR, FWCT, etc.) correctly for the master model. The methods deliberately skip recordSatelliteProduction() / recordSatelliteInjection() to avoid restart side effects. Restart support for reservoir coupling is planned separately.
|
jenkins build this opm-simulators=6979 serial please |
2 similar comments
|
jenkins build this opm-simulators=6979 serial please |
|
jenkins build this opm-simulators=6979 serial please |
|
Thanks. For me, this looks good. Smart to add it to the satellite production/injection. What do you think @bska is this an okay approach? |
bska
left a comment
There was a problem hiding this comment.
I see the convenience of this approach, but I don't think it's the right solution. The Schedule object is supposed to be unchanging, with the possible exception of action events, so I would prefer not adding back channels for reporting to it.
The right way to add reporting of dynamically calculated rates is to amend the existing Summary protocols.
On a tangential note, UDAValue objects are supposed to store numerical values in output units.
| // GSatProd::assign() stores rates as UDAValue. Reservoir coupling slave | ||
| // rates are already in SI, which is what GSatProd expects (the keyword | ||
| // handler converts from deck units to SI, but here we supply SI directly). |
There was a problem hiding this comment.
This kind of works by accident, but the overall system makes the general assumption that anything in a UDAValue is stored in output units and with an appropriate Dimension for the type. If we don't honour that assumption, then this will come back to haunt us later.
Replace Schedule mutation (GSatProd/GroupSatelliteInjection) with a proper Summary data flow for reservoir coupling master group rates. Add ReservoirCouplingGroupRates to DynamicSimulatorState so that Summary::eval() receives slave production/injection rates through the standard evaluation pipeline. The satellite_rate function in Summary.cpp checks these rates alongside the existing GSatProd/GSATINJE sources, reusing the accum_groups tree-walking logic for field-level and group-level vectors. Remove Schedule::updateSatelliteProduction/Injection() which mutated the Schedule object at runtime. This addresses the review concern that Schedule should be immutable outside of action events.
|
jenkins build this opm-simulators=6979 serial please |
@bska Thanks for the suggestion. I have updated the PR with a new commit to use the existing protocols. Let me know if I should squash the two commits into one commit before merging. |
bska
left a comment
There was a problem hiding this comment.
Thanks a lot for the updates, this is a much better approach in my opinion. I would have liked to see some unit tests that demonstrates the facility in practice and to aid future maintenance. Would you be able to add such tests, e.g., in tests/test_Summary.cpp?
Other than that general comment, I just minor remarks.
| /// Per master-group production rates (positive values, SI units). | ||
| std::map<std::string, ProductionRates> production; | ||
| /// Per master-group, per-phase injection rates (SI units). | ||
| std::map<std::string, std::map<Opm::Phase, InjectionRates>> injection; |
There was a problem hiding this comment.
You need to #include <opm/input/eclipse/EclipseState/Phase.hpp> to bring a definition of Opm::Phase into scope. Otherwise we're depending on transitive includes which may change at any time..
| if (!rc_rates.hasProduction(group)) { | ||
| return 0.0; | ||
| } | ||
| const auto& prod = rc_rates.production.at(group); |
There was a problem hiding this comment.
Since you need the element anyway, this is better written in terms of map<>::find():
const auto groupPos = rc_rates.production.find(group);
if (groupPos == rc_rates.production.end()) {
return 0.0;
}
if constexpr (phase == rt::oil) { return groupPos->oil; }
// ...
return 0.0;| bool hasProduction(const std::string& group) const { | ||
| return production.count(group) > 0; | ||
| } | ||
| bool hasInjection(const std::string& group, Opm::Phase phase) const { | ||
| auto it = injection.find(group); | ||
| return it != injection.end() && it->second.count(phase) > 0; | ||
| } |
There was a problem hiding this comment.
Please consider if you actually need these. From the implementation here, these look extraneous.
Add unit test in test_Summary.cpp verifying that reservoir coupling group rates flow through DynamicSimulatorState to Summary::eval() and produce correct FOPR/GOPR values. Also address minor review comments: - Add explicit Phase.hpp include in Groups.hpp - Use map::find() instead of double lookup in rc_group_prod() - Remove unused hasProduction()/hasInjection() helpers
|
jenkins build this opm-simulators=6979 serial please |
bska
left a comment
There was a problem hiding this comment.
Thanks a lot for the updates and the quick response. This looks good to me now and I'll merge into master.
Schedule::updateSatelliteProduction()— updatesGSatProdfor a master group at a given report step with slave production rates (oil, gas, water, reservoir voidage)Schedule::updateSatelliteInjection()— updatesGroupSatelliteInjectionfor a master group with slave injection rates (surface and reservoir, per phase)recordSatelliteProduction()/recordSatelliteInjection()to avoid restart side effects; restart support for reservoir coupling is planned separatelyBackground
In reservoir coupling, the master model has no wells — all production and injection happens in slave models. The master's summary vectors (FOPR, GOPR, FGOR, FWCT, etc.) were previously zero because
Summary.cppcomputes them by summing well rates.opm-common already has a satellite rate mechanism (
GSatProd/GroupSatelliteInjection) designed for theGSATPROD/GSATINJEkeywords. Therate<>function inSummary.cppcallssatellite_rate<>which reads from these structures and accumulates rates through the group tree, for both group-level and field-level vectors. This works becauseargs.group_nameis"FIELD"for field vectors, sosatellite_rateis called and descends through the group tree viaaccum_groups().These new methods allow opm-simulators to populate the satellite structures at runtime with slave rates received via MPI, so the existing summary machinery produces correct output without any changes to
Summary.cpp.Companion PR
OPM/opm-simulators#6979 : calls these methods from
EclWriter::evalSummaryState()viaReservoirCouplingMasterReportStep::updateScheduleSatelliteData()