Skip to content
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

fixing the parsing of unit with string "Temperature" #4155

Merged
merged 2 commits into from
Aug 1, 2024
Merged

fixing the parsing of unit with string "Temperature" #4155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bc68e4
fixing the parsing of unit with string "Temperature"
GitPaean Jul 31, 2024
5f76ed8
using AbsoluteTemperature for composite dimensions
GitPaean Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions opm/input/eclipse/Units/UnitSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,12 +1536,18 @@ namespace {
for( const auto& x : dimensionList ) {
auto dim = this->getDimension( x );

// all constituing dimension must be compositable. The
// all constituting dimensions must be compositable. The
// only exception is if there is the "composite" dimension
// consists of exactly a single atomic dimension...
if (dimensionList.size() > 1 && !dim.isCompositable())
throw std::invalid_argument("Composite dimensions currently cannot require a conversion offset");

if ( !dim.isCompositable() ) {
if (dimensionList.size() > 1) {
throw std::invalid_argument("Composite dimensions currently cannot handle conversion offsets");
} else {
// If there is only one single dimension, and it's not compositable (involving conversion offsets),
// return it directly
return dim;
}
}
SIfactor *= dim.getSIScaling();
}
return Dimension( SIfactor );
Expand All @@ -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()) {
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

@GitPaean GitPaean Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you confirm this is a correct understanding?

Your understanding is correct.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, merging.

throw std::invalid_argument("Composite dimensions currently cannot handle conversion offsets");
}

return Dimension( dividend.getSIScaling() / divisor.getSIScaling() );
}
Expand Down
4 changes: 2 additions & 2 deletions opm/input/eclipse/share/keywords/900_OPM/G/GASJT
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
{
"name": "JOULE_THOMSON_COEFFICIENT",
"value_type": "DOUBLE",
"dimension": "Temperature/Pressure",
"dimension": "AbsoluteTemperature/Pressure",
"default": 0,
"comment": "if defaulted the JT coefficient is calculated"
"comment": "if defaulted the JT coefficient is calculated. Unit Temperature can not be used due to conversion offset"
}
]
}
4 changes: 2 additions & 2 deletions opm/input/eclipse/share/keywords/900_OPM/O/OILJT
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
{
"name": "JOULE_THOMSON_COEFFICIENT",
"value_type": "DOUBLE",
"dimension": "Temperature/Pressure",
"dimension": "AbsoluteTemperature/Pressure",
"default": 0,
"comment": "if defaulted the JT coefficient is calculated"
"comment": "if defaulted the JT coefficient is calculated. Unit Temperature can not be used due to conversion offset"
}
]
}
3 changes: 2 additions & 1 deletion opm/input/eclipse/share/keywords/900_OPM/T/THERMEXR
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"description": "The THERMEXR item is used to set the thermal expansion ratio for a cell in mechanics models.",
"data": {
"value_type": "DOUBLE",
"dimension": "1/Temperature"
"dimension": "1/AbsoluteTemperature",
"comment": "Unit Temperature can not be used due to conversion offset"
}
}
4 changes: 2 additions & 2 deletions opm/input/eclipse/share/keywords/900_OPM/W/WATJT
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
{
"name": "JOULE_THOMSON_COEFFICIENT",
"value_type": "DOUBLE",
"dimension": "Temperature/Pressure",
"dimension": "AbsoluteTemperature/Pressure",
"default": 0,
"comment": "if defaulted the JT coefficient is calculated"
"comment": "if defaulted the JT coefficient is calculated. Unit Temperature can not be used due to conversion offset"
}
]
}