-
Notifications
You must be signed in to change notification settings - Fork 112
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.
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
Add support for output of relperm to .INIT file with SLGOF input. #4307
Conversation
jenkins build this please |
jenkins build this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good for the most part, although I'd normally prefer checking !TableContainer::empty()
instead of TableContainer::size() > 0
. That said, there may be one incorrect condition here, so please check carefully where I've posed I direct question. We also need to understand why so many regression tests fail as a result of this PR.
opm/output/eclipse/Tables.cpp
Outdated
// auto So = std::vector<double>{}; | ||
// So.reserve(numActRows); | ||
|
||
// // Two-phase system => So = 1-Sg | ||
// std::transform(std::begin(Sg), std::end(Sg), | ||
// std::back_inserter(So), | ||
// [](const double sg) -> double | ||
// { | ||
// return 1.0 - sg; | ||
// }); | ||
|
||
// std::copy(So.rbegin(), So.rend(), | ||
// linTable.column(tableID, primID, 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could remove this block?
opm/output/eclipse/Tables.cpp
Outdated
return fromSGOF(numRows, tolcrit, sgof); | ||
} | ||
const auto& slgof = tabMgr.getSlgofTables(); | ||
if (sgof.size() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should check slgof.size()
instead of sgof.size()
, although
if (!slgof.empty()) {
...
}
might be more expressive. Uses TableContainer::empty()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good point! Will update.
jenkins build this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the updates. This looks good to me now and as I finally understand how you fixed the initial regression test failures I'll merge into master. We should look into the equivalence predicate for joint saturations at a later date.
Thanks! I think it would be a good idea to use a non-zero default tolerance to mergeTables and update the test results, as there are currently lots of nearly-duplicate saturation values in the .INIT output. I'll create a PR and start looking at the test results.. |
Sounds good. For this purpose, a threshold of |
Thanks, trying with 1e-6: #4313 |
No description provided.