Krowgend add to factory water_oil_gas - #471
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #471 +/- ##
=======================================
Coverage 98.79% 98.79%
=======================================
Files 18 18
Lines 2647 2664 +17
=======================================
+ Hits 2615 2632 +17
Misses 32 32 ☔ View full report in Codecov by Sentry. |
|
Included in this PR there should be a corresonding update to the docs on https://equinor.github.io/pyscal/modelling.html |
2ee72a3 to
f385e8b
Compare
Documentation is updated |
8138a5e to
27a7eab
Compare
| params_copy["kroend"] = params_copy.pop("krowend") | ||
| params_copy.pop("krogend", None) | ||
| return params_copy | ||
| params_copy = params.copy() |
There was a problem hiding this comment.
Last 3 lines are also present inside the if statement, should be simplified so that the code is not duplicated.
Also the case for kro_endpoint_go.
There was a problem hiding this comment.
fixed and also added a warning when the removing krowend and krogend. also, added a test for correct warning
| to the oil curve parametrization for WaterOil and GasOil. From | ||
| pyscal 0.6.0, krogend and krowend are merged to kroend. | ||
| After pyscal 0.8 presence of krogend and krowend is a ValueError | ||
| After pyscal 0.14 presence krogend and krowend is accepted as |
There was a problem hiding this comment.
@larsevj, still valid version or should it be 0.15?
There was a problem hiding this comment.
Could maybe be version 0.16 i think.
2374cdd to
376806d
Compare
|
It seems most of the PNG files are changed due to a slight change of font, maybe font updates upstream? Can you merge a commit first with only regenerated PNG files, and then rebase this PR? This is to be able to look at changes to the PNGs from this code change only. |
| After pyscal 0.8 presence of krogend and krowend is a ValueError | ||
| After pyscal 0.16 presence krogend and krowend is accepted as | ||
| parameters for create_wateroilgas, but is still ValueError for | ||
| create_water_oil and create_gas_oil is a |
@berland will do. pyscal/.github/workflows/pyscal.yml Lines 117 to 121 in 4d9afc9 The fonts generated locally in this commit will not appear in their original form in the online documentation, because the font setup on the GitHub build server differs from the local configuration. In current documentation the online fonts are different from previous versions. Suggest to finish #471, but the workflow and structure could/should be changed in a separate PR: images generated automatically should be in docs/images/generated and added to .gitignore so auto‑generated files are not commited. Binary files, could be commited and placed in a separate folder docs/images/static |
There was a problem hiding this comment.
There is a file already in the repo called "gaswater-dci3_paleogas.png". Something is mixed up?
There was a problem hiding this comment.
Is this image addition related to this PR?
| "gaswater-dci1", | ||
| "gasoil-condensate-cid2", | ||
| "wateroil-condensate-idc2", | ||
| "gaswater-condensate-icd2", |
There was a problem hiding this comment.
(I would expect only three images coming out of this)
| .. image:: images/wet-condensate_field_illustration.png | ||
| :width: 600 | ||
|
|
||
| Following processes are associated with the regions depicted in the figure above: |
There was a problem hiding this comment.
Inject "The " in front of "following"
| ------------------------------------------- | ||
|
|
||
| .. image:: images/gasoil-cid2.png | ||
| This field scenario is for a three-phase Eclipse simulation allowing for condendsate |
There was a problem hiding this comment.
write "reservoir simulation" rather than the vendor specific Eclipse simulation.
| ------------------------------------------- | ||
|
|
||
| .. image:: images/gasoil-cid2.png | ||
| This field scenario is for a three-phase Eclipse simulation allowing for condendsate |
There was a problem hiding this comment.
typo: condendsate -> condensate
| **The processes are described in the following figures:** | ||
|
|
||
| Water displacing gas (ICD2) | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
align number of tilde characters with the length of the header (hmm, I thought rstcheck would catch those)
|
|
||
|
|
||
| Water displacing oil (IDC2) | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
same as above, align line lengths
berland
left a comment
berland
left a comment
There was a problem hiding this comment.
I don't think I have any further comments, approval from me pending the minor last things getting fixed. Good job!
make doc figures ruff check fix comic font + docs/pyscal removal test simplified + test warning Fix formatting issues noted in review Add 'The' prefix Fix minor last typos
5 participants
This pull request proposes a solution for issue #455.
Issue The original issue reported that the current implementation of the WaterOilGas object did not allow for separate values of KROEND for water/oil and oil/gas systems. This is desirable in cases such as gas condensate systems and when oil moves into a gas cap. The issue includes examples and a figure to illustrate the problem.
Proposed solution The proposed solution is to accept both KROGEND (oil/gas) and KROWEND (water/oil) when creating the three-phase WaterOilGas object (create_water_oil_gas() function), then convert these into two separate KROEND and apply them to the respective WaterOil and OilGas objects. Thus, it does not affect anchoring of KROEND to critical and residual saturations. KROEND for WaterOil anchor to SWL, while KROEND GasOil anchor to SGCR=SGRO.
A test has been added to ensure this feature works.
The Pyscal documentation has been updated to include the modeling of wet gas/condensate systems, demonstrating the use of both KROGEND (gas/oil) and KROWEND (water/oil) in a three-phase field scenario.
Example of implemented solution
Figure created using the pull request version of pyscal (#471) show Kro in oil-water system where KROWEND = 1, Kro in gas-oil system KROGEND = 0.25, and KROMAX = 1
PySCAL format file that produce the plot (uses the base case) is attached:
pySCAL_krow_krog_endpoint_for_new_implementation.csv