Wrong behaviour of backend in algorithm section

[Francesco Casella]

Please consider the following model

model foo
   discrete Real data[2];
algorithm
   when sample(0,0.1) then
     data[1] := time;
   end when;
end foo;

The backend issues the following error:

[1] 23:59:23 Translation Error
Internal error IndexReduction.pantelidesIndexReduction failed!
Found empty set of continues equations. 
Use -d=bltdump to get more information.

The fact that the model doesn't contain continuous equations (wrong spelling, BTW) doesn't mean that the model is invalid, it just only contains discrete variables. I'm not aware of any restrictions prohibiting this particular case.

If I add one dummy continuous equation

model bar
   discrete Real data[2];
   Real x = time;
algorithm

   when sample(0,0.1) then
     data[1] := time;
   end when;
end bar;

I get

[1] 00:02:03 Symbolic Error
An independent subset of the model has imbalanced number of equations (2) and variables (1).
variables:
data[1]
equations:
1 : algorithm
  when sample(1, 0.0, 0.1) then
    data[1] := time;
  end when;

As I understand, this is also wrong. ​Section 11.1.2 of the Modelica Specification states:

Whenever an algorithm section is invoked, all variables appearing on the left hand side of the assignment operator ”:=” are initialized (at least conceptually)

• A discrete variable v is initialized with pre(v).
• If at least one element of an array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section.

Hence, the entire data array should be initialized to pre(v). Then, the fact that there is no explicit assignment to data[2] means that this variable will remain constant, but the model is otherwise balanced and well-posed.

[Karim Adbdelhak]

I worked out a solution for this issue. All discrete output variables will be matchable for algorithms, but i ran into exactly the problems we talked about at the dev meeting. Models like Modelica.Electrical.Digital.Examples.DFFREG run into circular dependencies because of that. My solution was to assume that a discrete-only algorithm-only strong component can be split up again and use the original matching, because this loop is artificial anyways. Let's see what the testsuite will say about this.

​PR482

[Martin Sjölund]

How can you get a circular dependency? The algorithm block depends on pre(v) which is not calculated in the block.

[Karim Adbdelhak]

Replying to sjoelund.se:

How can you get a circular dependency? The algorithm block depends on pre(v) which is not calculated in the block.

Consider following model:

model foo2
   discrete Real data[2];
algorithm
   when sample(0,0.1) then
     data[1] := time;
   end when;
algorithm
   when sample(0,0.2) then
     data[2] := time;
   end when;
end foo2;

The first algorithm should only compute data[1], but due to the specification data[2] has to also be initialized here. If you consider the first model foo in the description, it becomes clear that data[2] has to be matched to that algorithm if the second algorithm did not exist. In this new case data[1] has to be matched to the first and data[2] to the second algorithm. Since they (seemingly due to the specification forcing it in) can both be solved in the other algorithm this is an algebraic loop. Algebraic loops of purely discrete variables are not (reliably) solvable by iterative solvers, so it fails.

We did not allow the matching of variables that do not occur in the algorithms so this did not happen, but to make models like foo work we have to change it. Also the x := pre(x) equations get generated just before SimCode, i guess it was implemented this way to avoid these artificial algebraic loops.

Anyway my fix does not seem to work, the events of the when-equation seem to be missed during runtime, i don't know why jet.

[Martin Sjölund]

That's an illegal model I guess, as the specification says:

model Test // wrong Modelica model (has 4 equations for 2 unknowns)
  Real x[2](start={-11, -22});
algorithm // conceptually: x = {1,-22}
  x[1] := 1;
algorithm // conceptually: x = {-11,2}
  x[2] := 2;
end Test;

[Karim Adbdelhak]

Replying to sjoelund.se:

That's an illegal model I guess, as the specification says:

model Test // wrong Modelica model (has 4 equations for 2 unknowns)
  Real x[2](start={-11, -22});
algorithm // conceptually: x = {1,-22}
  x[1] := 1;
algorithm // conceptually: x = {-11,2}
  x[2] := 2;
end Test;

Instantiating Modelica.Electrical.Digital.Examples.DFFREG yields these two equations:

algorithm
  when {initial(), (dFFREG.delay.inertialDelaySensitive[1].tLH > 0.0 or dFFREG.delay.inertialDelaySensitive[1].tHL > 0.0) and change(dFFREG.delay.inertialDelaySensitive[1].x) and not initial()} then
    dFFREG.delay.inertialDelaySensitive[1].y_old := if initial() or pre(dFFREG.delay.inertialDelaySensitive[1].y) == Modelica.Electrical.Digital.Interfaces.Logic.'U' then dFFREG.delay.inertialDelaySensitive[1].y0 else pre(dFFREG.delay.inertialDelaySensitive[1].y);
    dFFREG.delay.inertialDelaySensitive[1].lh := {{0, 0, -1, 1, 0, 0, -1, 1, 0}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {1, 1, 0, 1, 1, 1, 0, 1, 1}, {-1, -1, -1, 0, -1, -1, -1, 0, -1}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {1, 1, 0, 1, 1, 1, 0, 1, 1}, {-1, -1, -1, 0, -1, -1, -1, 0, -1}, {0, 0, -1, 1, 0, 0, -1, 1, 0}}[dFFREG.delay.inertialDelaySensitive[1].y_old, dFFREG.delay.inertialDelaySensitive[1].x];
    dFFREG.delay.inertialDelaySensitive[1].delayTime := if dFFREG.delay.inertialDelaySensitive[1].lh > 0 then dFFREG.delay.inertialDelaySensitive[1].tLH else if dFFREG.delay.inertialDelaySensitive[1].lh < 0 then dFFREG.delay.inertialDelaySensitive[1].tHL else 0.0;
    dFFREG.delay.inertialDelaySensitive[1].t_next := time + dFFREG.delay.inertialDelaySensitive[1].delayTime;
    if dFFREG.delay.inertialDelaySensitive[1].lh == 0 or abs(dFFREG.delay.inertialDelaySensitive[1].delayTime) < 1e-60 then
      dFFREG.delay.inertialDelaySensitive[1].y_auxiliary := dFFREG.delay.inertialDelaySensitive[1].x;
    end if;
  elsewhen time >= dFFREG.delay.inertialDelaySensitive[1].t_next then
    dFFREG.delay.inertialDelaySensitive[1].y_auxiliary := dFFREG.delay.inertialDelaySensitive[1].x;
  end when;
  dFFREG.delay.inertialDelaySensitive[1].y := if dFFREG.delay.inertialDelaySensitive[1].tLH > 0.0 or dFFREG.delay.inertialDelaySensitive[1].tHL > 0.0 then dFFREG.delay.inertialDelaySensitive[1].y_auxiliary else dFFREG.delay.inertialDelaySensitive[1].x;
algorithm
  when {initial(), (dFFREG.delay.inertialDelaySensitive[2].tLH > 0.0 or dFFREG.delay.inertialDelaySensitive[2].tHL > 0.0) and change(dFFREG.delay.inertialDelaySensitive[2].x) and not initial()} then
    dFFREG.delay.inertialDelaySensitive[2].y_old := if initial() or pre(dFFREG.delay.inertialDelaySensitive[2].y) == Modelica.Electrical.Digital.Interfaces.Logic.'U' then dFFREG.delay.inertialDelaySensitive[2].y0 else pre(dFFREG.delay.inertialDelaySensitive[2].y);
    dFFREG.delay.inertialDelaySensitive[2].lh := {{0, 0, -1, 1, 0, 0, -1, 1, 0}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {1, 1, 0, 1, 1, 1, 0, 1, 1}, {-1, -1, -1, 0, -1, -1, -1, 0, -1}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {0, 0, -1, 1, 0, 0, -1, 1, 0}, {1, 1, 0, 1, 1, 1, 0, 1, 1}, {-1, -1, -1, 0, -1, -1, -1, 0, -1}, {0, 0, -1, 1, 0, 0, -1, 1, 0}}[dFFREG.delay.inertialDelaySensitive[2].y_old, dFFREG.delay.inertialDelaySensitive[2].x];
    dFFREG.delay.inertialDelaySensitive[2].delayTime := if dFFREG.delay.inertialDelaySensitive[2].lh > 0 then dFFREG.delay.inertialDelaySensitive[2].tLH else if dFFREG.delay.inertialDelaySensitive[2].lh < 0 then dFFREG.delay.inertialDelaySensitive[2].tHL else 0.0;
    dFFREG.delay.inertialDelaySensitive[2].t_next := time + dFFREG.delay.inertialDelaySensitive[2].delayTime;
    if dFFREG.delay.inertialDelaySensitive[2].lh == 0 or abs(dFFREG.delay.inertialDelaySensitive[2].delayTime) < 1e-60 then
      dFFREG.delay.inertialDelaySensitive[2].y_auxiliary := dFFREG.delay.inertialDelaySensitive[2].x;
    end if;
  elsewhen time >= dFFREG.delay.inertialDelaySensitive[2].t_next then
    dFFREG.delay.inertialDelaySensitive[2].y_auxiliary := dFFREG.delay.inertialDelaySensitive[2].x;
  end when;
  dFFREG.delay.inertialDelaySensitive[2].y := if dFFREG.delay.inertialDelaySensitive[2].tLH > 0.0 or dFFREG.delay.inertialDelaySensitive[2].tHL > 0.0 then dFFREG.delay.inertialDelaySensitive[2].y_auxiliary else dFFREG.delay.inertialDelaySensitive[2].x;

