The NF accepts illegal discrete variables

[casella]

Consider this test model

model M
  discrete Real x;
equation
  if time < 0.5 then
    x = 1;
  else
    x = 0;
  end if;
end M;

OMC accepts this model. However, the Modelica Specification ​Section 4.4.4 states:

If a Real variable is declared with the prefix discrete it must in a simulation model be assigned in a when-clause, either by an assignment or an equation.

So I guess it's pretty clear that model M is not valid. I'm not sure whether this kind of issues should be detected by the NF already, or rather by the backend.

According to the above-mentioned rule, discrete-time Real variables are only expected to be found in the left-hand side of an equation or assigment in a when statement. I guess the frontend can already figure this out, without the need of any sophisticated structural analysis. But maybe it's best to leave this to the backend, I'm not sure.

[perost]

Replying to casella:

According to the above-mentioned rule, discrete-time Real variables are only expected to be found in the left-hand side of an equation or assigment in a when statement.

The NF already keeps track of whether it's inside a when-statement or not when typing equations and algorithm statements, so it could easily check this. However, the rule mentioned specifies a requirement for discrete Real variables, not a restriction. In other words, the rule doesn't forbid using a discrete Real variable in other contexts.

Say you have a model such as this:

model M
  discrete Real x;
equation
  when time > 1 then
    x = 1;
  end when;

  x = 0;
end M;

The model is of course not valid, but the question is what kind of error should be given. It's not very helpful to give an error that x is not assigned in a when-equation in this case, since it is. So, is it actually enough to only check that discrete Real variables are not assigned outside of when-clauses? And if so, what kind of error should be given?

The reason why I bring this up is because checking if a discrete Real variable is assigned outside a when-equation is trivial in the NF, both in implementation and performance. But checking what the rule says, that a discrete Real variable must be assigned in a when-equation, is a different kettle of fish since it involves rounding up all such variables and going through the model until assignments for all of them has been found. It's not particularly hard to implement, though arrays might be a bit tricky, but much more computationally heavy.

[casella]

As I understand, the model above is invalid because there is a Real equation that cannot be matched to a non-discrete Real variable; also, it has two equations and one variable, so it is not balanced.

The former property can be detected by the backend, the latter even by the frontend.

Also, as I understand there are no such things as "assignments outside when clauses" in equation sections - the only case where assignments in equation sections are defined (and in fact compulsory) is within when clauses that determine the values of discrete equations, see ​Section 8.3.5.2.

checking what the rule says, that a discrete Real variable must be assigned in a when-equation, is a different kettle of fish since it involves rounding up all such variables and going through the model until assignments for all of them has been found

Not necessarily, you could collect the discrete variables as you generate the final flattened model, and at the same time collect all the left-hand-side terms in when-equations. When you're finshed with the flattening, check that there is a one-to-one correspondence between the two sets. I guess that's an O(N*log(N)) complexity task, isn't it?

Of course we could also leave this to the backend to check, since the backend will have to match equations and variables anyway.

What do you think?

[casella]

Any conclusion on this ticket. @Karim, @Andreas, based on your recent backend redesign work, what do you think?

[Karim.Abdelhak]

Replying to casella:

Any conclusion on this ticket. @Karim, @Andreas, based on your recent backend redesign work, what do you think?

It should be quite easy to check the matching if any discrete Real variable is matched to something that is not a when-equation or an algorithm. It is linear with respect to the number of variables and the overhead for a single check is fairly small since only has some array lookups and type matching. There might be a problem if the discrete variable is solved in a loop, could that happen?

It might also be weird that a model gets rejected during causalization in the backend for such a basic thing, also CheckModel would not cover it.

All in all it would be possible but does not feel like the correct approach.

In the new backend branch we added input and output information to all algorithms. A related question: Why do we have when-equations? Why don't we treat them as when-algorithms? If they were also based on statements we could use the input output modle (which would also be great for backend) and you just have to check if all discrete Real variables are covered in these inputs of when equations.

[casella]

Replying to Karim.Abdelhak:

It should be quite easy to check the matching if any discrete Real variable is matched to something that is not a when-equation or an algorithm. It is linear with respect to the number of variables and the overhead for a single check is fairly small since only has some array lookups and type matching.

Good.

There might be a problem if the discrete variable is solved in a loop, could that happen?

Good question. There are some restrictions on the structure of when equations, see ​Section 8.3.5.2 of the specification - these restrictions are meant to make it clear which variable should be held constant when the when clause is not active. But as far as I understand, this doesn't prevent implicit equations to be used, e.g.

model foo
  discrete Real v1, v2;
