Inconsistent flattening of for loops in NF with -d=nonfScalarize

[Francesco Casella]

Please consider the following test case

package TestNoScalarize
  model A
    B[100] b;
  equation
    for i in 1:100 loop
      b[i].c[10] = 2;
    end for;
  end A;

  model B
    Real[10] c;
  end B;
end TestNoScalarize;

The output of non-scalarized flattening of A is

class TestNoScalarize.A
  Real[100, 10] b.c;
equation
  for i in 1:100 loop
    b[i].c[10] = 2.0;
  end for;
end TestNoScalarize.A;

For consistency with the declarations, it should rather be

class TestNoScalarize.A
  Real[100, 10] b.c;
equation
  for i in 1:100 loop
    b.c[i,10] = 2.0;
  end for;
end TestNoScalarize.A;

@perost, would it be possible to fix this without too much effort?

Thanks!

[Per Östlund]

It would be quite trivial to move the subscripts to the end of each component reference, but I'm not sure what the consequences will be. For the frontend we can just do it at the end to avoid any issues, but the backend might run into problems if the subscripts are no longer in the correct places.

[Karim Adbdelhak]

It actually is something different i think... right? This is pretending there is only one model b with a matrix c as an attribute. I am not really sure this works out in the long run... at least for records we need to make an exception here because we want to keep the structure intact.

[Francesco Casella]

Replying to perost:

It would be quite trivial to move the subscripts to the end of each component reference, but I'm not sure what the consequences will be. For the frontend we can just do it at the end to avoid any issues, but the backend might run into problems if the subscripts are no longer in the correct places.

Maybe for the time being you could implement this feature under an experimental debug flag, which is normally deactivated? This would allow us to proceed with our work.

More in general, the question is "What is a non-scalarized flattened Modelica model"? The question is not trivial at all.

In fact, the question "What is a (scalarized) flattened Modelica model" is finding an answer within the working group on flat Modelica of the EMPHYSIS project (see ​summary). I understand this is almost finalized. @perost and/or @adrpo and/or @sjoelund.se are involved directly in the project and can comment on that. In that case, it was an explicit requirement that flat Modelica is (or almost is) a strict subset of Modelica, so you don't need a different syntax and parser. This effort already took some time.

Answering the original question is even harder and we sure can't do that in the next few days, but at least we need to start the discussion, and maybe agree on some subsets of all possible models, which are the ones we are currently tackling with our experimental compiler.

More specifically, we understood that, as long as we have (arrays of) models containing (arrays of) Reals, everything can be boiled down to multi-dimensional arrays of Reals, and to array equations handling them.

In the case of records, if you have a.b.c, I understand at most one among a, b, and c can be a record name R, so you can declare the flat model as R a.b.c[x, y, z]. For example, consider this test case

package TestRecordArray
  model A
    B[100] b;
  equation
    for i in 1:10 loop
      b[i].c[10] = Complex(i,0);
    end for;
  end A;

  model B
    Complex[10] c;
  end B;
end TestRecordArray;

Curently, if you flatten TestRecordArray.A with -d=nonfScalarize you get

class TestRecordArray.A
  Real[100, 10] b.c.re "Real part of complex number";
  Real[100, 10] b.c.im "Imaginary part of complex number";
equation
  for i in 1:10 loop
    b[i].c[10] = Complex.'constructor'.fromReal(/*Real*/(i), 0.0);
  end for;
end TestRecordArray.A;

I guess what you should get is instead

operator Record Complex
  Real re;
  Real im;
  ...
end Complex;

class TestRecordArray.A
  Complex[100, 10] b.c;
equation
  for i in 1:10 loop
    b.c[i,10] = Complex.'constructor'.fromReal(/*Real*/(i), 0.0);
  end for;
end TestRecordArray.A;

Then it would be up to the backend to figure out if and how to expand the records to their Real components, or keep them together. We already know how to deal with this configuration in our compiler, so we can support Complex numbers, which are useful for demonstrate big power grid models.

