Add Support for Initialising Constant Flux Aquifers from Restart by bska · Pull Request #4520 · OPM/opm-simulators

bska · 2023-03-09T17:36:41Z

In particular, form constant flux aquifer objects from the restart step's collection if available and properly initialise their total produced volume. To that end also calculate the local face area fraction for the aquifer's connections which handles the constant flux aquifer from the restart file in the case that the aquifer's connections are split across multiple MPI ranks.

bska · 2023-03-09T17:37:58Z

This PR depends on, and contains, PR #4519. It must not be merged into the master branch until the upstream PR has been merged. I will keep this in "draft" mode until that time.

bska · 2023-03-09T17:53:14Z

When combined with upstream PR OPM/opm-common#3437 I am able to restart the simple AQFLUX-01. Here are a couple of summary curves from a parallel base and restart run (restarted at step 3, in the middle of the flux = 0.12 stage

Level	Curves
Field
Aquifer
Well

tskille · 2023-03-10T09:53:18Z

This PR has been tested on a full field model in Equinor and results has been check. Looking forward to see this PR merged into master.

GitPaean · 2023-03-17T14:51:08Z

    {
-        this->initializeConnections();
+        const auto connected_face_area = this->initializeConnections();
+        this->area_fraction_ = this->computeFaceAreaFraction(connected_face_area);


At least in one build, the area_fraction_ caused problem for the parallel restarting. But I have not got clear understanding of the issue. Will post more later.

I need to check whether it is my building is problematic since the output is not very reasonable.

I need to check whether it is my building is problematic

Yes please. For what it's worth, we need something like the area fraction for parallel restart if the set of aquifer connections happens to be split among multiple processes.

since the output is not very reasonable.

What kind of symptoms do you see?

Using the following code to output inside function computeFaceAreaFraction,

const double fraction =(tot_face_area > 0.0) ? connected_face_area / tot_face_area : 0.0; const auto rank = this->ebos_simulator_.vanguard() .grid().comm().rank(); std::cout << " id " << this->aquiferID() << " rank " << rank << " connected_face_area " << connected_face_area << " total_face_area " << tot_face_area << " fraction " << fraction << std::endl;

For running with 1 process computeFaceAreaFraction, (the first number is line number, you can ignore it.)

1234 id 2 rank 0 connected_face_area 25000 total_face_area 25000 fraction 1 1235 id 1 rank 0 connected_face_area 25000 total_face_area 25000 fraction 1

For running with 5 processes

1252 id 2 rank 0 connected_face_area 20000 total_face_area 45000 fraction 0.444444 1253 id 1 rank 4 connected_face_area 0 total_face_area 45000 fraction 0 1254 id 1 rank 1 connected_face_area 0 total_face_area 45000 fraction 0 1255 id 1 rank 2 connected_face_area 0 total_face_area 45000 fraction 0 1256 id 1 rank 3 connected_face_area 25000 total_face_area 45000 fraction 0.555556 1257 id 2 rank 2 connected_face_area 0 total_face_area 5000 fraction 0 1258 id 1 rank 0 connected_face_area 0 total_face_area 5000 fraction 0 1259 id 2 rank 3 connected_face_area 5000 total_face_area 5000 fraction 1 1260 id 2 rank 4 connected_face_area 0 total_face_area 5000 fraction 0 1261 id 2 rank 1 connected_face_area 0 total_face_area 5000 fraction 0

It is a build with clang from CLION, so I need to double check with other build. I will send you the case through slack.

GitPaean · 2023-03-27T09:19:59Z

Yes please. For what it's worth, we need something like the area fraction for parallel restart if the set of aquifer connections happens to be split among multiple processes.

If we always keep the accumulative values (AAQT) and total values (AAQR) the same across the processes, then we might not need this fraction. Just need to remember to communicate when update them. How do you think?

But I still think we should have a understanding how the tricky output was resulted.

bska · 2023-03-27T09:43:45Z

Yes please. For what it's worth, we need something like the area fraction for parallel restart if the set of aquifer connections happens to be split among multiple processes.

If we always keep the accumulative values (AAQT) and total values (AAQR) the same across the processes, then we might not need this fraction. Just need to remember to communicate when update them. How do you think?

I think that's more work than just having this fraction.

But I still think we should have a understanding how the tricky output was resulted.

Probably contributions from unrelated aquifers. The current code is correct when there's just a single aquifer in the model.

GitPaean · 2023-03-27T09:46:24Z

I think that's more work than just having this fraction.

I think it is the same both in term of computation and code. We need to sum across the processes anyway for each time step.

bska · 2023-03-27T11:17:42Z

I think that's more work than just having this fraction.

I think it is the same both in term of computation and code. We need to sum across the processes anyway for each time step.

Sure, but

Your suggestion would lead to doing that twice for each timestep whereas the area fraction is a single number updated once
We'd need to be very careful about updating the cumulative values correctly.

It's still not clear to me why the aquifer objects need to track their total produced volumes. If we defer that work to the summary state (or similar), then there's no problem at all and we can dispense with all of this.

GitPaean · 2023-05-25T21:49:33Z

I will try to add the AQUFLUX case I am referring to opm-tests tomorrow.

GitPaean · 2023-05-26T08:36:52Z

I have open a PR to add the case mentioned above to opm-tests. Maybe we should have the parallel restart tests with that one.

bska · 2023-05-26T08:44:28Z

I have open a PR to add the case mentioned above to opm-tests.

Thanks. I'll update this PR to add that case for restart testing too. In the meantime I'll reset this back to "draft" mode to prevent inadvertent merging.

bska · 2023-05-26T12:09:16Z

jenkins build this please

GitPaean · 2023-05-26T13:29:37Z

one question here, for the parallel restarting running, how many processes will be used?

GitPaean · 2023-05-26T13:29:51Z

jenkins build this update_data please

akva2 · 2023-05-26T13:30:14Z

if nothing is specified; 4.

GitPaean · 2023-05-26T13:31:20Z

jenkins build this update_data please

bska · 2023-05-26T14:14:04Z

update_data please

Sorry, I forgot to register the new regression test. I'll update right away.

GitPaean · 2023-05-26T14:19:39Z

jenkins build this update_data please

Reason: PR OPM/opm-simulators#4520 opm-common = 4e1333deda7808216da451941a150226fb1e1236 opm-grid = 43bcc8646f3017586719901e1f034eed065908d9 opm-models = fb2a4030f57a4e19b71a7555776a582abad3cb5f opm-simulators = 952fe86aad463229e85fb9fe73fd44769e908b7a ### Changed Tests ### * aquflux_02

bska · 2023-05-26T14:54:38Z

jenkins build this opm-tests=977 please

In particular, form constant flux aquifer objects from the restart step's collection if available and properly initialise their total produced volume.

Uses the example models AQUFLUX-01 and AQUFLUX-02 from OPM-Tests. Both sequential and parallel modes activated for both models.

bska · 2023-05-26T18:23:08Z

I consider this work complete now and I'm to merge this so I'm marking it "ready for review".

bska · 2023-05-26T18:23:26Z

jenkins build this opm-tests=977 please

akva2 · 2023-05-26T18:27:10Z

Note #4667 if you still want those timesteps reduced.

bska · 2023-05-26T18:32:43Z

Note #4667 if you still want those timesteps reduced.

I do, but I think I'll make that a follow-up to getting #4667 in later.

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

GitPaean · 2023-05-26T20:00:26Z

The reference update OPM/opm-tests#977 is in, I am getting this in now.

bska force-pushed the load-aquflux-rst-info branch 9 times, most recently from 4be2d1b to 51c4a01 Compare March 16, 2023 16:22

bska force-pushed the load-aquflux-rst-info branch from 51c4a01 to a5177f0 Compare March 17, 2023 12:56

GitPaean reviewed Mar 17, 2023

View reviewed changes

bska force-pushed the load-aquflux-rst-info branch 7 times, most recently from e5242d4 to 230af27 Compare March 27, 2023 08:49

bska force-pushed the load-aquflux-rst-info branch 5 times, most recently from 0286557 to d8b4d6c Compare March 29, 2023 08:55

GitPaean mentioned this pull request May 26, 2023

adding a new AQUFLUX case OPM/opm-tests#974

Merged

bska marked this pull request as draft May 26, 2023 08:44

bska force-pushed the load-aquflux-rst-info branch from 1c01b64 to 9dce129 Compare May 26, 2023 12:00

jenkins4opm mentioned this pull request May 26, 2023

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

Closed

bska force-pushed the load-aquflux-rst-info branch from 9dce129 to d0cd248 Compare May 26, 2023 14:16

bska force-pushed the load-aquflux-rst-info branch from d0cd248 to 7a8bfd9 Compare May 26, 2023 14:36

jenkins4opm mentioned this pull request May 26, 2023

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

Merged

bska added 2 commits May 26, 2023 20:21

Add Support for Initialising Constant Flux Aquifers from Restart

7a77f28

In particular, form constant flux aquifer objects from the restart step's collection if available and properly initialise their total produced volume.

Enable Restart Tests for Constant Flux Aquifers

f156bc9

Uses the example models AQUFLUX-01 and AQUFLUX-02 from OPM-Tests. Both sequential and parallel modes activated for both models.

bska force-pushed the load-aquflux-rst-info branch from 7a8bfd9 to f156bc9 Compare May 26, 2023 18:22

bska marked this pull request as ready for review May 26, 2023 18:23

bska added a commit to OPM/opm-tests that referenced this pull request May 26, 2023

Merge pull request #977 from jenkins4opm/update_opm-simulators_4520

f745164

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

bska mentioned this pull request Oct 2, 2024

fixed: missing TEST_ARGS in test specifications #5639

Merged

Conversation

bska commented Mar 9, 2023

Uh oh!

bska commented Mar 9, 2023

Uh oh!

bska commented Mar 9, 2023

Uh oh!

tskille commented Mar 10, 2023

Uh oh!

GitPaean Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

GitPaean Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

bska Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

GitPaean Mar 17, 2023 • 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 commented Mar 27, 2023

Uh oh!

bska commented Mar 27, 2023

Uh oh!

GitPaean commented Mar 27, 2023

Uh oh!

bska commented Mar 27, 2023

Uh oh!

GitPaean commented May 25, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

akva2 commented May 26, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

akva2 commented May 26, 2023

Uh oh!

bska commented May 26, 2023

Uh oh!

GitPaean commented May 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GitPaean Mar 17, 2023 •

edited

Loading