Add support for output of relperm to .INIT file with SLGOF input.#4307
Add support for output of relperm to .INIT file with SLGOF input.#4307bska merged 3 commits intoOPM:masterfrom
Conversation
|
jenkins build this please |
|
jenkins build this please |
bska
left a comment
There was a problem hiding this comment.
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.
| // 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)); |
| return fromSGOF(numRows, tolcrit, sgof); | ||
| } | ||
| const auto& slgof = tabMgr.getSlgofTables(); | ||
| if (sgof.size() > 0) { |
There was a problem hiding this comment.
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.
Thanks, good point! Will update.
|
jenkins build this please |
bska
left a comment
There was a problem hiding this comment.
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.