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

stricter tolerance for wells with zero rate target #4572

Merged
merged 1 commit into from
Apr 14, 2023
Merged

stricter tolerance for wells with zero rate target #4572

merged 1 commit into from
Apr 14, 2023

Conversation

GitPaean
Copy link
Member

@GitPaean GitPaean commented Mar 30, 2023 •
edited

for StandardWell only at this moment.

It improves the results of a reported case with zero RESV control (black line referee result, green line master branch, red line this PR ).

image

And more importantly, with the defaulted relative slack tolerance, there is some error when coming to the connection rates and the well rates (0 for this circumstance), potentially affecting the mass balance of the whole system (it means the flow out from the reservoir is not equal the flow into the reservoir through the wellbore with 0 well rates).

Ideally stricter tolerance should be applied to all the wells (#4556), but not sure how we should test and proceed with that direction, there might be some substantial investigation efforts involved. This PR is implemented as a temporary approach to fix the immediate issue from the reported case.

@GitPaean
stricter tolerance for wells with zero rate target
7c06ec3 
for StandardWell only at this moment.
@GitPaean
Copy link
Member Author

benchmark please

@GitPaean
Copy link
Member Author

jenkins build this please

@ytelses
Copy link

ytelses commented Mar 30, 2023

Benchmark result overview:

Test Configuration Relative
opm-git OPM Benchmark: drogon - Threads: 1 1.138
opm-git OPM Benchmark: drogon - Threads: 8 0.972
opm-git OPM Benchmark: smeaheia - Threads: 1 1.002
opm-git OPM Benchmark: smeaheia - Threads: 8 1.002
opm-git OPM Benchmark: spe10_model_1 - Threads: 1 0.998
opm-git OPM Benchmark: spe10_model_1 - Threads: 8 0.999
opm-git OPM Benchmark: flow_mpi_extra - Threads: 1 1
opm-git OPM Benchmark: flow_mpi_extra - Threads: 8 1.009
opm-git OPM Benchmark: flow_mpi_norne - Threads: 1 1.002
opm-git OPM Benchmark: flow_mpi_norne - Threads: 8 0.973
  • Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

View result details @ https://www.ytelses.com/opm/?page=result&id=2024

@GitPaean
Copy link
Member Author

I have checked all the jenkins failures locally, most are looking okay.

image

For the case ACTIONX_M1, small difference in the BHP and also field pressure cause the well WI1 switched to WRAT control a few days earlier due to the ACTION setup.
Screenshot from 2023-03-31 11-32-11

@GitPaean
Copy link
Member Author

Anyway, I am making this PR draft to put a little more thinking time.

@GitPaean GitPaean marked this pull request as draft March 31, 2023 11:41
@GitPaean GitPaean marked this pull request as ready for review April 11, 2023 09:30
@GitPaean GitPaean marked this pull request as draft April 11, 2023 09:31
@GitPaean
Copy link
Member Author

Although I believe we should use tighter tolerance for the wells (StandardWell for now, like PR #4556), but there is no capacity to investigate and test from my side. Maybe we should use this PR as a transitional solution. It does not break things and it fixes the reported issue.

@GitPaean GitPaean marked this pull request as ready for review April 11, 2023 11:23
@GitPaean
Copy link
Member Author

jenkins build this please

atgeirr
atgeirr approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

@atgeirr atgeirr left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think we should add the same for MSW afterwards, unless you think a different approach should be used there.

@GitPaean
Copy link
Member Author

GitPaean commented Apr 14, 2023 •
edited

I do believe this is hacky and can only be used a temporary approach. At the same time, it does not break things and it improves the results for certain setups.

Before we want to enforce a stricter or physically more meaningful tolerance for wells in general, we can use this PR for now to prioritize other developments.

I am getting this PR in now.

@GitPaean
Copy link
Member Author

jenkins build this update_data please

@jenkins4opm jenkins4opm mentioned this pull request Apr 14, 2023
Merged
jenkins4opm pushed a commit to jenkins4opm/opm-tests that referenced this pull request Apr 14, 2023
Automatic Reference Data Update for PR OPM/opm-simulators#4572
d5a01f9 
Reason: PR OPM/opm-simulators#4572

opm-common     = 5048b4e9ece004595bb37cb2e42b0e6bc2c40b9e
opm-grid       = 719d3f15e8ca0df1e3c9a348c82e49753d66431b
opm-models     = cec7082cde00cea722869485bd2c3534c604a738
opm-simulators = fc2b0641c89841d2dd3ec437c1ced29234d4c499

### Changed Tests ###

  * base_model2
  * base_model2_welpi
  * multregt_model2
  * actionx_m1
  * multxyz_model2
  * multflt_model2
  * multflt_sched_model2
  * multpvv_model2
  * swatinit_model2
  * endscale_model2
  * hysteresis_model2
  * multiply_tranxyz_model2
  * editnnc_model2
  * norne_reperf
  * 0_base_model6
  * 0a_aquct_model6
  * rxft_smry
  * actionx_wpimult
@GitPaean
Copy link
Member Author

jenkins build this opm-tests=944 please

GitPaean added a commit to OPM/opm-tests that referenced this pull request Apr 14, 2023
@GitPaean
Merge pull request #944 from jenkins4opm/update_opm-simulators_4572
9505a22 
Automatic Reference Data Update for PR OPM/opm-simulators#4572
@GitPaean GitPaean merged commit ceba5c4 into OPM:master Apr 14, 2023
tskille pushed a commit to tskille/opm-simulators that referenced this pull request Apr 15, 2023
@GitPaean @tskille
Merge pull request OPM#4572 from GitPaean/tightenging_tolerance_for_z…
6922226 
…ero_rate_wells

stricter tolerance for wells with zero rate target
@GitPaean GitPaean deleted the tightenging_tolerance_for_zero_rate_wells branch April 17, 2023 09:00
@OPMUSER
Copy link
Contributor

OPMUSER commented Apr 21, 2023

@GitPaean Did this make it into 2023-04-RC1?

@GitPaean
Copy link
Member Author

no, it did not, but I am not sure whether it has to.

@GitPaean GitPaean mentioned this pull request Mar 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atgeirr atgeirr atgeirr approved these changes

@totto82 totto82 Awaiting requested review from totto82

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@GitPaean @ytelses @OPMUSER @atgeirr