Inline condition replicates on all three branches

[ke-yu]

Purpose

This PR is to make inline condition replicate on all three branches (Defect MAGN-3979 Ternary operator should act on each item of a List).

That is, for an inline condition, it will evaluate its condition, true and false branches firstly, and replicate over all three branches. Previously, inline condition only replicates on its condition. For example, for the following code,

x = {1, 2, 3};
y = {4, 5, 6};
z = {7, 8, 9};
r = x > 1 ? y : z;

Previously r = x > 1 ? y : z only replicates on condition x > 1, therefore r is {z, y, y}, that is {{7, 8, 9}, {4, 5, 6}, {4, 5, 6}}. Now as it replicates on all three branches, r is {7, 5, 6}:

{1>1  2>1  3>1}
  ?    ?    ?
{ 4    5    6}
  :    :    :
{ 7    8    9}

Evaluating both true and false branches might have unexpected result in some cases (but it wouldn't be a problem in Dynamo because Dynamo will evaluate each node that put on the canvas immediately in Run Automatically mode):

• Recursive function doesn't work in inline condition, it will cause an infinite loop. For example:

    def fact(x)
    {
        return x > 1 ? x * fact(x - 1) : 1;  // infinite loop
    }

• Global value may be changed in unexpected way

    a = 0;
    def update_a() 
    {
        a = a + 1; 
        return = a;
    }
    r = false ? update_a() : update_a();    // r = 2, a = 2;

Declarations

• The code base is in a better state after this PR
• The level of testing this PR includes is appropriate

Reviewers

• @lukechurch
• @junmendoza

FYIs

@kronz , @mccrone As IF node is compiled to inline condition, its output will be changed.

[mccrone]

Thank you, @ke-yu! Great explanation, too. I understand that the If node with change. I think that's good news, because it will mean that we can finally use conditional logic easily without mapping. But as this will break lots of existing code, perhaps this means we need a version of an If node that doesn't replicate over all the three branches. For replication and some other uses, we need something like a switch that is identical to how the If node previously worked, that is, something that passed through all of the data from ifTrue and ifFalse en masse. Could we call this Switch? CC: @kronz @lukechurch

[andydandy74]

Please don't call it Switch. Most every programming language has a switch command and it's never a simple if/else.

[mccrone]

Good point. I was thinking "switch" in the circuit sense. Any suggestions?

[andydandy74]

Not really, just to not call it Switch - but I am sure one of you native speakers will come with something catchy... ;-)

[kronz]

@mccrone can you illustrate a situation where behavior will be broken for users with this new behavior and make a separate tasks to make the node to address it?

[ke-yu]

@mccrone , if we need to keep the old behavior of IF node, shall we keep the old behavior of inline condition and add a new operator / node? CC @lukechurch

[mccrone]

@ke-yu @lukechurch There must be reasons to also keep a version of IF with the old behavior, where it replicated only on the condition. Consider the case of needing to choose among sets of data that are not guaranteed to have the same structure, like passing on matrix A with sets of 3-points for triangular panels or matrix B with sets of 4-points for rectangular panels.

The new behavior seems right to me for the IF node. The old behavior is useful too, but for the use cases I can imagine, it seems arbitrary to limit ourselves to two choices. Perhaps the old behavior of the IF node should be the basis for a SwitchCase node.

Input:
  switch =  2
  case0  =  {1,2,3}
  case1  =  {30,40,50,60}
  case2  =  {A,B,C,D,E,F,G}

Output:
  result  =  {A,B,C,D,E,F,G}

Rather than the condition being limited ot the binary true/false, we could keep everybody happy and make the language significantly more flexible. The number of inputs (cases) could be user-determined like the Python or List.Combine nodes.

*mad MS Paint skillz

[andydandy74]

If you're doing a Switch/Case node you should definitely include a default case as well in case there is no matching case for the test value. Actually that was one of my earliest feature requests: #290 ... ;-)

But I think being able to use the IF node replication-free so you can test quickly against a single boolean is still crucial. We regularly work with boolean toggles to make decisions on how a graph is supposed to be run. For example: Imagine a workflow where I can choose to work with a list of elements that have either been filtered by some device or with the original unfiltered list of elements. That's a use case that regularly comes up in model checking workflows (e.g. process all wall types vs. process only exterior or interior wall types).
My question would be: Would there really be a need for another separate IF node for that scenario? If the new replication-savvy IF node only receives a single boolean shouldn't it just be able to return the entire list of elements fed into the val_if_true / val_if_false inputs?

[ke-yu]

@andydandy74 , unfortunately, if new IF node receives a single boolean, and true/false branches are list, the boolean value will be promoted to a list, so neither shortest, longest nor cross product lacing will return one of the entire list.

[lukechurch]

I think the right thing to do here is to bring this in, as it's clearly an improvement - I think we will need to add a new node that does something like 'ifBranched" which has a signature closer to if(cond : bool, x : var[]..[], y : var[]..[]) - @ke-yu could you look at putting something together along these lines.

If nodes for controlling how the branches should be run really need imperative functionality, they are using control flow not data flow.