Make the TPFA linearizer default for 1/2/3 phase blackoil. #4017

atgeirr · 2022-08-19T13:10:12Z

All other use cases still use the traditional linearizer.

atgeirr · 2022-08-19T13:10:24Z

benchmark please

atgeirr · 2022-08-19T13:36:35Z

jenkins build this please

blattms · 2022-08-19T14:25:42Z

Is there a reason why only 3-phase?

ytelses · 2022-08-20T13:26:26Z

Benchmark result overview:

Test	Configuration	Relative
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 1	1.081
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 8	1.066
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 1	1.127
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 8	1.135
opm-git	OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 1 - FOPT (Total Oil Production At End Of Run)	1
opm-git	OPM Benchmark: flow_mpi_norne_4c_msw - Threads: 8 - FOPT (Total Oil Production At End Of Run)	1

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=1752

bska · 2022-08-20T18:33:35Z

A 6% to 8% total time improvement on "extra" is encouraging indeed. We just need to figure out and solve whatever's happening with the regression test failures and then I support merging this into the master branch.

atgeirr · 2022-08-21T20:32:42Z

Is there a reason why only 3-phase?

No, other than requiring me to change more files, so this is more minimal.

akva2 · 2022-08-24T15:27:36Z

jenkins build this please

atgeirr · 2022-08-25T08:34:47Z

jenkins build this please

atgeirr · 2022-09-01T11:56:56Z

The remaining failure is due to boundary conditions. I will add support for boundary conditions to this PR, and also move two-phase to the TpfaLinearizer.

atgeirr · 2022-09-02T13:40:28Z

jenkins build this please

This affects the gas/water model, which should not have dissolution, to be consistent with definitions of FluidState and ScalarFluidState in the BlackOilIntensiveQuantities class.

atgeirr · 2022-09-30T09:04:12Z

jenkins build this please

bska · 2022-09-30T09:06:26Z