They seem to somewhat be like the simplified version i described. Or does initial() make a difference?

[Karim Adbdelhak]

Ah i see, originally those algorithms are one block in Modelica.Electrical.Digital.Delay.InertialDelaySensitive. So this should not be split up?

algorithm
  when {initial(),(tLH > 0 or tHL > 0) and change(x) and not initial()} then
    y_old := if initial() or pre(y) == L.'U' then y0 else pre(y);
    lh := delayTable[y_old, x];
    delayTime := if (lh > 0) then tLH else (if (lh < 0) then tHL else 0);
    t_next := time + delayTime;
    if (lh == 0 or abs(delayTime) < Modelica.Constants.small) then
      y_auxiliary := x;
    end if;
 elsewhen time >= t_next then
    y_auxiliary := x;
 end when;

[Karim Adbdelhak]

@per

Please consider the simplified model from comment:3. As martin explained it is not valid due to the specification, but the frontend seems to split up the algorithm in my comment:6 and produces two algorithms like those in comment:5. This seems to be incorrect, could you have a look at that?

[Per Östlund]

Replying to Karim.Abdelhak:

@per

Please consider the simplified model from comment:3. As martin explained it is not valid due to the specification, but the frontend seems to split up the algorithm in my comment:6 and produces two algorithms like those in comment:5. This seems to be incorrect, could you have a look at that?

It's not really that it's been split up, but that there's an array of IntertialDelaySensitive instances that each contain their own algorithm section.

[Karim Adbdelhak]

Replying to perost:

It's not really that it's been split up, but that there's an array of IntertialDelaySensitive instances that each contain their own algorithm section.

So the rule from the specification "If at least one element of an array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section.", should only apply to the bottom most level?

[Karim Adbdelhak]

Thank you @per ... i just did not see that. :)

The approach only considering last subscripts worked for Modelica.Electrical.Digital.Examples.DFFREG, hopefully also for the others.

[Per Östlund]

Replying to Karim.Abdelhak:

Replying to perost:

It's not really that it's been split up, but that there's an array of IntertialDelaySensitive instances that each contain their own algorithm section.

So the rule from the specification "If at least one element of an array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section.", should only apply to the bottom most level?

That's how I would interpret it at least. If you have multiple instances of a model that contains an array then each instance has its own array, they're not combined into one large array (except if you use -d=-nfScalarize, but that's not really standard Modelica).

[Karim Adbdelhak]

Everything besides the model from #2452 works. I guess we should strip all other subscripts if they are iterators of a for loop... Or we should strip all subscripts that cannot be fully evaluated to be a number.

[Mahder Alemseged Gebremedhin]

Replying to Karim.Abdelhak:

Thank you @per ... i just did not see that. :)

The approach only considering last subscripts worked for Modelica.Electrical.Digital.Examples.DFFREG, hopefully also for the others.

@Karim I think you should not consider just the last subscript. The reason is that you have arrays of records. What you have to consider is all subscripts that did NOT come from a model instance.
The test case that was removed actually shows that :)

model bug_2452_2
  record R
    Integer d;
  end R;
  R r[2];
algorithm 
  r[1].d := 0;
end bug_2452_2;