Of course you could have records containing records, but I'm not sure how many libraries around actually need that (I'm aware of none).

[Per Östlund]

Replying to casella:

Maybe for the time being you could implement this feature under an experimental debug flag, which is normally deactivated? This would allow us to proceed with our work.

Sure, I can implement that tomorrow.

In the case of records, if you have a.b.c, I understand at most one among a, b, and c can be a record name R, so you can declare the flat model as R a.b.c[x, y, z].

You can as you mention have records that contain records so any of a, b, and c can be a record in this case.

[Francesco Casella]

Replying to perost:

Replying to casella:

Maybe for the time being you could implement this feature under an experimental debug flag, which is normally deactivated? This would allow us to proceed with our work.

Sure, I can implement that tomorrow.

That would be great, thanks!

[Francesco Casella]

Replying to perost:

Replying to casella:

In the case of records, if you have a.b.c, I understand at most one among a, b, and c can be a record name R, so you can declare the flat model as R a.b.c[x, y, z].

You can as you mention have records that contain records so any of a, b, and c can be a record in this case.

Sure.

The question is, do we really need to support records of records and non-scalarized equations at the same time? I'm not aware of any library that uses that combination. except, perhaps, Buildings - I'm not 100% sure about that.

In that case, we should declare the variables using the enclosing record types, and then use whatever is necessary in the equation section, depending on whether the records are accessed as a whole, or by individual elements. For example, consider this test package

package Test
  record R1
    Real x;
    Real y;
  end R1;

  record R2
    R1 ra;
    R1 rb;
  end R2;

  model A
    B[100] b;
  equation
    for i in 1:100 loop
      b[i].c[10] = R2(R1(1, 0), R1(i,1));
      b[i].c.rb[1] = R1(i,0);
    end for;
  end A;

  model B
    R2 c[10];
  end B;
end Test;

Model Test.A should be flattened into

model A
  R2 b.c[100,10];
equation
    for i in 1:100 loop
      b.c[i,10] = R2(R1(1, 0), R1(i,1));
      b.c.rb[i,1] = R1(i,0);
    end for;
end A;

What do you think?

[Mahder Alemseged Gebremedhin]

Do not forget slices. If both a and b array types and b "has" an a, then b.a[i] and b[i].a can both exist and should be not flattened to the same thing.

Basically every subscript must exist in the final flattened cref if the original had at least one.

record A
  Real[4] a;
end A;

model B
  A[3] b;
equation
  // normal
  for i in 1:4 loop
    b[3].a[i] = 1;
    // b.a[3][i] = {1, 2, 3};
  end for;

  // sliced. 
  for i in 1:4 loop
    b.a[i] = {1, 2, 3};
    // { b[1].a[i], b[2].a[i], b[3].a[i] } = {1, 2, 3}
    // b.a[:][i] = {1, 2, 3};
  end for;

  for i in 1:3 loop
    b[i].a = {1, 2, 3, 4};
    // { b[i].a[1], b[i].a[2], b[i].a[3], b[i].a[3] } = {1, 2, 3, 4}
    // b.a[i][:] = {1, 2, 3};
  end for;
end B;

So the flattened crefs for last two loops should have the missing subs as whole dimension indices.

[Francesco Casella]

Replying to mahge930:

So the flattened crefs for last two loops should have the missing subs as whole dimension indices.

Absolutely! Thanks @mahge930 for pointing this out.

[Per Östlund]

The proposed behaviour is now implemented in 1d285900/OpenModelica, it's enabled by the debug flag combineSubscripts.

[Francesco Casella]

Thanks @perost, we'll try this out and report ASAP

[Francesco Casella]

Retargeted to 1.18.0 because of 1.17.0 timed release.

[Francesco Casella]

Ticket retargeted after milestone closed

[Francesco Casella]

TestNoScalarize.A is now flattened as expected if -d=combineSubscripts is set.

The problem remains open for Test.A. We can continue that discussion in ​#8131 and close this ticket.