Does this new enableDissolution predicate also handle the "humid gas" case (#4034)?

atgeirr · 2022-09-30T10:52:44Z

The DIFFUSE case failed, as expected. Will address by instantiating two simulator variants with diffusion on (using the traditional linearizer) and adding runtime dispatch as for the rest of the variants.

atgeirr · 2022-09-30T10:58:49Z

Does this new enableDissolution predicate also handle the "humid gas" case (#4034)?

I do not know. I looked at the flow_ebos_gaswater_saltprec_vapwat.cpp variant, which I presume sets this up, it looks like it uses the regular two-phase indexes, meaning that the "compositionSwitchIdx" should be invalid as it is a gas/water case. I do not know the details of this variant, so I cannot really tell. There were no new test failures though, so I guess it is fine.

bska · 2022-09-30T11:10:49Z

Does this new enableDissolution predicate also handle the "humid gas" case (#4034)?

I do not know. [...] There were no new test failures though, so I guess it is fine.

There might not be any regression tests running with humid gas at the moment. @goncalvesmachadoc: Did your team build and/or activate regression tests for this feature? I know there is an in-progress PR (#4111) that creates unit tests for the equilibration facility, but I don't see any currently running regression tests.

This is only instantiated for two-phase gas/oil and for 3-phase blackoil. Runtime safeguards have been added to avoid the mistake of running with a simulator combination that silently ignores DIFFUSE.

atgeirr · 2022-09-30T13:30:45Z

jenkins build this please

bska · 2022-09-30T14:27:08Z

jenkins build this serial please

bska

This looks good for to me. I only had one remark concerning making the FlowMainEbosType public. At least superficially, that seems unrelated to this work.

bska · 2022-09-30T15:26:17Z

opm/simulators/flow/Main.hpp

@@ -236,6 +235,7 @@ class Main
        return exitCode;
    }

+    using FlowMainEbosType = FlowMainEbos<Properties::TTag::EclFlowProblem>;


I'm probably blind, but I didn't see any of the new code using this type. Is there a reason it has to become public?

The initFlowEbosBlackoil() method is public, and returns a unique_ptr to this type (see just below these lines), so I think it should be defined publicly, and defined here just before the function that refers to it.

At first, I assumed I must change this type (and to get the fast assembly supported from the python interface I suppose we must), but realized that was not really necessary, that it was only used within the method discussed. I decided to make the minor improvement still, although not really required here.

I didn't see any of the new code using this type.

The initFlowEbosBlackoil() method is public, and returns a unique_ptr<> to this type (see just below these lines),

Okay, so this was indeed an unrelated change.

I think it should be defined publicly, and defined here just before the function that refers to it.

That's fine. It just stood out to me as something curious about the PR.

goncalvesmachadoc · 2022-10-01T17:42:18Z

Does this new enableDissolution predicate also handle the "humid gas" case (#4034)?

I do not know. [...] There were no new test failures though, so I guess it is fine.

There might not be any regression tests running with humid gas at the moment. @goncalvesmachadoc: Did your team build and/or activate regression tests for this feature? I know there is an in-progress PR (#4111) that creates unit tests for the equilibration facility, but I don't see any currently running regression tests.

There are only one test for two-phase (gas-water) humid gas case. We haven't added a wet-humid (three-phase) test case yet since we were facing some convergence issues. I finished the unit tests that covers the 3-phase case. Will go ahead and push a running 3-phase test case for DISGAS + VAPWAT + VAPOIL regression test.

goncalvesmachadoc · 2022-10-01T17:46:09Z

Does this new enableDissolution predicate also handle the "humid gas" case (#4034)?

I do not know. I looked at the flow_ebos_gaswater_saltprec_vapwat.cpp variant, which I presume sets this up, it looks like it uses the regular two-phase indexes, meaning that the "compositionSwitchIdx" should be invalid as it is a gas/water case. I do not know the details of this variant, so I cannot really tell. There were no new test failures though, so I guess it is fine.

There is also flow_ebos_brine_precsalt_vapwat.cpp that we are using for modelling water evaporation + precipitation in condensate gas reservoirs (compositionSwitchIdx valid).

atgeirr · 2022-10-03T07:29:51Z

The effect of the enableDissolution flag is to enable Rs and Rv storage in the fluid state. I think this should correspond to the presence of a valid compositionSwitchIdx variable index, as done here.

The fix to make the fluid states consistent should be made anyway, and is not directly related to the main part of this PR (but the problem was discovered due to it). If simulators no longer give correct results with evaporation etc., we should discuss why and fix it, but I am inclined to think that in that case it is the simulator setups that are wrong, not the code changed here.

I think this PR should be merged now, and any necessary fixes for the humid gas etc. made afterwards in a separate PR.

atgeirr · 2022-10-03T10:01:19Z

Merging now. @goncalvesmachadoc please test if this breaks your humid gas case. If so, I will help you get it back to working order in time for the upcoming release.

goncalvesmachadoc · 2022-10-07T09:10:16Z

Merging now. @goncalvesmachadoc please test if this breaks your humid gas case. If so, I will help you get it back to working order in time for the upcoming release.

Hi Atgeirr, thanks for checking with us. This PR did not affect the humid-wet gas case (flow_brine_precsalt_vapwat module).

atgeirr force-pushed the make-fast-assembly-default branch from 655917b to 912a934 Compare August 25, 2022 08:34

This was referenced Sep 1, 2022

Bugfix: make TPFA version of local linearizer work for two-phase runs. OPM/opm-models#713

Merged

Ensure intensive quantities are calculated at start of simulation. #4044

Merged

atgeirr force-pushed the make-fast-assembly-default branch from 912a934 to cf902aa Compare September 2, 2022 13:39

atgeirr changed the title ~~Make the TPFA linearizer default for 3 phase blackoil.~~ Make the TPFA linearizer default for 1/2/3 phase blackoil. Sep 2, 2022

atgeirr force-pushed the make-fast-assembly-default branch from cf902aa to 3af3e8a Compare September 5, 2022 11:29

atgeirr mentioned this pull request Sep 9, 2022

Add support for directional relative permeabilities #4048

Merged

atgeirr added 3 commits September 30, 2022 09:21

Make the TPFA linearizer default for 3 phase blackoil.

f1762a1

Make one- and two-phase sims also use fast assembly.

616b145

Bugfix: fix dis/en-abling dissolution in the fluid state.

b9e157e

This affects the gas/water model, which should not have dissolution, to be consistent with definitions of FluidState and ScalarFluidState in the BlackOilIntensiveQuantities class.

atgeirr force-pushed the make-fast-assembly-default branch from 3af3e8a to b9e157e Compare September 30, 2022 09:01

Ensure DIFFUSE is supported.

f603370

This is only instantiated for two-phase gas/oil and for 3-phase blackoil. Runtime safeguards have been added to avoid the mistake of running with a simulator combination that silently ignores DIFFUSE.

bska reviewed Sep 30, 2022

View reviewed changes

atgeirr merged commit dacb774 into OPM:master Oct 3, 2022

atgeirr deleted the make-fast-assembly-default branch October 3, 2022 11:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the TPFA linearizer default for 1/2/3 phase blackoil. #4017

Make the TPFA linearizer default for 1/2/3 phase blackoil. #4017

atgeirr commented Aug 19, 2022

atgeirr commented Aug 19, 2022

atgeirr commented Aug 19, 2022

blattms commented Aug 19, 2022 via email

ytelses commented Aug 20, 2022

bska commented Aug 20, 2022

atgeirr commented Aug 21, 2022

akva2 commented Aug 24, 2022

atgeirr commented Aug 25, 2022

atgeirr commented Sep 1, 2022

atgeirr commented Sep 2, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

atgeirr commented Sep 30, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

bska left a comment

bska Sep 30, 2022

atgeirr Oct 2, 2022

bska Oct 2, 2022

goncalvesmachadoc commented Oct 1, 2022

goncalvesmachadoc commented Oct 1, 2022

atgeirr commented Oct 3, 2022

atgeirr commented Oct 3, 2022

goncalvesmachadoc commented Oct 7, 2022

Make the TPFA linearizer default for 1/2/3 phase blackoil. #4017

Make the TPFA linearizer default for 1/2/3 phase blackoil. #4017

Conversation

atgeirr commented Aug 19, 2022

atgeirr commented Aug 19, 2022

atgeirr commented Aug 19, 2022

blattms commented Aug 19, 2022 via email

ytelses commented Aug 20, 2022

bska commented Aug 20, 2022

atgeirr commented Aug 21, 2022

akva2 commented Aug 24, 2022

atgeirr commented Aug 25, 2022

atgeirr commented Sep 1, 2022

atgeirr commented Sep 2, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

atgeirr commented Sep 30, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

atgeirr commented Sep 30, 2022

bska commented Sep 30, 2022

bska left a comment

Choose a reason for hiding this comment

bska Sep 30, 2022

Choose a reason for hiding this comment

atgeirr Oct 2, 2022

Choose a reason for hiding this comment

bska Oct 2, 2022

Choose a reason for hiding this comment

goncalvesmachadoc commented Oct 1, 2022

goncalvesmachadoc commented Oct 1, 2022

atgeirr commented Oct 3, 2022

atgeirr commented Oct 3, 2022

goncalvesmachadoc commented Oct 7, 2022