active phases seriously limited in the flow well model(s)

[andlaus]

the flow well model does not work if the compiled in conservation quantities is a proper superset of those that are actually activated by the deck. e.g., it will hit a few assertions in that case. This is easier to trigger this using ebos, but in principle it also applies to flow:

> ebos SPE1CASE2_OILGAS
[...]
Initializing the problem
WARNING: The number of active phases specified by the deck (2) is smaller than the number of compiled-in phases (3). This usually results in a significant performance degradation compared to using a specialized simulator.
Simulator successfully set up
Applying the initial solution of the "SPE1CASE2_OILGAS" problem
Report step 1/36 at day 0/1095, date = 2015-Jan-01
 Begin time step 1. Start time: 0 seconds, step size: 86400 seconds (1 days, 0 hours, 0 minutes)
ebos: /home/and/src/opm-simulators/opm/autodiff/StandardWell_impl.hpp:43: Opm::StandardWell<TypeTag>::StandardWell(const Opm::Well*, int, const Wells*, const typename Opm::StandardWell<TypeTag>::Base::ModelParameters&, const typename Opm::StandardWell<TypeTag>::Base::RateConverterType&, int, int) [with TypeTag = Ewoms::Properties::TTag::EbosTypeTag; typename Opm::StandardWell<TypeTag>::Base::ModelParameters = Opm::BlackoilModelParametersEbos<Ewoms::Properties::TTag::EbosTypeTag>; typename Opm::StandardWell<TypeTag>::Base::RateConverterType = Opm::RateConverter::SurfaceToReservoirVoidage<Opm::BlackOilFluidSystem<double>, std::vector<int> >]: Assertion `num_components_ == numWellConservationEq' failed.

Received signal 6 ("Aborted"). Trying to reset the terminal.

if the binary is compiled in release mode, it also crashes. That said, ebos_plain works just fine as for the threephase version of the deck, i.e. core eWoms does not have any problem with this (but it is slightly slower, of course)...

[GitPaean]

I agree the component/phase system is one big piece to be addressed later eventually.

Are you suggesting we should be able to use a three-phase blackoil simulator to simulate a two-phase oil-gas case? If it is the case, let us figure it out.

[OPMUSER]

In the real world of oil and gas reservoirs water is always present as it is the initial phase occupying the pore volume. Hydrocarbons migrate in the reservoir trap partially displacing the in situ water. With this mind the various two phase combinations are:

• Gas-Water, this is a very common combination for dry gas reservoirs to be modelled using these two phases and is something that should be handled by OPM Flow. This is handled by declaring the GAS phase in the RUNSPEC section and using the PVTDG or PVZG keywords and the RVCONST or RVCONSTT in the PROPS section.
• Oil-Water, not common in conventional reservoirs but very common in heavy oil reservoirs where the GOR of the oil is negligible. This is handled by declaring the OIL phase in the RUNSPEC section and using the PVTDO or PVCDO keywords and the RSCONST or RSCONSTT keywords in the PROPS section.

OPM Flow should handle the above phase formulations, as stated above. Testing the Gas-Water combination causes OPM Flow to abort because the RVCONST keyword is not recognized.

Currently, the way to do this is to declare all three phases in the RUNSPEC section and set the PVT properties of the missing phases to have negligible effect on the calculations. This means that the simulator formulates and solves a three phase problem, rather than the less computational intensive two phase problem.

However, the Oil-Gas case is not a realistic phase combination.

[andlaus]

Are you suggesting we should be able to use a three-phase blackoil simulator to simulate a two-phase oil-gas case? If it is the case, let us figure it out.

Yes: The point of this issue is that the only thing which prevents doing this is the well model. note that if a flow_blackoil simulator was introduced as you proposed, it would have exactly the same issue as ebos does have now.

For somebody who's more familiar with the well model's code than me, this issue should also not be very difficult to fix: Just add dummy equations for the deactivated components (core-eWoms does this) or use your new-and-shiny model based on dynamically-sized AD objects.

On a different well model related note, there also seem to be severe issues with the scaling of the well equations (or rather with B, the matrix that couples the well equations to the reservoir model): If the scaling factor is wildly different between the two submodels, the linear solver will have a very hard time converging. The worst offender for this currently is the energy conservation equation where the well model uses 1.0 (?) as a scaling factor and the reservoir model uses something in the order of 1e-6, but I'm pretty sure that this also applies to mass conservation. this should also be not too hard to fix by scaling the well equations by ebosSimulator.model().eqWeight(cellIdx, eqIdx). (I can send you a deck if you're interested in fixing it.)

[GitPaean]

For somebody who's more familiar with the well model's code than me, this issue should also not be very difficult to fix: Just add dummy equations for the deactivated components (core-eWoms does this) or use your new-and-shiny model based on dynamically-sized AD objects.

@totto82 might be the best one to handle this. I am still nervous when touching the component/phase indices related, especially when solvent is involved. Although I am still not convinced that we have this requirement. Eventually we should not limit what components we are dealing with during compilation time.

For the scaling related, energy one is easy to fix. For the mass conservation equations, the scaling factor I suppose is still 1.0, since they are pure surface volume.

Can you explain a little more about how the eqWeight works for different components/equations?

[andlaus]

For the scaling related, energy one is easy to fix.

can you open a PR?

For the mass conservation equations, the scaling factor I suppose is still 1.0, since they are pure surface volume.

for flow_legacy, that statement is probably true, for the other simulators which we're looking at not necessarily: IIRC, eclipse uses something like the B factors and ebos has a different weight for the water equation.

Can you explain a little more about how the eqWeight works for different components/equations?

the concept is to simply multiply a row in the system of equations of the reservoir model by the value returned, i.e., you pass it an index of a degree of freedom (cell) and the index of the conservation quantity you're interested in (e.g., water) and the method returns a scalar value for it. the following pseudo code scales all reservoir equations for a given cell:

for (unsigned eqIdx = 0; eqIdx < Indices::numEq; ++ eqIdx)
  eqVector[cellIdx][eqIdx] *= model.eqWeight(cellIdx, eqIdx);

[GitPaean]

the concept is to simply multiply a row in the system of equations of the reservoir model by the value returned, i.e., you pass it an index of a degree of freedom (cell) and the index of the conservation quantity you're interested in (e.g., water) and the method returns a scalar value for it. the following pseudo code scales all reservoir equations for a given cell:

I mean, what are the values for the water equation, gas equation and oil equation. Are you suggesting that the weight can be different for different cells?

[andlaus]

Are you suggesting that the weight can be different for different cells?

at the moment they are identical across cells, but as said: E100 seems to use the B factors as weights. @totto82 and me looked at this a while ago but we concluded that it was not necessary for now.

[andlaus]

(in this case I mean the formation volume factor with of a phase with "B", not the coupling matrix between the reservoir and the well equations.)

[GitPaean]

at the moment they are identical across cells, but as said: E100 seems to use the B factors as weights

We can try it, while not sure how to prioritize it. Just pin this issue if nothing happens.

[GitPaean]

at the moment they are identical across cells, but as said: E100 seems to use the B factors as weights.

If they are identical across cells, are we using the average B as weights? or something else. I just want to make sure to test with the way ebos is doing. For sure, we can test different options, while ebos option is tweaked/tested in some sense.

[andlaus]

ok, I'll keep nagging.

If they are identical across cells, are we using the average B as weights?

currently we use fixed constants because these values only need to be at the right order of magnitude; that said, E100 seems to use cell-specific values. Note that, since flow uses the same code as ebos for linearization of the reservoir equations, I'm also not entirely sure that the water equation is unscaled there, so we might have been just "lucky" that this did not fall on our feet so far.

when you fix this, you should probably have a look at the eqWeight() function yourself.

[GitPaean]

Basically, at the moment, for water it is 10, gas it is 1.0, oil it is 650.

The rationale is hard to me here. It is looks like based on the surface density, while water is significantly reduced. What is the reason? Is there some findings based on testing?

[GitPaean]

And also, will these eqWeight impact the determination of the convergence? I mean do we check the tolerance of the weighted equations or un-weighted?

[andlaus]

The rationale is hard to me here. It is looks like based on the surface density, while water is significantly reduced. What is the reason? Is there some findings based on testing?

the idea is to make all kilograms approximately equal (except water because nobody cares about water.)

And also, will these eqWeight impact the determination of the convergence?

for the non-linear solver in flow: no. for the linear solver: yes. for the non-linear solver in ebos: yes. Also, if the well model severely screws up the coupling matrices, the linear solver will blow up.

[GitPaean]

Good. That is why we should discuss. It might be tricky to find something out by purely checking code.

the idea is to make all kilograms approximately equal (except water because nobody cares about water.)

If it does not impact the performance in any significant way, it is good to take care of it. Reservoir condition can be pretty general depends on the scenario

for the non-linear solver in ebos: yes

If I remember correctly, there is some case ebos perform so well with ebos. Maybe this is the key part. We have some observations about the convergence checking for flow, it is too strict for some cases, while it is too loose for some other ones. Maybe we should have a better way to handle it.

[andlaus]

If it does not impact the performance in any significant way, it is good to take care of it. Reservoir condition can be pretty general depends on the scenario

we have seen some decks which exhibit significantly better performance when water is de-prioritized (e.g., CO2CASE). Anyway, I agree with you that the well model should not get in the way of such things...

If I remember correctly, there is some case ebos perform so well with ebos. Maybe this is the key part.

possibly, but I think that this is likely only a small part of the reason. Anyway, I don't want to waste weeks of work for cleaning up flow's crufty getConvergence() code. (The development experience with flow is horrible IMO. I'm open to let myself be bribed if the compensation for it is sufficient, though.)

[GitPaean]

The development experience with flow is horrible IMO

Totally agree, while possibly for different reasons.

[andlaus]

I don't really know what your reasons are (I have a suspicion), but mine are that supposedly trivial changes are almost impossible to conduct, cf. #1775.

[GitPaean]

I don't really know what your reasons are (I have a suspicion), but mine are that supposedly trivial changes are almost impossible to conduct, cf. #1775.

Maybe you changed too many things in #1775. Maybe you can minimize the change for the purpose of the PR first. It is probably one line somewhere changes something.

[andlaus]

Maybe you changed too many things in #1775. Maybe you can minimize the change for the purpose of the PR first. It is probably one line somewhere changes something.

if with "too many" you mean "specify the correct episode size at the beginning of a report step", you're right: the other changes where mainly ripple down effects of this. That said, I'd be very happy if you tried this yourself and I'd be even happier if you succeeded...

[GitPaean]

And also, did you test the PR with small models? If the residual is identical there, then it is likely to be related to time step chopping.

We need to chop time steps to get through the big models.

[GitPaean]

the other changes where mainly ripple down effects of this

There are something moving around, the order might matter. (style changes or rewriting are just noise. It is always good to do it in different PRs.)

[andlaus]

And also, did you test the PR with small models? If the residual is identical there, then it is likely to be related to time step chopping.

I wasted a week of work to make the test suite work. is that what you mean?

There are something moving around, the order might matter. (style changes or rewriting are just noise. It is always good to do it in different PRs.)

as said: I only want the change itself, I don't care if my patch or anyone else's gets merged to that effect in the end. So please, descent to this hell yourself if you dare.

[GitPaean]

as said: I only want the change itself, I don't care if my patch or anyone else's gets merged to that effect in the end. So please, descent to this hell yourself if you dare.

I am not following. I have my own work to do, and I am just suggesting information that might help.

[andlaus]

I am not following. I have my own work to do, and I am just suggesting information that might help.

okay, that's fine with me. Note that in this case you just agreed to my point from above: the development experience for flow is awful because supposedly trivial changes are anything-but. (such a fix should not take longer that 20 minutes to implement and I'm pretty sure that for ebos it wouldn't.)

[GitPaean]

the development experience for flow is awful because supposedly trivial changes are anything-but

Yeah. But due to totally different reason and also depend on what you mean by flow. When I say flow, I mean everything is needed to run flow.

Currently, the things are so coupled, sooner or later, it will become a much bigger problem to change or extend. Currently, it is already hard.

If things are less coupled, we can test small part separately and we can refine the implementation of small parts. It is much easy to develop and debug and extend. When everything is coupled together, it is complicated to think about the consequence when we did some change.

[andlaus]

Currently, the things are so coupled, sooner or later, it will become a much bigger problem to change or extend. Currently, it is already hard.

I heard your words. If you want to make things less coupled, how about merging #1742?

[GitPaean]

I heard your words. If you want to make things less coupled, how about merging #1742?

I am not familiar enough with the content of PR #1742 and I will not be able to allocate substantial time for it.

[andlaus]

you may want to try to convince some of your colleagues who stated the same sentiment in the past and who are also co-maintainers of opm-simulators. rest be assured that (for now) merging #1742 does not change anything in terms of numerics, but it considerably decouples the flow infrastructure from eWoms.

[GitPaean]

sorry. You should request a proper review from a proper reviewer. I am not familiar with the PR enough to say anything.