fixing the parsing of unit with string "Temperature" #4155
Conversation
GitPaean
force-pushed
the
fixing_temperature_unit_parsing
branch
from
c516e93 to
08bc432
Compare
|
jenkins build this please |
or other dimenions that involve conversion offsset, make sure the offset will not be discarded.
|
Indeed there is some keywords using composite dimensions with Temperature, like the following, |
|
For this cases, they using as dT/dP, so the offset can be ignored. But the PR is trying to fix the situation that 25C will be converted to 25K. I think the solution will be to use Absolute temperature for |
GitPaean
force-pushed
the
fixing_temperature_unit_parsing
branch
from
08bc432 to
5f76ed8
Compare
|
jenkins build this please |
| @@ -1560,8 +1566,9 @@ namespace { | |||
| Dimension dividend = this->parseFactor( parts[0] ); | |||
| Dimension divisor = this->parseFactor( parts[1] ); | |||
|
|
|||
| if (dividend.getSIOffset() != 0.0 || divisor.getSIOffset() != 0.0) | |||
| throw std::invalid_argument("Composite dimensions cannot currently require a conversion offset"); | |||
| if (!dividend.isCompositable() || !divisor.isCompositable()) { | |||
There was a problem hiding this comment.
I just understood why this is necessary: If any factor of either dividend or divisor is not compositable, that would usually trigger an exception from parseFactor(), but the exception is if either dividend or divisor is a single incompositable factor, then it must be handled here.
Can you confirm this is a correct understanding?
There was a problem hiding this comment.
dividend and divisor themselves can be possibly a single atomic unit with conversion offset. In this PR, this unit can be Temperature.
Function paserFactor can parse Temperature while not Temperature*Pressure (throw).
And we do not allow either dividend or divisor contain conversion offset (like Temperature. ) before we think it is possible.
There was a problem hiding this comment.
Can you confirm this is a correct understanding?
Your understanding is correct.
make sure the offset will not be discarded.
it was spotted when trying to review OPM/opm-simulators#5490 .