use the PORV 3D property from opm-parser to determine the pore volume
this simplifies handling the pore volume by centralising the code,
i.e., moving it into opm-parser. (in particular, it makes the MINPV
handling consistent with the active cells which get removed by the
grid.) If eclipse turns out to be inconsistent here, we need to deal
with atrocities like the MULTREGP keyword on a case-by-case basis,
i.e., it would considerably uglify the code and be an additional
see OPM/opm-parser#909 for the opm-parser part.
I am not sure this will be correct when we use the void-filling way to do MINPV? I will look at this tomorrow.
I interpreted this
that `flow` calculates the pore volume itself (is that wrong?). If this is the case, this makes it difficult to keep all the places where the pore volume is needed consisted, but since we're talking about eclipse, I don't know if they are supposed to be consistent...
This change is not correct. While the simulator does use the porosity, net-to-gross, and multiplier information directly from the simulation case, it (the simulator) needs to use the grid's notion of the bulk (geometric) volume of a cell. This value is typically different from that which is calculated from the unhandled corner-point data (COORD and ZCORN)--especially in the case of our min-pv handling.
Having access to "PORV / BULKV" from the parser would be useful, though.
if I did not misinterpret your statement or the opm-parser code, opm-parser takes care of NTG and MULTPV when dealing with PORV:
in any case, not considering the PORV keyword for the storage term at all seems to be incorrect to me: why should it be possible to specify the pore volume explicitly if not for this? Again, Eclipse may do strange things here...
(in any case, this issue should not matter for any data set which is featured by opm-data...)
> Having access to "PORV / BULKV" from the parser would be useful, though.
that's probably not very complicated to implement BULKV in opm-parser. I'll have a look.
hm, I can't find a "BULKV" keyword in the documentation? is this a non-standard (Frontsim?) keyword?
`BULKV` is listed among the _ECLIPSE 300 only_ keywords under Operater EDIT section
ECLIPSE 300 only HEATTX HEATTY HEATTZ
ECLIPSE 300 only ROCKV BVRMX BVRMY
ECLIPSE 300 only BVRMZ BULKV DTPX1
ECLIPSE 300 only DTPX2 DTPY1 DTPY2
ECLIPSE 300 only DTPZ1 DTPZ2
To be honest, I did not mean the E300 "BULKV" property, but rather a derived thing that uses the PORV vector and divides out the bulk volume as computed from the corner-point description (COORD+ZCORN)--a sort of "effective" porosity if you will. This property would partially account for user specifying PORV values in the simulation deck.
Our approach to handling [PINCH/MINPV](https://github.com/OPM/opm-core/blob/master/opm/core/grid/MinpvProcessor.hpp#L84-L93) is incompatible with the user explicitly assigning (or changing) the PORV vector in the simulation deck. Unless (or until) we alter our grid analysis (i.e., function [`create_grid_cornerpoint()`](https://github.com/OPM/opm-core/blob/master/opm/core/grid/cornerpoint_grid.c#L163-L164)) we are not able use PORV directly for any of flow's calculations. Consequently, the original version of this code must remain in place.
> we are not able use PORV directly for any of flow's calculations
I think we can do this pretty well: we should accumulate the PORVs of removed cells into the remaining cells, that would account for the cell combination we do, while still letting users manipulate PORV in the deck.
> BULKV is listed among the ECLIPSE 300 only keywords under Operater EDIT section
seems like this is a newish keyword: my (oldish) RM does not mention BULKV anywhere. (I used full-text search.)
> [...] we are not able use PORV directly for any of flow's calculations.
@atgeirr and me just had a talk about this, and I think we have a solution. it will take while to implement (and even quite a bit longer to verify), though.
ok, I've switched the PR to a more comprehensive approach for the pore volume: it adds the pore volume of all cells which get disabled due to MINPV to the pore volume of the active-below cell which is closest.
since it only uses the "PORV" grid property from `opm-parser`, it has a few advantages like support for MULTREGP, explicitly specifying the cell pore volume via PORV and the pore volume should now be consistent with the deck. (the current approach uses applies the porosity of the active cell to the disabled ones which is wrong: remember that the disabled cells do not necessarily exhibit a small overall volume, but just a small pore volume.)
the performance of the norne deck seems to be unchanged or slightly better (on my machine solver time was 1751.79 vs 1637.01 seconds), and the results are identical except for numerical noise:
(this is the BHP of the well which seems to deviate most, most other wells do not exhibit any discernible deviation!)
> (on my machine solver time was 1751.79 vs 1637.01 seconds.)
to be clear: the 1751.79 seconds belong to the current master, the 1637.01 to this PR.