equation 
  when sample(0,1) then
    v1 = 2*v1 + v2 + time;
    v2 = 2*v1 - v2 + time;
  end when;
end foo;

This model is perfectly legal and it runs fine in Dymola. To be honest, it seems to me quite a corner case, I've never seen this pattern in real models so far. So, it would be fine for me if OMC gives an error in this case and says: "Implicit equations involving discrete variables are currently not supported.".

In fact, one may even go further, and write

model bar
  discrete Real v1, v2;
equation 
  when sample(0,1) then
    v1 = 2*v1 + v2 + time;
  end when;
  when sample(0,2) then
    v2 = 2*v1 - v2 + time;
  end when;
end bar;

This is also legal, but it's really an academic corner case in my view. In fact, Dymola says it can't solve it because of implicit equations involving discrete variables. So should we :)

It might also be weird that a model gets rejected during causalization in the backend for such a basic thing, also CheckModel would not cover it.

This is not specific to this issue, but depends in general how far we want to bring the CheckModel analysis. So far, we only check for balancedness, but not for structural issues, so e.g. this models also passes CheckModel

model M
  Real x,y;
equation
  x = time;
  der(x) = x;
end M;

We may add matching and sorting to CheckModel in order to catch this kind of issues earlier. Of course you could only do that for balanced models.

Note that we should be careful when doing this. For example, if you check a current generator model by itself, the model is balanced but the default connection equations actually constrain the current to be zero, so you'd get a failure. This would be confusing, because there is actually nothing wrong at all with that model from a structural point of view. So, if we wanted to do this for real, we shouldn't replace unconnected connectors with flow_var = 0 equations, but rather with f(flow_var, effort_var) = 0 equations.

Summing up, my recommendations are

• check that discrete variables are always matched with when-equations
• allow implicit equations involving discrete variables if this is super-easy to implement, otherwise issue a "not supported" warning when you get strong components involving discrete equations among the unknowns
• (long-term) include structural analysis in checkModel (-> please open a separate ticket if you want to do that)

All in all it would be possible but does not feel like the correct approach.

I hope I convinced you of the contrary. If not, please go ahead with some counterdeductions :)

In the new backend branch we added input and output information to all algorithms. A related question: Why do we have when-equations?

Because of the ​single assignment rule.

Why don't we treat them as when-algorithms?

As I understand it, it should rather be the opposite. When-algorithms (as all algorithms) have to be turned into when-equations before undergoing further structural analysis. Do I miss something?

If they were also based on statements we could use the input output modle (which would also be great for backend) and you just have to check if all discrete Real variables are covered in these inputs of when equations.

I'm not sure I understand what you mean by 'input output modle', could you please elaborate a bit further?

[Karim.Abdelhak]

Replying to casella:

We may add matching and sorting to CheckModel in order to catch this kind of issues earlier. Of course you could only do that for balanced models.

For big models that could lead to quite the overhead. I don't think we should do that...

Summing up, my recommendations are

• check that discrete variables are always matched with when-equations

I can do that.

• allow implicit equations involving discrete variables if this is super-easy to implement, otherwise issue a "not supported" warning when you get strong components involving discrete equations among the unknowns

I got to say that i don't really know how the examples you posted should behave. I will look at it in dymola and see what the expected results look like.

• (long-term) include structural analysis in checkModel (-> please open a separate ticket if you want to do that)

As i said, i am not quite sure if that is good idea. I have to think about it.

All in all it would be possible but does not feel like the correct approach.

I hope I convinced you of the contrary. If not, please go ahead with some counterdeductions :)

Maybe i am getting something wrong, but i will try to explain in the following. :)

Why don't we treat them as when-algorithms?

As I understand it, it should rather be the opposite. When-algorithms (as all algorithms) have to be turned into when-equations before undergoing further structural analysis. Do I miss something?

Well for matching and sorting that kind of is the case, but the point i wanted to get at is as follows: If you have a model like

model minimal
  discrete Real x,y;
initial equation
  x = 0.0;
equation
  when time > 0.5 then
    y = x;
  end when;
  y = 10.0;
end minimal;

It is not valid because the order of the body of the when equation is not correct. It needs to be x = y to be correct. It fails during SimCode-Phase, with

Error: Internal error function createSingleWhenEqnCode failed. When equations currently only supported on form v = ...

which is pretty much what the specification says. So structurally it is and will always be an algorithm because it is based on statements rather then equality. And for algorithms we can strictly define inputs and outputs (before we only did that for outputs but we implemented a new check that also detects inputs on the newBackend branch).