is a balanced model I think.

considering only the last subscripts for processing is one of the reasons we have problems with records and arrays.

[Karim Adbdelhak]

Replying to mahge930:

Replying to Karim.Abdelhak:

Thank you @per ... i just did not see that. :)

The approach only considering last subscripts worked for Modelica.Electrical.Digital.Examples.DFFREG, hopefully also for the others.

@Karim I think you should not consider just the last subscript. The reason is that you have arrays of records. What you have to consider is all subscripts that did NOT come from a model instance.
The test case that was removed actually shows that :)

model bug_2452_2
  record R
    Integer d;
  end R;
  R r[2];
algorithm 
  r[1].d := 0;
end bug_2452_2;

is a balanced model I think.

considering only the last subscripts for processing is one of the reasons we have problems with records and arrays.

I understand, so we have to make special cases for records? If a part of a record is computed inside an algorithm the whole record should be initialized there? Or at least the discrete part of it?

[Karim Adbdelhak]

I just want to fully understand what is going on. :D This should be valid:

model A
  record R
    Integer d;
  end R;
  R r[2];
algorithm 
  r[1].d := 0;
end A;

and this should not be valid:

model B
  model M
   Integer d;
  end M;
  M m[2];
algorithm 
  m[1].d := 0;
end B;

Is that correct?

[Mahder Alemseged Gebremedhin]

Replying to Karim.Abdelhak:

I just want to fully understand what is going on. :D This should be valid:

model A
  record R
    Integer d;
  end R;
  R r[2];
algorithm 
  r[1].d := 0;
end A;

and this should not be valid:

model B
  model M
   Integer d;
  end M;
  M m[2];
algorithm 
  m[1].d := 0;
end B;

Is that correct?

I think so. The way I understand it the second model is invalid. But the main reason is that model M is not balanced. If model M was balanced then making an array of it should not cause unbalance. For example this is model:

model M
  Real d;
algorithm
  d := 0;
end M;

model K
  M r[2];
end K;

Gets flattened to this

model K
  Real r[1].d;  
  Real r[2].d;
algorithm
  r[1].d := 0;
algorithm
  r[2].d := 0;  
end K;

which is balanced.

But this model

record R
  Real d;
end R;

model K
  R r[2];
algorithm
  r[1].d := 0;
algorithm
  r[2].d := 0; 
end K;

is over-determined EVEN though it gets flattened to

model K
  Real r[1].d;  
  Real r[2].d;
algorithm
  r[1].d := 0;
algorithm
  r[2].d := 0;  
end K;

which is similar to the one above.
The difference is that, now the two algorithm sections did not come from an array of components.

It's an odd thing :). And I do not even know how it should work with partial models ...

I just wanted to remind you that you should not rely on last subscripts usually. They do not account for arrays of records. Most usage of last subscripts in the compiler is assuming that all other subscripts of a qualified cref came from arrays of models.

If you need to ignore subscripts from a qualified cref, usually you should ignore all subscripts where the cref type is a complex type model.

There is some code in CheckModel.mo that accounts for this. I will check again and let you know.

[Mahder Alemseged Gebremedhin]

BTW do not take my word for this. It is just my interpretation. Maybe it is clarified more in the current specification.

[Karim Adbdelhak]

Replying to mahge930:

I just wanted to remind you that you should not rely on last subscripts usually. They do not account for arrays of records. Most usage of last subscripts in the compiler is assuming that all other subscripts of a qualified cref came from arrays of models.

If you need to ignore subscripts from a qualified cref, usually you should ignore all subscripts where the cref type is a complex type model.

Yea maybe, that sounds quite good. I will look at that.

There is some code in CheckModel.mo that accounts for this. I will check again and let you know.

Yea i updated that part to remove the last subscript and all other subscripts that are a cref. So numbers won't get removed.

[Karim Adbdelhak]

Replying to mahge930:

BTW do not take my word for this. It is just my interpretation. Maybe it is clarified more in the current specification.

Unfortunately it isn't, the definition states:
If at least one element of an array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section

And the provided example is the one that martin already posted. From this definition i would say that m[1].d should also initialize all other parts of the m array. That would make models like Modelica.Electrical.Digital.Examples.DFFREG invalid of course.

