Store instantaneous instead of effective tracer rates

[vkip]

This fixes a bug where well efficiency factors are applied twice in summary output.

[Copilot]

Pull request overview

This PR addresses a reporting bug where well efficiency factors (WEFAC and global efficiency scaling) end up being applied twice to tracer rates in summary output, by storing tracer rates in a way that avoids double-application downstream.

Changes:

• Added a const overload of getGenWell() to allow querying well data from const contexts.
• Adjusted well and MSW tracer summary rate assignment to undo the applied well efficiency factor before storing into data::Wells.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
opm/simulators/wells/BlackoilWellModelGeneric.hpp	Adds const overload for getGenWell() to support const reporting paths.
opm/simulators/wells/BlackoilWellModelGeneric.cpp	Implements const/non-const getGenWell() overloads and scales tracer rates by inverse well efficiency factor during reporting.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[vkip]

jenkins build this please

[vkip]

jenkins build this please

[vkip]

jenkins build this please

[vkip]

jenkins build this please

[bska]

What about group-level efficiency factors, do we need to account for those?

To be honest, this looks a bit like a workaround and I'd prefer if there were a way for the TracerModel to report instantaneous rates directly in BlackoilWellModel<>::assignWellTracerRates() instead of trying to undo parts of its internal implementation here.

[vkip]

What about group-level efficiency factors, do we need to account for those?

These will be correctly accounted for by the mechanicsms in Summary.cpp as long as the raw well rates are reported.

To be honest, this looks a bit like a workaround and I'd prefer if there were a way for the TracerModel to report instantaneous rates directly in BlackoilWellModel<>::assignWellTracerRates() instead of trying to undo parts of its internal implementation here.

I see you your point, this just seemed to be the least intrusive approach. I'll have a second look.

[vkip]

Second attempt, similar hack (and same results), but hopefully makes more sense to do it in TracerModel (?)

[bska]

jenkins build this please

[bska]

Second attempt, similar hack (and same results), but hopefully makes more sense to do it in TracerModel (?)

I agree that it makes sense to do this work closer to the source. In an ideal world the TracerModel would just use raw flow rates, unencumbered by well level efficiency factors, internally and we wouldn't need this at all, so maybe that suggests that there's a deeper problem elsewhere. I'll ask @totto82 to have a look at this from that point of view.

If no other fix will be forthcoming on a short-term basis then I'll nevertheless get this into the master branch.

[vkip]

I agree that it makes sense to do this work closer to the source. In an ideal world the TracerModel would just use raw flow rates, unencumbered by well level efficiency factors, internally and we wouldn't need this at all, so maybe that suggests that there's a deeper problem elsewhere. I'll ask @totto82 to have a look at this from that point of view.

If no other fix will be forthcoming on a short-term basis then I'll nevertheless get this into the master branch.

Thanks. One way or another the efficiencies need to be used in TracerModel since they ultimately impact how much tracer mass enters or leaves the reservoir, so for me it is not obvious that one approach is better than the other, but I'll leave further discussion to you guys. I'll be happy to close this PR if an alternative solution is provided.

[bska]

One way or another the efficiencies need to be used in TracerModel since they ultimately impact how much tracer mass enters or leaves the reservoir, so for me it is not obvious that one approach is better than the other, but I'll leave further discussion to you guys.

We'll figure it out.

That said, if the TracerModel needs to include efficiency factors to compute cumulatives like volumes or masses, then it should probably also care about group level efficiency factors (GEFAC) too. I don't know enough about the internal details of the solver to make a definitive statement, but I would have thought that it could compute a sort of "aggregate"/"effective" factor per well that includes the effect of both well level and group level efficiency factors and apply that to the raw rates when computing volumes and masses. Then the rates themselves could be left untouched.

[vkip]

That said, if the TracerModel needs to include efficiency factors to compute cumulatives like volumes or masses, then it should probably also care about group level efficiency factors (GEFAC) too.

Maybe I'm mistaken, but I believe this is properly taken care of (though naming could be discussed):

opm-simulators/opm/simulators/wells/BlackoilWellModelGeneric.cpp

Lines 1508 to 1522 in 13ed7bd

	template<typename Scalar, typename IndexTraits>
	void BlackoilWellModelGeneric<Scalar, IndexTraits>::
	calculateEfficiencyFactors(const int reportStepIdx)
	{
	for (auto& well : well_container_generic_) {
	const Well& wellEcl = well->wellEcl();
	Scalar well_efficiency_factor = wellEcl.getEfficiencyFactor() *
	wellState().getGlobalEfficiencyScalingFactor(well->name());
	this->groupStateHelper().accumulateGroupEfficiencyFactor(
	schedule().getGroup(wellEcl.groupName(), reportStepIdx),
	well_efficiency_factor
	);
	well->setWellEfficiencyFactor(well_efficiency_factor);
	}
	}