Skip to content

Add Conditional Low-Pressure Padding of Gas Property Tables#3779

Merged
bska merged 2 commits intoOPM:masterfrom
bska:low-pressure-gas-table-padding
Feb 14, 2025
Merged

Add Conditional Low-Pressure Padding of Gas Property Tables#3779
bska merged 2 commits intoOPM:masterfrom
bska:low-pressure-gas-table-padding

Conversation

@bska
Copy link
Member

@bska bska commented Nov 17, 2023

This PR adds logic to extend gas property tables entered using the PVDG or PVTG keywords to low pressure values if needed. When we compute well production potentials (e.g., summary vectors WxPP) and the well's bottom-hole pressure limit is defaulted we end up using pressures in the order of one atmosphere (1 atm) and this, in turn, may generate negative formation volume factors (mass densities) and/or phase viscosities. The problem is most pronounced for producing wells in history matching mode (keyword WCONHIST), but we should have guards in place to avoid negative densities regardless.

If needed we extend the tables down to 1 barsa and also insert a "limiting" pressure value $p_{\mathrm{lim}}$ corresponding to a maximum FVF value in the range $[1, 10\cdot B_0]$. In this case we create new input tables and redo the property table initialisation with this extended input table.

To this end, we add a way of creating empty DeckItem and DeckKeyword objects which preserve the structural elements of their live counterparts. This, in turn, enables programmatic creation of full DeckItem/DeckKeyword objects by first inserting the padding values and then copying the original table values into those objects.

@bska
Copy link
Member Author

bska commented Nov 17, 2023

I am creating this PR in draft mode because it needs quite a lot of testing before we consider enabling this in the mainline simulator.

@bska
Copy link
Member Author

bska commented Nov 17, 2023

jenkins build this please

@bska bska force-pushed the low-pressure-gas-table-padding branch 2 times, most recently from 678035b to b0772f0 Compare November 21, 2023 13:07
@bska
Copy link
Member Author

bska commented Nov 21, 2023

jenkins build this please

@bska bska force-pushed the low-pressure-gas-table-padding branch from b0772f0 to 0615b69 Compare November 22, 2023 12:18
@bska
Copy link
Member Author

bska commented Nov 22, 2023

jenkins build this please

@bska bska force-pushed the low-pressure-gas-table-padding branch 9 times, most recently from 8bcb7d7 to 99e8833 Compare November 28, 2023 10:42
@bska
Copy link
Member Author

bska commented Nov 28, 2023

jenkins build this please

@bska bska force-pushed the low-pressure-gas-table-padding branch 10 times, most recently from 1647b1d to bca7723 Compare December 7, 2023 08:18
@bska bska force-pushed the low-pressure-gas-table-padding branch 3 times, most recently from b4fb4f9 to 2194a00 Compare December 12, 2023 09:03
@akva2
Copy link
Member

akva2 commented Feb 8, 2024

sure i get that, but i figured the cosmetic stuff is still warranted. in fact the two first commits could probably be extracted and done now.

@bska bska force-pushed the low-pressure-gas-table-padding branch 2 times, most recently from 4eb0e02 to 09bdbab Compare February 19, 2024 08:12
@bska
Copy link
Member Author

bska commented Nov 15, 2024

jenkins build this failure_report please

@bska
Copy link
Member Author

bska commented Nov 15, 2024

I have downloaded the failure report locally and will analyse the failures when I have the time.

@akva2
Copy link
Member

akva2 commented Jan 30, 2025

maybe separate out the two first commits here? in particular the first one is a fix after all.

@bska
Copy link
Member Author

bska commented Jan 30, 2025

maybe separate out the two first commits here? in particular the first one is a fix after all.

Yep, agree. That part has lingered for long enough.

@tskille
Copy link
Contributor

tskille commented Feb 11, 2025

This PR really helped respect to robustness for a full field model in Equinor. The asset using this model has already implemented OPM Flow. Therefor I would very much appreciate if this PR could be merged.

Let me know if there is anything that I can do to assist.

@bska
Copy link
Member Author

bska commented Feb 11, 2025

Therefor I would very much appreciate if this PR could be merged.

Thank you for testing. The only thing that's been holding this PR back is lack of testing on real field cases. With your finding in hand, I can justify marking this as "ready for review" now. I need to do a couple of smaller things to have the tests pass, but I'll do those forthwith.

Copy link
Member

@akva2 akva2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me


ret.m_recordList.clear();