I just think we are missing out on useful information here. Also in this case it would make life much easier because we would only need to check if all discrete Real variables are outputs of such an algorithm or when-algorithm. If any are left undefined we can throw an error. This also is linear in computational complexity, if not better because we kind of collect that information anyway. No real need to drag that to matching because we can deduce that way before.

If they were also based on statements we could use the input output modle (which would also be great for backend) and you just have to check if all discrete Real variables are covered in these inputs of when equations.

I'm not sure I understand what you mean by 'input output modle', could you please elaborate a bit further?

I wanted to refer to the input output detection module for algorithms we added in the newBackend branch. As mentioned that can be used here.

[casella]

Replying to Karim.Abdelhak:

Replying to casella:

We may add matching and sorting to CheckModel in order to catch this kind of issues earlier. Of course you could only do that for balanced models.

For big models that could lead to quite the overhead. I don't think we should do that...

The overhead could be managed by only doing structural check if there are less than, say, 1000 equations. But I agree with you, this is not necessary, or at least it's not our top priority at this moment, I won't waste time on that.

I got to say that i don't really know how the examples you posted should behave. I will look at it in dymola and see what the expected results look like.

Why not? There is a system of two implicit equations in v1 and v2, and the tool solves the system of implicit equations using a linear solver whenever the when statement is activated. That's what Dymola does. See below.

Well for matching and sorting that kind of is the case, but the point i wanted to get at is as follows: If you have a model like

model minimal
  discrete Real x,y;
initial equation
  x = 0.0;
equation
  when time > 0.5 then
    y = x;
  end when;
  y = 10.0;
end minimal;

It is not valid because the order of the body of the when equation is not correct. It needs to be x = y to be correct. It fails during SimCode-Phase, with

Error: Internal error function createSingleWhenEqnCode failed. When equations currently only supported on form v = ...

which is pretty much what the specification says. So structurally it is and will always be an algorithm because it is based on statements rather then equality. And for algorithms we can strictly define inputs and outputs (before we only did that for outputs but we implemented a new check that also detects inputs on the newBackend branch).

I think this is precisely the point where you are making an assumption which is not in the specification of the language. It actually took me several years to figure this out, so you are in good company :)

The specification ​says:

The equations within the when-equation must have one of the following forms:

• v = <expr>
• ...

but doesn't say anything about the content of <expr>, which may as well contain v. Nor it explicitly forbids cyclic dependencies among discrete variable equations.

The rationale of this restriction is by no means to make those equations assignments with fixed causality, or to make them de facto algorithms. This is not written anywhere, and I understand it is also not intented.

The rationale of this structural restriction is instead that when-equations are activated only at event instants, when they are (conceptually) solved together with all the other non-when-equations that are always active (single assignment rule). Otherwise, if a when-equation is not activated, its corresponding discrete variable is kept constant. In practice, you have many blocks in the BLT, some of them matched to discrete variables, some to continuous variables, and you only execute the blocks that are active at the current step; if you end up with blocks matched with mixed continuous and discrete equations, you give up. At least that's what Dymola does, and it's fine, because most of the time the mixed system is the result of having incorrectly omitted some pre() operators.

The question now is is: what is the corresponding variable? This structural restriction makes it clear: it's the one showing up in the left-hand side of the when-equation.

Alles klar?

If they were also based on statements we could use the input output modle (which would also be great for backend) and you just have to check if all discrete Real variables are covered in these inputs of when equations.

I'm not sure I understand what you mean by 'input output modle', could you please elaborate a bit further?

I wanted to refer to the input output detection module for algorithms we added in the newBackend branch. As mentioned that can be used here.

I guess the argument reported above makes this not possible. Do you agree?

[Karim.Abdelhak]

Replying to casella:

For big models that could lead to quite the overhead. I don't think we should do that...

The overhead could be managed by only doing structural check if there are less than, say, 1000 equations. But I agree with you, this is not necessary, or at least it's not our top priority at this moment, I won't waste time on that.

I agree. We can add that in the future.

The specification ​says:

The equations within the when-equation must have one of the following forms:

• v = <expr>
• ...

but doesn't say anything about the content of <expr>, which may as well contain v. Nor it explicitly forbids cyclic dependencies among discrete variable equations.

The rationale of this restriction is by no means to make those equations assignments with fixed causality, or to make them de facto algorithms. This is not written anywhere, and I understand it is also not intented.

