m_efficiency_scaling_factors default to be 1. instead of 0. #6608

GitPaean · 2025-11-13T13:34:12Z

as an efficiency factor, it is more reasonable to default it be 1. otherwise, it can be 0 when WCYCLE is not around.

GitPaean · 2025-11-13T13:35:18Z

jenkins build this failure_report please

GitPaean · 2025-11-13T14:10:06Z

jenkins build this failure_report please

GitPaean · 2025-11-13T15:09:36Z

As far I can say, the only place that can modify this value is related to the WCYCLE.

 964             if (!wcycle.empty()) {
 965                 const auto schedule_open =
 966                     [&wg_events = this->report_step_start_events_](const std::string& name)
 967                     {
 968                         return wg_events.hasEvent(name, ScheduleEvents::REQUEST_OPEN_WELL);
 969                     };
 970                 for (const auto& [wname, wscale] : wcycle.efficiencyScale(this->simulator_.time(),
 971                                                                           this->simulator_.timeStepSize(),
 972                                                                           wmatcher,
 973                                                                           this->well_open_times_,
 974                                                                           schedule_open))
 975                 {
 976                     this->wellState().updateEfficiencyScalingFactor(wname, wscale);
 977                     this->schedule_.add_event(ScheduleEvents::WELLGROUP_EFFICIENCY_UPDATE, report_step);
 978                 }

I think it is okay to let it stay 1.0 when WCYCLE is not there. while I might miss something.

GitPaean · 2025-11-13T15:27:40Z

I could reproduce the failure from jenkins

https://ci.opm-project.org/job/opm-simulators-PR-builder/9024/artifact/mpi/build-opm-simulators/failure_report/01_max_watercut_4.pdf

locally the well curves are visually identical (different residual output), both are with 419 time steps.

On jenkins, the reference results has 416 time steps and 415 time steps with this PR. (or the other way around)

But the new results is smoother, which looks better.

steink · 2025-11-13T15:35:27Z

As far I can say, the only place that can modify this value is related to the WCYCLE.

I think also here:

opm-simulators/opm/simulators/wells/WellState.cpp

Lines 1027 to 1043 in 0a7715a

    
           template<typename Scalar, typename IndexTraits> 
        
           void WellState<Scalar, IndexTraits>::updateGlobalIsGrup(const Parallel::Communication& comm,  const std::vector<WellStatus>& well_status) 
        
           { 
        
               this->global_well_info.value().clear(); 
        
               for (std::size_t well_index = 0; well_index < this->size(); well_index++) { 
        
                   const auto& ws = this->well(well_index); 
        
                   // We cannot use the well status directly from the well state here as well, may 
        
                   // be temporarily stopped due to convergence or operability issues 
        
                   const auto& this_well_status = well_status[well_index]; 
        
                   this->global_well_info.value().update_efficiency_scaling_factor(well_index, ws.efficiency_scaling_factor); 
        
                   if (ws.producer) 
        
                       this->global_well_info.value().update_producer(well_index, this_well_status, ws.production_cmode); 
        
                   else 
        
                       this->global_well_info.value().update_injector(well_index, this_well_status, ws.injection_cmode); 
        
               } 
        
               this->global_well_info.value().communicate(comm); 
        
           }

In any case, the well testing appears before these functions are called, so I think the total efficiency factor always ends up to be zero when wells are tested at the beginning of a report step. Seems that this situation appear twice in the failed test, allthough not sure why it would trigger failure since it's standard wells (for MS wells zero efficiency factor results in singular matrix)

vkip

Looks very reasonable to me.

GitPaean · 2025-11-13T23:32:55Z

As far I can say, the only place that can modify this value is related to the WCYCLE.

I think also here:

opm-simulators/opm/simulators/wells/WellState.cpp

Lines 1027 to 1043 in 0a7715a

template<typename Scalar, typename IndexTraits>

void WellState<Scalar, IndexTraits>::updateGlobalIsGrup(const Parallel::Communication& comm, const std::vector<WellStatus>& well_status)

{

this->global_well_info.value().clear();

for (std::size_t well_index = 0; well_index < this->size(); well_index++) {

const auto& ws = this->well(well_index);

// We cannot use the well status directly from the well state here as well, may

// be temporarily stopped due to convergence or operability issues

const auto& this_well_status = well_status[well_index];

this->global_well_info.value().update_efficiency_scaling_factor(well_index, ws.efficiency_scaling_factor);

if (ws.producer)

this->global_well_info.value().update_producer(well_index, this_well_status, ws.production_cmode);

else

this->global_well_info.value().update_injector(well_index, this_well_status, ws.injection_cmode);

}

this->global_well_info.value().communicate(comm);

}

Possibly we should call this function or updateAndCommunicateGroupData() before well testing?

Seems that this situation appear twice in the failed test, allthough not sure why it would trigger failure since it's standard wells (for MS wells zero efficiency factor results in singular matrix)

Maybe it also effects other places not only well testing. while it needs some investigation to figure out.

As said above. we might need to do more to have a more complete setup. For example, WCYCLE + WTEST scenario, while initializing the m_efficiency_scaling_factors to be 1.0 is probably better than 0.

bska · 2025-11-14T08:12:04Z

opm/simulators/wells/GlobalWellInfo.hpp

    std::vector<int> m_in_producing_group;       // global_index -> int/bool
    std::vector<int> m_is_open;                  // global_index -> int/bool
-    std::vector<Scalar> m_efficiency_scaling_factors; // global_index --> double
+    std::vector<Scalar> m_efficiency_scaling_factors {1.}; // global_index --> double


This initialises m_efficiency_scaling_factors to a single-element vector<> holding the value 1. Are we sure that that's appropriate in all cases?

This can't possibly be the correct fix. We are supposed to hold one entry per well. If anything derefs this vector out-of-bounds there is a logic error elsewhere, since this is resized in the ctor. Maybe you meant

this->m_efficiency_scaling_factors.resize(num_wells, 1.0);

in the ctor? That is more in line with what you say you want to achieve in the pr description.

yeah, you guys are totally right. Not working today and will also check a few things around this. Will update.

vkip

Having a default of 1 seemed reasonable, but obviously I did not think this through. I'll stay out of this...

GitPaean · 2025-11-14T15:38:02Z

Having a default of 1 seemed reasonable, but obviously I did not think this through. I'll stay out of this...

Sorry for proposing the PR this way. I did not think it clearly at the end of the day.

GitPaean · 2025-11-14T15:41:28Z

In any case, the well testing appears before these functions are called, so I think the total efficiency factor always ends up to be zero when wells are tested at the beginning of a report step. Seems that this situation appear twice in the failed test, allthough not sure why it would trigger failure since it's standard wells (for MS wells zero efficiency factor results in singular matrix)

It looks like for this specific case, with 0 efficiency scaling factor, the well get OP_1 get singular matrix. Did not dig through the exact cause yet.

bska · 2025-11-14T15:42:23Z

Having a default of 1 seemed reasonable, but obviously I did not think this through. I'll stay out of this...

Sorry for proposing the PR this way. I did not think it clearly at the end of the day.

Don't worry about it. This is why we have PR reviews (and unit tests).

GitPaean · 2025-11-14T23:19:40Z

Seems that this situation appear twice in the failed test, allthough not sure why it would trigger failure since it's standard wells

For standard well, well efficiency factor is used in the following two places (third one in energy connection rate)

                const EvalWell cq_s_effective = cq_s[componentIdx] * this->well_efficiency_factor_;

            resWell_loc -= this->primary_variables_.getQs(componentIdx) * this->well_efficiency_factor_;

If well_efficiency_factor_ == 0., then the mass balance equation becomes

                resWell_loc += (this->primary_variables_.surfaceVolumeFraction(componentIdx) -
                                this->F0_[componentIdx]) * volume / dt

If it is rate controlled, it might be singular since there is no derivative against BHP at all.

GitPaean · 2025-11-15T19:51:01Z

jenkins build this failure_report please

GitPaean · 2025-11-15T21:27:50Z

Keeping a record for previous testing results.

https://ci.opm-project.org/job/opm-simulators-PR-builder/9030/#showFailuresLink

GitPaean · 2025-11-15T21:27:59Z

jenkins build this failure_report please

GitPaean · 2025-11-17T09:35:20Z

We can also get the efficiency factor from the SingleWellState directly.

updateGlobalIsGrup() is basically copying the one in SingleWellState to the global_well_info.

GitPaean · 2025-11-17T09:35:52Z

jenkins build this failure_report please

GitPaean · 2025-11-17T11:00:35Z

Seems that this situation appear twice in the failed test, allthough not sure why it would trigger failure since it's standard wells

For standard well, well efficiency factor is used in the following two places (third one in energy connection rate)
                const EvalWell cq_s_effective = cq_s[componentIdx] * this->well_efficiency_factor_;
            resWell_loc -= this->primary_variables_.getQs(componentIdx) * this->well_efficiency_factor_;
If well_efficiency_factor_ == 0., then the mass balance equation becomes
                resWell_loc += (this->primary_variables_.surfaceVolumeFraction(componentIdx) -
                                this->F0_[componentIdx]) * volume / dt
If it is rate controlled, it might be singular since there is no derivative against BHP at all.

With more digging in,
at least for StandardWell, if the well equation is singular, the invert function will just return a identity matrix. did not have the capacity to figure out how the well testing process continues with this identity matrix.

template<typename Scalar, typename IndexTraits, int numEq>
void StandardWellEquations<Scalar, IndexTraits, numEq>::invert()
{
    try {
        invDuneD_ = duneD_; // Not strictly need if not cpr with well contributions is used
        detail::invertMatrix(invDuneD_[0][0]);
    } catch (NumericalProblem&) {
        // for singular matrices, use identity as the inverse
        invDuneD_[0][0] = 0.0;
        for (std::size_t i = 0; i < invDuneD_[0][0].rows(); ++i) {
            invDuneD_[0][0][i][i] = 1.0;
        }
    }
}

GitPaean · 2025-11-17T11:31:29Z

With more digging in, at least for StandardWell, if the well equation is singular, the invert function will just return a identity matrix. did not have the capacity to figure out how the well testing process continues with this identity matrix.

template<typename Scalar, typename IndexTraits, int numEq>
void StandardWellEquations<Scalar, IndexTraits, numEq>::invert()
{
    try {
        invDuneD_ = duneD_; // Not strictly need if not cpr with well contributions is used
        detail::invertMatrix(invDuneD_[0][0]);
    } catch (NumericalProblem&) {
        // for singular matrices, use identity as the inverse
        invDuneD_[0][0] = 0.0;
        for (std::size_t i = 0; i < invDuneD_[0][0].rows(); ++i) {
            invDuneD_[0][0][i][i] = 1.0;
        }
    }
}

For the case

[mpi.compareECLFiles_flow+ACTIONX_UDQ]

during the well testing, it is under BHP control, singluar matrix is assembled, while the function with the above identity inverse matrix, the function

converged = solveWellWithOperabilityCheck(

managed to get converged after some iterations. I think this is simply wrong.

GitPaean · 2025-11-17T12:18:52Z

With #6614 , we can see in several scenarios, well testing will not be able to succeed if we do not use the identity matrix.

https://ci.opm-project.org/job/opm-simulators-PR-builder/9035/

GitPaean · 2025-11-18T11:16:26Z

the efficiency_scaling_factor is updated in createWellContainers(), which is called after wellTesting().

It is not totally clear how we copy/convey efficiency_scaling_factor between time steps, which can be some following up development. #6616

The main purpose of the PR is to avoid zero well_efficiency_factor during the wellTesting, then the PR should serve its purpose. At the same time, with the current version of wellTesting, the efficiency_scaling_factor might not matter much, while a zero-value efficiency_scaling_factor should NOT be used.

As said above, there might be more following up developments in both how we convey efficiency_scaling_factor between time steps and possibly we want to understand how the efficiency_scaling_factor can affect the wellTesting. That can be considered for future.

I will mark the PR as ready for review again for review and discussion.

@bska @akva2 @steink

opm/simulators/wells/BlackoilWellModel_impl.hpp

opm/simulators/wells/GlobalWellInfo.cpp

GitPaean · 2025-11-18T11:19:42Z

jenkins build this failure_report please

bska

I'll let others comment on the relationship between WellState::getGlobalEfficiencyScalingFactor(name) and SingleWellState::efficiency_scaling_factor. From my point of view, I only have one tiny remark concerning the details of the floating point types involved here.

opm/simulators/wells/GlobalWellInfo.cpp

GitPaean · 2025-11-18T14:27:04Z

jenkins build this failure_report please

as a efficiency factor, it is more reasonable to default it be 1. otherwise, it can be 0 when WCYCLE is not around.

GitPaean · 2025-11-19T11:44:33Z

jenkins build this failure_report please

GitPaean · 2025-11-19T12:07:17Z

The regression failure looks fine. 01_max_watercut_4.pdf is better with this PR.

02_actionx_udq.pdf shows no visual difference.

GitPaean · 2025-11-19T12:32:06Z

This PR looks like can remove a lot of UMFPack failures (not all) in one of the field cases being tested.

UMFPACK V6.3.2 (Jan 20, 2024): WARNING: matrix is singular

bska

Thanks a lot for the updates. This will improve the overall situation and we can come back to it if there are additional failures.

For now I'll merge this into the master branch.

bska · 2025-11-19T13:48:57Z

jenkins build this update_data please

Reason: PR OPM/opm-simulators#6608 opm-common = 07b07df558dd2f6db4c68c36d7880c0223fe9d34 opm-grid = 0ce0e8d833ece748a914bb0ae0191125bb8d4941 opm-simulators = 2164e3c30eb786e74644b19bedf529b399eb3994 ### Changed Tests ### * actionx_udq * max_watercut_4

bska · 2025-11-19T14:01:14Z

jenkins build this opm-tests=1434 please

Automatic Reference Data Update for PR OPM/opm-simulators#6608

bska · 2025-11-19T14:20:26Z

For now I'll merge this into the master branch.

The new reference solutions have been installed on the CI system. Merging this now to activate the fix. Thanks a lot, again, for tracking down this problem.

GitPaean added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Nov 13, 2025

GitPaean force-pushed the efficency_factor branch from 2b5988f to ff16298 Compare November 13, 2025 14:02

GitPaean changed the title ~~testing well efficicey factor~~ m_efficiency_scaling_factors default to be 1. instead of 0. Nov 13, 2025

GitPaean requested review from akva2 and vkip November 13, 2025 15:09

GitPaean marked this pull request as ready for review November 13, 2025 15:10

vkip approved these changes Nov 13, 2025

View reviewed changes

bska reviewed Nov 14, 2025

View reviewed changes

vkip reviewed Nov 14, 2025

View reviewed changes

GitPaean marked this pull request as draft November 14, 2025 15:13

GitPaean force-pushed the efficency_factor branch from ff16298 to 636fc30 Compare November 15, 2025 19:50

GitPaean mentioned this pull request Nov 17, 2025

Not creating identity matrix as reverse of singular well matrix for StandardWell. #6614

Merged

GitPaean mentioned this pull request Nov 17, 2025

copying efficiency_scaling_factor from previous well state #6616

Draft

GitPaean marked this pull request as ready for review November 18, 2025 11:17

GitPaean commented Nov 18, 2025

View reviewed changes

opm/simulators/wells/BlackoilWellModel_impl.hpp Outdated Show resolved Hide resolved

GitPaean commented Nov 18, 2025

View reviewed changes

opm/simulators/wells/GlobalWellInfo.cpp Outdated Show resolved Hide resolved

GitPaean requested review from bska and steink November 18, 2025 11:19

bska reviewed Nov 18, 2025

View reviewed changes

opm/simulators/wells/GlobalWellInfo.cpp Outdated Show resolved Hide resolved

GitPaean force-pushed the efficency_factor branch from e8b6eac to adf7a87 Compare November 18, 2025 14:26

GitPaean force-pushed the efficency_factor branch from adf7a87 to 1a2c337 Compare November 19, 2025 11:43

m_efficiency_scaling_factors default to be 1 in GlobalWellInfo

1a2c337

as a efficiency factor, it is more reasonable to default it be 1. otherwise, it can be 0 when WCYCLE is not around.

GitPaean requested a review from bska November 19, 2025 12:01

bska approved these changes Nov 19, 2025

View reviewed changes

jenkins4opm mentioned this pull request Nov 19, 2025

Automatic Reference Data Update for PR https://github.com/OPM/opm-simulators/pull/6608 OPM/opm-tests#1434

Merged

bska added a commit to OPM/opm-tests that referenced this pull request Nov 19, 2025

Merge pull request #1434 from jenkins4opm/update_opm-simulators_6608

ff7614c

Automatic Reference Data Update for PR OPM/opm-simulators#6608

bska merged commit d791b71 into OPM:master Nov 19, 2025
2 checks passed

m_efficiency_scaling_factors default to be 1. instead of 0. #6608

m_efficiency_scaling_factors default to be 1. instead of 0. #6608

Uh oh!

Conversation

GitPaean commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Nov 13, 2025

Uh oh!

GitPaean commented Nov 13, 2025

Uh oh!

GitPaean commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Nov 13, 2025

Uh oh!

steink commented Nov 13, 2025

Uh oh!

vkip left a comment

Choose a reason for hiding this comment

Uh oh!

GitPaean commented Nov 13, 2025

Uh oh!

bska Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

akva2 Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GitPaean Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

vkip left a comment

Choose a reason for hiding this comment

Uh oh!

GitPaean commented Nov 14, 2025

Uh oh!

GitPaean commented Nov 14, 2025

Uh oh!

bska commented Nov 14, 2025

Uh oh!

GitPaean commented Nov 14, 2025

Uh oh!

GitPaean commented Nov 15, 2025

Uh oh!

GitPaean commented Nov 15, 2025

Uh oh!

GitPaean commented Nov 15, 2025

Uh oh!

GitPaean commented Nov 17, 2025

Uh oh!

GitPaean commented Nov 17, 2025

Uh oh!

GitPaean commented Nov 17, 2025

Uh oh!

GitPaean commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitPaean commented Nov 17, 2025

Uh oh!

GitPaean commented Nov 18, 2025 • edited by bska Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GitPaean commented Nov 18, 2025

Uh oh!

bska left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GitPaean commented Nov 18, 2025

Uh oh!

GitPaean commented Nov 19, 2025

Uh oh!

GitPaean commented Nov 19, 2025

Uh oh!

GitPaean commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GitPaean commented Nov 13, 2025 •

edited

Loading

GitPaean commented Nov 13, 2025 •

edited

Loading

akva2 Nov 14, 2025 •

edited

Loading

GitPaean commented Nov 17, 2025 •

edited

Loading

GitPaean commented Nov 18, 2025 •

edited by bska

Loading

GitPaean commented Nov 19, 2025 •

edited

Loading