return ret;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this keeps m_location, intentional? i reckon it is but just to confirm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this keeps m_location, intentional?

It is indeed intentional. I want any diagnostic message to refer to the location at which the input table is entered.

/// Whether or not input table needs padding at low pressures
bool inputNeedsPadding() const { return this->needPadding_; }

/// Low pressure padding rows for input table. Not needed unless
Copy link
Member

@akva2 akva2 Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho it is better with Needed only if inputNeedsPadding() returns true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed only if inputNeedsPadding() returns true

Fair enough. I've pushed an update to rephrase this.

Copy link
Member Author

@bska bska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for looking at this. I've pushed an update to address your comments.


ret.m_recordList.clear();

return ret;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this keeps m_location, intentional?

It is indeed intentional. I want any diagnostic message to refer to the location at which the input table is entered.

/// Whether or not input table needs padding at low pressures
bool inputNeedsPadding() const { return this->needPadding_; }

/// Low pressure padding rows for input table. Not needed unless
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed only if inputNeedsPadding() returns true

Fair enough. I've pushed an update to rephrase this.

@bska
Copy link
Member Author

bska commented Feb 11, 2025

jenkins build this failure_report please

@bska
Copy link
Member Author

bska commented Feb 11, 2025

benchmark please

@bska
Copy link
Member Author

bska commented Feb 12, 2025

build this failure_report

I have reviewed the regression failures and they're mostly all of the expected form; the contents of the .INIT file's TAB vector changes due to low-pressure table padding. There are some smaller changes to certain XGRP elements–relative changes in the order of 5.0e-5 at most–which I have not drilled into. Other than that, the main difference is that this PR as a side effect appears to make the dew-point calculation more robust. At least we're getting more non-zero values in the PDEW per-cell result array in the restart file now than we used to. That said, the dew-point calculation should probably be revisited regardless of this work.

I recommend updating the reference solutions and merging this, but we should maybe/probably wait for the benchmark results before doing so?

@bska
Copy link
Member Author

bska commented Feb 13, 2025

benchmark please

I issued that request nearly 48 hours ago now. Is there a high system load on the benchmark runner, @blattms, or did I make a mistake somewhere?

@akva2
Copy link
Member

akva2 commented Feb 13, 2025

last time i triggered it took 50h.

@bska
Copy link
Member Author

bska commented Feb 13, 2025

last time i triggered it took 50h.

Okay. If that's the case now too then we'll have the response around 8pm CET today.

@bska
Copy link
Member Author

bska commented Feb 14, 2025

last time i triggered it took 50h.

Okay. If that's the case now too then we'll have the response around 8pm CET today.

Still no results. I'm tempted to just update the reference solutions now and merge this. Any objections, @akva2, @atgeirr, @steink?

@akva2
Copy link
Member

akva2 commented Feb 14, 2025

no objections.

@bska
Copy link
Member Author

bska commented Feb 14, 2025

jenkins build this update_data please

@bska
Copy link
Member Author

bska commented Feb 14, 2025

jenkins build this opm-tests=1292 please

bska added 2 commits February 14, 2025 15:30
This commit adds a new member function,

    X emptyStructuralCopy() const

to the DeckKeyword and DeckItem classes.  This member function will
form a "structural" copy of "*this", thus preserving aspects such as
the currently active "Dimensions", but removing all data values.
This is in preparation of adding low-pressure table expansion for
the gas PVT tables (i.e., keywords 'PVDG' and 'PVTG').
This commit adds logic to extend gas property tables entered using
the PVDG or PVTG to low pressure values if needed.  When we compute
well production potentials (e.g., summary vectors WxPP) and the
well's bottom-hole pressure limit is defaulted we end up using
pressures in the order of one atmosphere (1 atm) and this, in turn,
may generate negative formation volume factors (mass densities)
and/or phase viscosities.  The problem is most pronounced for
producing wells in history matching mode (keyword WCONHIST), but we
should have guards in place to avoid negative densities regardless.

If needed we extend the tables down to 1 barsa and also insert a
"limiting" pressure value PLim corresponding to a maximum FVF value
in the range [1, 2].  In this case we create new input tables and
redo the property table initialisation with this extended input table.
@bska
Copy link
Member Author

bska commented Feb 14, 2025

jenkins build this opm-tests=1292 please

@bska
Copy link
Member Author

bska commented Feb 14, 2025

PR approved, build check is green, and the new reference solutions have been installed on the CI system. I'll merge this into the master branch now to enable the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants