Conversation
atgeirr
added
the
manual:irrelevant
|
jenkins build this please |
bska
left a comment
bska
left a comment
There was a problem hiding this comment.
I'll let others review the technical details concerning the indices here, so I have only minor comments on the implementation of the new willBalanceOnNextIteration() member function.
| // if network is not active, we do not need to balance the network | ||
| const auto& network = well_model_.schedule()[reportStepIdx].network(); | ||
| if (!network.active()) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Intentional or not, the network object isn't use for anything other than this conditional so you might as well write it as
if (! well_model_.schedule()[reportStepIdx].network().active()) {
return false;
}On another though, well_model_.schedule()[reportStepIdx] is accessed repeatedly so we might want to create an alias (reference) for that object.
There was a problem hiding this comment.
Refactored as suggested.
|
The changes to the gaslift part during well testing are fine. Thanks for the fix. |
One of them used to pass iteration + 1 to check if the next iteration would balance the network. Replaced with separately named new method to fix.
atgeirr
force-pushed
the
fix-refactoring-and-glift-bugs
branch
from
b8c1726 to
ca50b2a
Compare
|
jenkins build this please |
bska
left a comment
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 the gas lift aspects have been approved I'll merge into master.
|
Thanks, now we still have the question of changing the behaviour restored in the middle commit: 842e7fc Should it be modified or is it correct as is now? |
3 participants
This addresses three minor bugs (each of these is a separate commit).
The second one is arguably dubious. The change was introduced in #6880, which was intended as a refactoring, and therefore it is a "bug" to change, however the new behaviour is arguably more correct and should not be reverted. I may therefore remove the second commit.
The gas lift change should be verified by someone familiar with that code. @totto82 or @svenn-t perhaps?