Skip to content

water viscosity calculation for brine + thermal #4656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
plgbrts wants to merge 1 commit into
base: master
Choose a base branch
from
Open

water viscosity calculation for brine + thermal #4656

plgbrts wants to merge 1 commit into from

Conversation

@plgbrts
Copy link
Contributor

@plgbrts plgbrts commented Jul 3, 2025 •
edited
Loading

Allow for combination of the BRINE and TEMP 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

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