Conversation
|
jenkins build this failure_report please |
There was a problem hiding this comment.
This looks a little unexpected to me. Are you sure that you want this change? Please note that enableSatScaling() is true if any kind of end-point scaling is active in the run–e.g., the ENDSCALE keyword is active or any of the horizontal scaling vectors like SWL, SGCR, or SGU are present.
On the other hand, enablePcScaling() and enableKrwScaling()/enableKrnScaling() return true only if the corresponding vertical scaling keywords–PCW/PCG, KRW/KRG, or KRGR/KRWR–are present in the run. That's a very different condition.
There was a problem hiding this comment.
No, this is not what I want. I have a case with Leverett Scaling that's causing me some trouble, and this PR is supposed to fix it. But I still have some work to make sure it doesn't mess up the other cases.
There was a problem hiding this comment.
I still have some work to make sure it doesn't mess up the other cases.
Right. But then please note that unless you intend to fully change the meaning of enablePcScaling() &c, you can't use that predicate family to determine whether or not horizontal (i.e., saturation) end-point scaling is active.
|
jenkins build this failure_report please |
1 similar comment
|
jenkins build this failure_report please |
|
A proper fix has been made in #4923, but I'm keeping this open as it seems unnecessary to scale horizontally if only vertical scaling is needed. I will keep in in draft as some more work is needed in order to make sure horizontalscaling is activated for all input |
|
@bska What do you think? Could it be worthwhile to activate horizontal scaling only when necessary to gain some speedup? ENDSCALE + SWATINIG is a common setup that only requires vertical scaling. If we want to conditionally activate horizontal scaling, we need to check whether any horizontal scaling inputs are present. |
Maybe. We need to measure in order to assess the potential gains from that change. |
2 participants
No description provided.