Skip to content

Conversation

@bska
Copy link
Member

@bska bska commented Nov 28, 2025

This commit adds a new UnitSystem::measure for "foam density" quantities such as those entered in the WFOAM keyword. This is mainly intended for client code that wishes to tag output arrays appropriately for unit conversion in restart file output.

While here, also add unit tests for the 'concentration' unit of measure that was introduced in commit d4e33a4 (PR #4749).

This commit adds a new UnitSystem::measure for "foam density"
quantities such as those entered in the WFOAM keyword.  This is
mainly intended for client code that wishes to tag output arrays
appropriately for unit conversion in restart file output.

While here, also add unit tests for the 'concentration' unit of
measure that was introduced in commit d4e33a4 (PR OPM#4741).
@bska bska added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Nov 28, 2025
@bska
Copy link
Member Author

bska commented Nov 28, 2025

jenkins build this please

bska added a commit to bska/opm-simulators that referenced this pull request Nov 28, 2025
The FOAM result array should be treated as a "foamdensity" (PR
OPM/opm-common#4842) instead of a saturation when converted to
output units.
@GitPaean
Copy link
Member

The PR looks good, while not directly related to this PR, from the manual I read, it looks like Density should actually be a concentration. Maybe we should have used FoamConcentration in the first place. I am not opposing this PR, while would like to correct the usage of Density to Concentration at some point.

"GPa",
"KJ/M/DAY/K",
"DAY/SM3",
"KG/SM3",
Copy link
Member

@GitPaean GitPaean Nov 28, 2025

Choose a reason for hiding this comment

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

not all the units have that, but I do think a comment can help the readability and also you are here to adding a new one, since it is a relatively long list now.

"KG/SM3", /* foam density */ 

something like that.

Copy link
Member

@GitPaean GitPaean left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, it looks good to me. A small comment regarding comment, I will leave it up to you whether to add it.

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

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants