Differences in minPvFillPropos in ebos and legacy?

[blattms]

I tried to compare the pinching of cells (is this the correct phrase?) in ebos with the original Geology::minPvFillProps_. The code is highly refactored (different loops, breaks etc.) which make comparison rather hard. To me it seems like there is a subtle difference, though.

The ewoms version does not seem to keep track of the changed volume of cells above but always uses the original value of porvData[cartElemIdx] and the cell volume. Is that not neglecting merges happened above?

Let i be the current index, cart(i) the cartesian index of the cell, r be the cell indices of cells that are merged, porv the PV from eclipse, ntg the "NTG" from eclispe then ebos does IMO:

pv=porv[cart(i)];
for(j in r)
  pv+=porv[cart(j)];
porosity[i]=pv/grid.cellVolume(i); // Should not the volume change, too?

ntg is never modified.

legacy does:

ntg[cart(i) *= eclipseGrid.getCellVolume(cart(i));
totalvol = eclipseGrid.getCellVolume(cart(i));
for( j in r) {
  totalvol += eclipseGrid.getCellVolume(cart(j));
  ntg[cart(i)] += ntg[cart(j)] * eclipseGrid.getCellVolume(cart(j)); // wrong if j was modified previously?
}
ntg[cart(i)] /= totalVolume;

I have no idea how the porosity is calculated in legacy, but these two codes seem a tiny bit different.

[andlaus]

I'm not sure if I understand you correctly: do you mean NTG and MULTPV are not considered? if I remember correctly, the PORV grid property from opm-parser already includes MULTPV and NTG (except for the cells where the pore volume is specified directly and the multipliers must not be applied). did I ever mention that this part of the code is tricky?

porosity[i]=pv/grid.cellVolume(i); // Should not the volume change, too

I guess this is correct, because we want to keep the pore volume constant while we don't alter the grid. Obviously this can lead to weird situations where the porosity is larger than 1.0...

I have no idea how the porosity is calculated in legacy, but these two codes seem a tiny bit different.

differences are possible, in particular because the code in eWoms tends to be newer. as far as I know, the porosity in DerivedGeology is the pristine value specified in the deck.

[alfbr]

In flow, pore volume was calculated after the grid processing. Since we handle MINPV+PINCH by extending the cell above or below to fill the void of the removed cell, the cell volume gets larger than what you find in Eclipse. In flow_ebos the original cell volume is kept, which makes the results closer to what you find in Eclipse. Personally, I actually like better what we did in flow, but it is no big deal. Hopefully we will be able to re-factor the pore volume code soon anyway. Today's solution is a bit of a hack.

[andlaus]

In flow, pore volume was calculated after the grid processing.

right. the same is actually the case here too, so the total volume of the cell includes the volume of the disabled cells, at least if Dune::CpGrid works correctly. (hopefully.) did I ever mention that this code is tricky?

In flow_ebos the original cell volume is kept

I'd like to mention that "original" means "whatever Dune::CpGrid produces for the active cells", which is identical to what's used by flow_legacy?! That aside, the total volume of the cell is not relevant for reservoir simulations -- only the pore volume is.

Personally, I actually like better what we did in flow, but it is no big deal

the grids should be identical. Though I'm not sure if it is (or ever was) used for the visualization output, so possibly what's shown by ResInsight is slightly different from what the simulator uses... the same issues applies to flow_legacy, though.

[alfbr]

the same is actually the case here too, so the total volume of the cell includes the volume of the disabled cells

Good to know. If that is the case, then I was misled when the implementation was done. I have to admit that I never checked though.

[andlaus]

I was misled when the implementation was done.

I can't blame you: as @blattms noted, that stuff is very opaque and my impression is that everyone has a slightly incorrect idea of this initially.

[andlaus]

It seems this was working as it should, albeit in non-obvious ways and with many side-conditions. closing the issue. please reopen if this does not work properly.