The rationale of this structural restriction is instead that when-equations are activated only at event instants, when they are (conceptually) solved together with all the other non-when-equations that are always active (single assignment rule). Otherwise, if a when-equation is not activated, its corresponding discrete variable is kept constant. In practice, you have many blocks in the BLT, some of them matched to discrete variables, some to continuous variables, and you only execute the blocks that are active at the current step; if you end up with blocks matched with mixed continuous and discrete equations, you give up. At least that's what Dymola does, and it's fine, because most of the time the mixed system is the result of having incorrectly omitted some pre() operators.

The question now is is: what is the corresponding variable? This structural restriction makes it clear: it's the one showing up in the left-hand side of the when-equation.

Alles klar?

si, capisco :)

It may be implicit, but it still restricts the variable that has to be solved in that equation. Very good to know actually! We should use that for matching... we are not allowed to match anything else to this equation. It still might end up in a loop though.

With this explanation i also understand how the examples you posted should behave.

I wanted to refer to the input output detection module for algorithms we added in the newBackend branch. As mentioned that can be used here.

I guess the argument reported above makes this not possible. Do you agree?

Well somewhat. We cannot use that module, but since it is definitely clear what has to be matched to that equation we could test that during frontend phase and do not need to wait for matching right? But maybe i am nitpicking here, i see what i can do. :) I think it is not that much work.

[casella]

Replying to Karim.Abdelhak:

Alles klar?

For the record, this was a quote from the song ​Der Kommissar by the great Austrian pop singer Falco. It got quite popular in Italy when I was a kid. Of course we didn't understand a word, except the "Alles klar, Herr Kommissar?" refrain. The song was actually about the Viennese undeground cocaine scene. Interesting lyrics, btw, with lines like

Dieser Fall ist klar, lieber Herr Kommissar
Auch wenn sie anderer Meinung sind
Den Schnee auf dem wir alle talwärts fahren
kennt heute jedes Kind

We had no idea back then, you couldn't Google it up and look for a translation in 1981. But it was cool anyway :)

It may be implicit, but it still restricts the variable that has to be solved in that equation. Very good to know actually! We should use that for matching...

Absolutely.

I wanted to refer to the input output detection module for algorithms we added in the newBackend branch. As mentioned that can be used here.

I guess the argument reported above makes this not possible. Do you agree?

Well somewhat. We cannot use that module, but since it is definitely clear what has to be matched to that equation we could test that during frontend phase and do not need to wait for matching right?

You still need to figure out if the equation ends up in a larger strong component, I guess.

[Karim.Abdelhak]

Replying to casella:

Replying to Karim.Abdelhak:

Alles klar?

For the record, this was a quote from the song ​Der Kommissar by the great Austrian pop singer Falco.

Not quite my time, but i remember him. :)

Well somewhat. We cannot use that module, but since it is definitely clear what has to be matched to that equation we could test that during frontend phase and do not need to wait for matching right?

You still need to figure out if the equation ends up in a larger strong component, I guess.

Why? I thought we concluded that it does not matter if it is solved implicit or not, as long as a when-equation is strictly assigned to each discrete Real variable. I think we should make a short meeting about all of this! (Check mail)

[Karim.Abdelhak]

Fixed with ​PR992. Some cases for nested for-loops and variable indexed discrete Real variables still might pose a problem, but that is an entirely different problem.

The attached model now throws an error:

Error: Following variable is discrete, but does not appear on the LHS of a when-statement: ‘x‘.

Also the modelica compliance coverage has been updated accordingly.

[casella]

Great, thanks!

[casella]

There are still some cases that are not covered correctly. For example, ​Buildings.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.FiniteDifference_1Week fails with

[Buildings 7.0.0/Fluid/Geothermal/Borefields/BaseClasses/HeatTransfer/GroundTemperatureResponse.mo:65:3-66:41:writable] Error: 
Following variable is discrete, but does not appear on the LHS of a when-statement: 
groTemRes.QAggShi_flow[283].

The corresponding source code line is found ​here, and involve a call to a function with multiple outputs

(curCel,QAggShi_flow) = Buildings.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.LoadAggregation.shiftAggregationCells(
      i=i,
      QAgg_flow=QAgg_flow,
      rCel=rCel,
      nu=nu,
      curTim=(time - t_start));

that corresponds to the second case mentioned in the ​specification.

As such it is legal and should be accepted.

[Karim.Abdelhak]

I implemented it, but it is a new frontend module, so i changed the description back!

Seems like tuple management is not correct, i will see what i can do!

[Karim.Abdelhak]

Should be fixed with ​PR6744. Tuple handling was fine actually, but array element types were not parsed correctly.

I will close this ticket once it is pushed.

[Karim.Abdelhak]

The error is fixed, but the mentioned model still fails for other reasons.

We probably need to open another ticket for that.