Maybe this needs to be updated to:
If at least one element of a non-model-array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section.

[Mahder Alemseged Gebremedhin]

Ya I think it intended to be considered in a model by model basis. If a model is balanced then an array of the model is balanced. So the first model in my example is valid but the second is not.

The tricky part is to tell them apart after flattening. I think looking at parts of a qualified cref that are not of model type should work.

[Karim Adbdelhak]

Replying to mahge930:

Ya I think it intended to be considered in a model by model basis. If a model is balanced then an array of the model is balanced. So the first model in my example is valid but the second is not.

The tricky part is to tell them apart after flattening. I think looking at parts of a qualified cref that are not of model type should work.

I will try to update that in CheckModel.mo

[Mahder Alemseged Gebremedhin]

Replying to Karim.Abdelhak:

Replying to mahge930:

I just wanted to remind you that you should not rely on last subscripts usually. They do not account for arrays of records. Most usage of last subscripts in the compiler is assuming that all other subscripts of a qualified cref came from arrays of models.

If you need to ignore subscripts from a qualified cref, usually you should ignore all subscripts where the cref type is a complex type model.

Yea maybe, that sounds quite good. I will look at that.

There is some code in CheckModel.mo that accounts for this. I will check again and let you know.

Yea i updated that part to remove the last subscript and all other subscripts that are a cref. So numbers won't get removed.

Ohh I see. I just hate that function crefStripLastSubs. I wish I could remove it but of course 24734 unrelated models will fail :)

Let me know if you need help with it. I wrote some of it a while back.

[Francesco Casella]

We agreed that the sanity check for "questionable" user input, setting StateSelect.prefer where this is really not needed, would be nice to have, but it's not top priority now.

We'll keep the MSL example as it is so we have it as a test case for this feature.

Tentatively rescheduling to 2.0.0

[Karim Adbdelhak]

Replying to casella:

We agreed that the sanity check for "questionable" user input, setting StateSelect.prefer where this is really not needed, would be nice to have, but it's not top priority now.

We'll keep the MSL example as it is so we have it as a test case for this feature.

Tentatively rescheduling to 2.0.0

This is the wrong ticket? I guess you wanted to change #5459

[Francesco Casella]

Replying to Karim.Abdelhak:

This is the wrong ticket? I guess you wanted to change #5459

Of course :)

[Francesco Casella]

Rescheduled to 1.16.0 (i.e. master, as soon as Adrian branches 1.14.0 and 1.15.0)

[Karim Adbdelhak]

Replying to casella:

Rescheduled to 1.16.0 (i.e. master, as soon as Adrian branches 1.14.0 and 1.15.0)

This is actually already fixed, i am just pushing some last changes for future stuff regarding records and arrays. I will close this ticket as soon as i am done with it.

[Karim Adbdelhak]

Replying to mahge930:

Ohh I see. I just hate that function crefStripLastSubs. I wish I could remove it but of course 24734 unrelated models will fail :)

I made the function crefStripSubsExceptModelSubs and replaced crefStripLastSubs. Maybe we could use it at the other places too?

[Francesco Casella]

Replying to Karim.Abdelhak:

Replying to casella:
This is actually already fixed

Good, I'm marking this for 1.14.0 then.

I am just pushing some last changes for future stuff regarding records and arrays. I will close this ticket as soon as i am done with it.

OK. When you push on master please check if in the meantime @adrpo has already branched off 1.14.0 and 1.15.0 maintenance. In that case, please push the last changes also to those branches.

[Karim Adbdelhak]

Replying to casella:

OK. When you push on master please check if in the meantime @adrpo has already branched off 1.14.0 and 1.15.0 maintenance. In that case, please push the last changes also to those branches.

It's already pushed by adrian himself. :)

[Mahder Alemseged Gebremedhin]

Replying to Karim.Abdelhak:

Replying to mahge930:

Ohh I see. I just hate that function crefStripLastSubs. I wish I could remove it but of course 24734 unrelated models will fail :)

I made the function crefStripSubsExceptModelSubs and replaced crefStripLastSubs. Maybe we could use it at the other places too?

Oh nice. Ya we can change it in other places as well. But if you do it try to do it a few changes at a time. Otherwise you will spend days trying to find out why seemingly unrelated models are failing. I will try removing some as well.