adding keyword WSEGSICD

[GitPaean]

Adding the keywor WSEGSICD.

The unit for the item 4 by definition is bar/(rm^3/day)^2 (METRIC), psi/(rft^3/day)^2 (FIELD)....

Since the rft^3 is used for the FIELD unit system instead of rb, I use Volume for the rm^3 and rft^3 instead of ReservoirVolume.

And also I have question about how to write this unit for the item 4.

The confusion is from the definition of the keywords COMPDAT, the transmissibility factor unit is defined as cP.rm^3 /day/bars and the definition of the keyword is Viscosity*ReservoirVolume/Time*Pressure instead of Viscosity*ReservoirVolume/Time/Pressure. It is something we have been using for almost all the cases and it has been working well, while some explanation is required here about the unit rule of the keyword definition.

[joakim-hove]

the transmissibility factor unit is defined as cP.rm^3 /day/bars and the definition of the keyword is ViscosityReservoirVolume/TimePressure

Yes - this is a peculiarity - probably originating in my childhood difficulties with fractions. The division signs in opm-parser should be read as looooong lines, i.e.

                                             Viscosity*ReservoirVolume   
Viscosity*ReservoirVolume/Time*Pressure =    -------------------------
                                                   Time*Pressure

[atgeirr]

The division signs in opm-parser should be read as looooong lines

I think a/b/c is really confusing, and while C accepts it I think it should not. I take the opm-parser policy to mean that there is only one single division sign allowed?

[joakim-hove]

I take the opm-parser policy to mean that there is only one single division sign allowed?

Yes that is right - so to be honest I was a little suprised (and disappointed) that the code would build at all with Kai's original suggestion - this should have killed it?

[GitPaean]

Thanks for the comments. I have updated the unit for the item 4.

[GitPaean]

I think something with this PR.

[ 11%] Generating ParserKeywords.cpp, inlinekw.cpp
terminate called after throwing an instance of 'std::invalid_argument'
  what():  Parsing json input failed
Aborted (core dumped)
make[2]: *** [lib/eclipse/ParserKeywords.cpp] Error 134
make[1]: *** [lib/eclipse/CMakeFiles/opmparser.dir/all] Error 2
make: *** [all] Error 2

[joakim-hove]

I think something with this PR.

yes indeed - it looks like there is a problem! The error message indicates that the json input is invalid; so I would certainly start looking at that.

Does it compile at your place?

[GitPaean]

Does it compile at your place?

I reproduced the problem and now I updated with two small fixed. one is related to [ and the other one is related to :.

It should fix the problem.

[GitPaean]

More problem showing up. I am checking this.

2: Running 417 test cases...
2: /home/kaib/OPM-devel/debug/opm-parser-build/lib/eclipse/inlinekw.cpp(7740): error in "TESTWSEGSICDKeyword": exception thrown by unitSystem.getNewDimension( dimString )
2: /home/kaib/OPM-devel/debug/opm-parser-build/lib/eclipse/inlinekw.cpp(7740): error in "TESTWSEGSICDKeyword": exception thrown by unitSystem.getNewDimension( dimString )

[GitPaean]

Basically, we do not have unit Volume, different from manual, so I will find something to replace it.

[GitPaean]

I will add a unit Volume for all different unit systems. At the same time, I noticed in enum class measure : int of UnitSystem.hpp, volume and rate are used to refer to reservoir volume and reservoir volume / time (reservoir rate?), which can be a little confusing naming. And I am not sure how they are used.

I am suggesting to change them to be reservoir_volume and reservoir_rate respectively.

And I will add an unit called Volume, defined as m^3 (Metric), ft^3 (Field), cc (Lab), m^3(PVT-M), which follows the manual.

Please comment if you object to this.

[joakim-hove]

which can be a little confusing naming.

Yes I know - that was the reason I did not immediately ask you to do this.

Please comment if you object to this.

Go ahead - thank you.

[bska]

And I will add an unit called Volume, defined as m^3 (Metric), ft^3 (Field), cc (Lab), m^3(PVT-M), which follows the manual.

Please be extremely careful here. The current "volume" unit relates to flowing fluids and pore-volumes at reservoir conditions. These go into, among other uses, defining the units of measure for the formation volume factors. In METRIC and PVT-M, those are cubic metres and in LAB it's cubic centimetres. There's therefore a one-to-one correspondence between your proposed Volume unit and the current "volume" unit. However in FIELD, the "volume" is reservoir barrels and there are about 5.61 (42*231/12^3) cubic feet per barrel.

I'd probably name your proposed unit something like "bulkVolume" to distinguish it from the current quantity.

[GitPaean]

However in FIELD, the "volume" is reservoir barrels and there are about 5.61 (42*231/12^3) cubic feet per barrel.

Yes. The rb is the one stopping using the ReservoirVolume.

These go into, among other uses, defining the units of measure for the formation volume factors.

Since in the unit system, we use ReservoirVolume already, I am wondering how damaging it can be to change the variable name of volume in the measure class.

[bska]

Since in the unit system, we use ReservoirVolume already, I am wondering how damaging it can be to change the variable name of volume in the measure class.

You basically have to manually inspect every units.from_si() and units.to_si() call in the entire code-base to verify that the change is safe. I don't think there are any .from_si() or .to_si() calls hidden behind macros, but I'm not willing to bet on it either.

[joakim-hove]

The simplest for you will probably be:

Lengt*Length*Length*Length*Length*Length

[GitPaean]

The simplest for you will probably be:

Yes.

[GitPaean]

Adding a new unit Volume, especially renaming the volume in the measure will include separate PR and some downstream PRs possibly, it should be addressed separately.

I just updated the keyword definition based on Length.

[GitPaean]

jenkins build this please