Instantiation of arrays with multiple iterators

[carlj <carlj@…>]

The model

model Model1
  Real Z[2];
equation
  Z = array(i + time for i in 1:2);
end Model1;

is instantiated to

class Model1
  Real Z[1];
  Real Z[2];
equation
  Z[1] = 1.0 + time;
  Z[2] = 2.0 + time;
end Model1;

However, the model

model Model1
  Real Z[2,2];
equation
  Z = array(i + time for i in 1:2, j in 1:2);
end Model1;

is instantiated to

class Model1
  Real Z[1,1];
  Real Z[1,2];
  Real Z[2,1];
  Real Z[2,2];
equation
  Z[1,1] = array(/*Real*/(i * j) + time for j in {1, 2}, i in {1, 2})[1, 1];
  Z[1,2] = array(/*Real*/(i * j) + time for j in {1, 2}, i in {1, 2})[1, 2];
  Z[2,1] = array(/*Real*/(i * j) + time for j in {1, 2}, i in {1, 2})[2, 1];
  Z[2,2] = array(/*Real*/(i * j) + time for j in {1, 2}, i in {1, 2})[2, 2];
end Model1;

it would be nice if this model was instantiated in a similar way as the first, without referencing elements in a temporary array.

[Martin Sjölund]

ExpressionSimplify needs to support multiple iterators then

[carlj <carlj@…>]

Note that the equivalent model

model Model1
  Real Z[2,2];
equation
  Z = array(array(i*j + time for i in 1:2) for j in 1:2);
end Model1;

is instantiated to

class Model1
  Real Z[1,1];
  Real Z[1,2];
  Real Z[2,1];
  Real Z[2,2];
equation
  Z[1,1] = 1.0 + time;
  Z[1,2] = 2.0 + time;
  Z[2,1] = 2.0 + time;
  Z[2,2] = 4.0 + time;
end Model1;

[Francesco Casella]

As of v1.13.0-dev-155-g68350e9, the current front-end produces

class M
  Real Z[1,1];
  Real Z[1,2];
  Real Z[2,1];
  Real Z[2,2];
equation
  Z[1,1] = 1.0 + time;
  Z[1,2] = 2.0 + time;
  Z[2,1] = 1.0 + time;
  Z[2,2] = 2.0 + time;
end M;

which I guess is wrong; if i is the index of the first subscript, I would have expected

  Z[1,1] = 1.0 + time;
  Z[1,2] = 1.0 + time;
  Z[2,1] = 2.0 + time;
  Z[2,2] = 2.0 + time;

The new front end fails with

Modelica Assert: NFCall.Call.instArgs got unknown function args!

@perost, can you please fix the order in the current front-end and make sure this works with the new one?

[Per Östlund]

Replying to casella:

@perost, can you please fix the order in the current front-end and make sure this works with the new one?

The current order is correct:

array(i + time for i in 1:2, j in 1:2) =>
array(array(i + time for i in 1:2) for j in 1:2) =>
array({1 + time, 2 + time} for j in 1:2) =>
{{1 + time, 2 + time}, {1 + time, 2 + time}}

So this has been fixed some time ago for the current instantiation, but array construction using iterators hasn't been implemented for the new instantiation yet.

[Francesco Casella]

Flattening with the NF in v1.13.0-dev-798-g1c8bb86de results in

class Test.Model1
  Real Z[1,1];
  Real Z[1,2];
  Real Z[2,1];
  Real Z[2,2];
equation
  Z = array(/*Real*/(i) + time for i in 1:2, j in 1:2);
end Test.Model1;

I guess array() should be expanded by the NF somewhere.

[Francesco Casella]

Rescheduled to 1.14.0 after 1.13.0 releasee

[Francesco Casella]

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2. This issue is rescheduled to 1.16.0

[Francesco Casella]

Still as in comment:6 as of v1.16.0-dev-170-g3af6123f8

[Per Östlund]

Replying to casella:

Still as in comment:6 as of v1.16.0-dev-170-g3af6123f8

I'm assuming this is for performance reasons, or is there any other reason for expanding array constructors in this case?

I got curious about whether it's actually faster to expand array constructors in the frontend or not, it's easy to test since expanding them is trivial in the NF. But it turns out that the model doesn't even compile when not expanding them, it generates invalid C-code.

I'll have to investigate why it doesn't work right now, it would be nice if we could see what the performance implications of expanding them is.

[Francesco Casella]

Replying to perost:

I'm assuming this is for performance reasons, or is there any other reason for expanding array constructors in this case?

Because of compatibility with the current backend, which doesn't handle array(). We had lots of similar cases already, we agreed that for the time being they will be passed to the backend as scalarized flattened equations, unless you set -d=nonfScalarize. In the future, the non-scalarized new backend should handle these cases in a smarter way.

I got curious about whether it's actually faster to expand array constructors in the frontend or not, it's easy to test since expanding them is trivial in the NF. But it turns out that the model doesn't even compile when not expanding them, it generates invalid C-code.

Exactly.

[Per Östlund]

The compilation issues where fixed in 92c05a73.

The issue was that the comment for DAE.RANGE said that the structure contains the array type of the range it represents, like Integer[10] for the range 1:10, which seems the sane thing to do. The old frontend disregarded this and instead created ranges with only the element type, e.g. Integer for 1:10, and the new frontend mimicked this since things would otherwise break.

The result was a lot of confusion that lead to some parts of the compiler assuming that ranges contained the array type while others assumed it contained the element type, which mostly worked with the old frontend since it aggressively tries to expand ranges anyway.

I've now fixed this so that all parts of the compiler agree that ranges contain the array type, which fixes the compilation issue.

As for the performance of expanding array constructors or not I scaled the example model up to 100x100 and simulated it. The times reported are:

Not expanded:
    timeFrontend = 0.020568322,
    timeBackend = 1.740746169,
    timeSimCode = 0.853962535,
    timeTemplates = 0.395255447,
    timeCompile = 7.562222474,
    timeSimulation = 0.22506118,
    timeTotal = 10.797933717

Expanded:
    timeFrontend = 0.08520860200000001,
    timeBackend = 4.302087229,
    timeSimCode = 0.885503033,
    timeTemplates = 0.335738253,
    timeCompile = 7.803955128,
    timeSimulation = 0.23591323,
    timeTotal = 13.64850828

So there doesn't seem to be any benefit to expanding array constructors, since doing so slows down all stages of the compilation in this case.

I'm closing this ticket since the original issue was fixed a long time ago in the old frontend, and the new frontend also seems to handle the models in a satisfactory manner now.