Wrong code generation for *and*

[adrpo]

The condition in the when:

when (index > 0) and (time >= Array[index])
  ...
end when

is translated to:

  tmp1 = Greater(data->localData[0]->integerVars[1] /* index DISCRETE */,((modelica_integer) 0));
  tmp2 = GreaterEq(data->localData[0]->timeValue, (&data->localData[0]->realVars[1] /* Array[1] variable */)[calc_base_index_dims_subs(1, 10, data->localData[0]->integerVars[1] /* index DISCRETE */)]);
  data->localData[0]->booleanVars[1] /* $whenCondition1 DISCRETE */ = (tmp1 && tmp2);

which will generate an error:

assert            | error   | Dimension 1 has bounds 1..10, got array subscript 0
assert            | info    | simulation terminated by an assertion at initialization

The first condition should be checked before the second one!

[sjoelund.se]

The first condition should be checked before the second one!

Actually, no. This is Modelica and there is no short-circuit evaluation unless it is guaranteed the expression won't trigger errors. We've had this same or similar appear in tickets multiple times...

[mahge930]

Does Modelica guarantee short circuit evaluation for when conditions?. If not this is technically a modelling problem. Maybe it is better to fix it in the models.

The reason I am saying is because the changes we make to handle this will probably affect all multi-operand Boolean expressions and not just when equations. Even if we can handle this for when equations specifically now what about if conditions?

If we have to handle short circuit evlauation, there are two options I can see

• We can generate the conditions inline:

data->localData[0]->booleanVars[1] = Greater(data->localData[0]->integerVars[1] ... && ...

and let the C compiler do short circuit evaluation.

This will probably not be possible in general due to how Susan works. It is also a bad idea as it will make things unreadable and hard to debug.

OR

• Maybe we can generate them like this.

data->localData[0]->booleanVars[1] /* $whenCondition1 DISCRETE */ = Greater(data->localData[0]->integerVars[1] /* index DISCRETE */,((modelica_integer) 0));
data->localData[0]->booleanVars[1] /* $whenCondition1 DISCRETE */ = data->localData[0]->booleanVars[1] /* $whenCondition1 DISCRETE */ && GreaterEq(data->localData[0]->timeValue, (&data->localData[0]->realVars[1] /* Array[1] variable */)[calc_base_index_dims_subs(1, 10, data->localData[0]->integerVars[1] /* index DISCRETE */)]);

essentially transforming e = a && b && c into

e = a;
e = e && b;
e = e && c;

This might be possible if we really have to handle this.

[adrpo]

I haven't seen anything in the spec on how logical operators should behave, it just says: logical and. I would handle this as short circuit because it would make evaluation faster if you skip evaluating some part of the condition: when fastCheck() and slowCheck() and slowerCheck()

I would go for this translation:

a = fastCheck();
e = a;
if (e)
{
  b = slowCheck();
  e = e && b;
}
if (e)
{
  c = slowerCheck();
  e = e && c;
}

[adrpo]

@sjoelund, there is even an example in the spec about this: ​https://specification.modelica.org/v3.4/Ch3.html#example-guarding-expressions-against-incorrect-evaluation

[adrpo]

Ok, I read a bit more here:
​https://specification.modelica.org/v3.4/Ch3.html#evaluation-order
So the tool *is free to do* the boolean short circuit but is not mandatory.

[casella]

Modelica is a declarative language, so I guess a compiler is not only free to do or avoid short circuiting, but also to replace a and b with b and a, as they should be equivalent, right?

[anonymous]

Replying to casella:

a and b with b and a, as they should be equivalent, right?

Real x = if a > 0 and log(a) > 0 then a else 0;
Real y = if log(a) > 0 and a > 0 then a else 0;

I would accept that x defined for a =-1 and evaluate y failed

[casella]

​Section 3.3 of the specification seems pretty clear to me and would rule out the initially reported code fragment.

One could write

when (index > 0) and (time >=Array[max(1,index)])
  ...
end when;

to avoid such a problem.