Matching fails if ASUB-conversion is disabled

[Per Östlund]

In InstSection.instForStatement there's a call to replaceLoopDependentCrefs in the second case, which replaces crefs that depend on for-iterators with ASUB expressions. If this call is commented out, then the matching will fail on the following model (simplified from AlgorithmForInClass test case):

model Test
  Real x[2, 2];
  Real y[2];
algorithm
  for i in 1:2 loop
    for j in 1:2 loop
      y[i + j] := j;
      x[i, j] := i;
    end for;
  end for;
end Test;

This is an issue because it's not actually possible to convert all crefs to ASUBs, e.g. x[i].y can't be represented as an ASUB. So it would be good if the back end could handle these cases, and replaceLoopDependentCrefs removed.

[Volker Waurich]

If y has the size of 2, it might not be possible to adress index 3 and 4. If I set the array size of y to 4, everything is fine.

[Per Östlund]

Replying to vwaurich:

If y has the size of 2, it might not be possible to adress index 3 and 4. If I set the array size of y to 4, everything is fine.

It seems I made a mistake when I simplified the model, y should indeed have size 4. But it makes no difference for me though, the backend still claims the model is structurally singular if the call to replaceLoopDependentCrefs is removed.

[Volker Waurich]

The model is singular because the incidenceMatrix is calculated wrong. I can fix this but then the simulation fails because the generated code is wrong since we need to address an array by its index. Thats why we need the ASUB type instead of a CREF.

Would it be ok to move the function replaceLoopDependentCrefs to the backend? For instance to BackendDAECreate.lower?

[Per Östlund]

Replying to vwaurich:

Would it be ok to move the function replaceLoopDependentCrefs to the backend? For instance to BackendDAECreate.lower?

Well, replaceLoopDependentCrefs is not correct as written right now, since it converts e.g. x[i].y into an ASUB (x.y)[i]. But shouldn't a CREF have the same type as an ASUB?

[Volker Waurich]

Sorry, I expressed myself in a wrong way. The type is of course the same. The question is which DAE.Exp should be used in order to refer to the subscript.

btw. I commited the fix for the incidence matrix.

[Per Östlund]

Replying to vwaurich:

Sorry, I expressed myself in a wrong way. The type is of course the same. The question is which DAE.Exp should be used in order to refer to the subscript.

Well, ASUBs can only really be used for crefs where only the last part of the cref has subscripts, so it can't be used in the general case. I guess we need to generate the correct code for CREFs, which isn't entirely trivial with regards to slicing etc. I'm not sure how much such functionality we already have in the runtime though, if any.

[Mahder Alemseged Gebremedhin]

I have removed replaceLoopDependentCrefs and updated the code generation accordingly. r21592. However this is still not a full proof code generation. I will change most of it again soon.

But as far as cref to ASUB conversion in for loops is concerned (and some more cases for ASUBs) the issue has been fixed. So I am going to close this ticket and continue with the rest of the issues.

ASUBs are confusing :)
Maybe we should find a way to remove ASUBs completely. We probably only need them at instantiation time (I guess. Per?) but they should be removed afterwards. The only place I can think where we need ASUBs is when dealing with {...} array expressions in some places. However, there we always have constant subscripts in the ASUB which means we can actually index out and use the actual expression instead of wrapped ASUB exp. Just wondering. Maybe we need them for something else. Per any ideas?