some tidying up in compostional - #5351
Conversation
GitPaean
added
the
manual:irrelevant
|
jenkins build this please |
There was a problem hiding this comment.
🔵 Needs a closer look
It touches core compositional viscosity/flash numerical code paths where small refactoring differences can have outsized simulation impact and should receive final human review/validation.
Pull request overview
This PR performs targeted cleanup in the compositional (EOS/flash/viscosity) code paths by replacing hard-coded “magic numbers” with named constants and by factoring repeated correlation logic into helpers, aiming to make numerical intent and units clearer without changing the underlying algorithms.
Changes:
- Refactors the LBC viscosity implementations to use explicit unit constants and shared helper functions for the Stiel–Thodos dilute-gas branch and the LBC polynomial evaluation.
- Replaces several hard-coded tolerances/clamps in PT flash and cubic EOS code with named
static constexprconstants. - Small test tidy-up: switches component names to
std::string_viewin the volume-shift test fixture.
File summaries
| File | Description |
|---|---|
| tests/material/test_volume_shift.cpp | Uses std::string_view for component names in test data setup. |
| opm/material/viscositymodels/ViscosityModels.hpp | Refactors viscosity correlations, adds unit constants usage, and introduces helper functions/constants. |
| opm/material/eos/CubicEOS.hpp | Names and centralizes fugacity/molar-volume clamp constants. |
| opm/material/constraintsolvers/PTFlash.hpp | Names and centralizes solver tolerances and Wilson slope constant. |
| opm/input/eclipse/EclipseState/Compositional/CompositionalConfig.cpp | Names default EOS omega constants for readability/maintainability. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
It looks like some racing issue in the jenkins failures. |
The flash tolerances, the Wilson slope, the fugacity clamps, the molar volume floor, the EOS omega defaults and the unit conversions of the LBC correlation were inline literals. Each now has a name and a line saying what it is for. The arithmetic is untouched, so results are unchanged. The test's component name is a string_view.
Reuse existing unit definitions, document correlation unit conversions, and consolidate repeated calculations while retaining the original variable names.
|
jenkins build this please |
I suppose you could call it a race condition. To me it looks like the underlying cause is slightly different though. I suspect that the reference solution changes brought by #5348 (i.e., OPM/opm-tests#1595) ended up undoing/reverting the reference solution changes introduced by #5346 (i.e., OPM/opm-tests#1594), possibly due to a stale GitHub commit graph when merging #5348. I'll trigger another reference solution update, this time from #5334, to try to re-introduce the changes from #5346. Please hold data updates for the time being. |
I just merged OPM/opm-tests#1596. You can try to rerun your build check here whenever you're ready. |
|
jenkins build this please |
|
I need to work on it some more before it is ready for review. |
|
I think it is mostly fine now. I am marking it as ready for review. |
|
jenkins build this please |
some cleaning up partly following #5322 (review) , please let me know if it is something welcomed.