Skip to content

water viscosity calculation for brine + thermal#4656

Open
plgbrts wants to merge 1 commit intoOPM:masterfrom
plgbrts:brine-thermal
Open

water viscosity calculation for brine + thermal#4656
plgbrts wants to merge 1 commit intoOPM:masterfrom
plgbrts:brine-thermal

Conversation

@plgbrts
Copy link
Contributor

@plgbrts plgbrts commented Jul 3, 2025 •
edited
Loading

Allow for combination of the BRINE and THERMAL and adaptation for the pressure, temperature dependent water viscosity calculation for this combination.

@plgbrts
Copy link
Contributor Author

plgbrts commented Jul 3, 2025

jenkins build this please

@plgbrts plgbrts marked this pull request as ready for review July 9, 2025 12:33
@plgbrts
Copy link
Contributor Author

plgbrts commented Jul 9, 2025

Note: The functionality BRINE plus TEMP works for the standalone executable flow_brine_energy. For flow to deal with this combination an opm-simulators PR is in preparation.

@plgbrts plgbrts mentioned this pull request Jul 9, 2025
Open
@arturcastiel
Copy link
Member

arturcastiel commented Aug 18, 2025

@plgbrts are we ready to ask for a review?

@arturcastiel
Copy link
Member

arturcastiel commented Aug 18, 2025

@bska could you please help @plgbrts to add a label to this PR? @plgbrts is this a new feature?

@GitPaean GitPaean added the manual:new-feature This is a new feature and should be described in the manual label Sep 3, 2025
@GitPaean
Copy link
Member

GitPaean commented Sep 3, 2025 •
edited
Loading

do we have a test case for it? If yes, please add it to the opm-tests to demonstrate the usage. The demonstration case can also help the PR review.

@totto82 totto82 self-requested a review September 19, 2025 06:36
totto82
totto82 approved these changes Sep 19, 2025
View reviewed changes
Copy link
Member

@totto82 totto82 left a comment

Choose a reason for hiding this comment

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

Thanks. The code looks good. Sorry for the late review. As @GitPaean also points out, I miss a small test case for regression testing. I will approve this PR, but not the one in simulator, until we have a regression test.

@arturcastiel
Copy link
Member

arturcastiel commented Sep 25, 2025

so has this been approved for merging?

@arturcastiel
Copy link
Member

arturcastiel commented Sep 25, 2025

@bska , should this be merged?

@arturcastiel
Copy link
Member

arturcastiel commented Sep 25, 2025

Thanks. The code looks good. Sorry for the late review. As @GitPaean also points out, I miss a small test case for regression testing. I will approve this PR, but not the one in simulator, until we have a regression test.

thank you for the review, but is out of office now.

@GitPaean
Copy link
Member

GitPaean commented Sep 26, 2025

Thanks. The code looks good. Sorry for the late review. As @GitPaean also points out, I miss a small test case for regression testing. I will approve this PR, but not the one in simulator, until we have a regression test.

thank you for the review, but is out of office now.

I think we should wait to have a case to run and test.

@arturcastiel
Copy link
Member

arturcastiel commented Oct 21, 2025

@GitPaean ,@plgbrts is back. do you think this can be merged now?

@GitPaean
Copy link
Member

GitPaean commented Oct 21, 2025

@GitPaean ,@plgbrts is back. do you think this can be merged now?

Hei, thanks for the notification. We would like to have a test to demonstrate/protect this development.

@plgbrts plgbrts mentioned this pull request Nov 19, 2025
Merged
totto82
totto82 requested changes Nov 20, 2025
View reviewed changes
opm/material/fluidsystems/blackoilpvt/WaterPvtThermal.cpp
}
for (unsigned regionIdx = 0; regionIdx < regions; ++ regionIdx) {
referenceSaltConcentration_[regionIdx] = pvtwSaltTables[regionIdx].getReferenceSaltConcentrationValue();
}
Copy link
Member

@totto82 totto82 Nov 20, 2025

Choose a reason for hiding this comment

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

You will need to set the viscosity and viscosibility also for PVTWSALT

Copy link
Contributor Author

@plgbrts plgbrts Jan 6, 2026

Choose a reason for hiding this comment

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

This indeed solved the assertion failure I got after a rebase. Thank for pointing out this.

@plgbrts
Copy link
Contributor Author

plgbrts commented Jan 6, 2026

With the change of the option TEMP (#4825) the flow_brine_energy simulator is for the combination of the keywords BRINE and THERMAL (and not for the combination BRINE and TEMP)

@plgbrts
Copy link
Contributor Author

plgbrts commented Jan 6, 2026

jenkins build this please

@plgbrts plgbrts mentioned this pull request Jan 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@totto82 totto82 totto82 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

manual:new-feature This is a new feature and should be described in the manual

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@plgbrts @arturcastiel @GitPaean @totto82