Opened 9 years ago

Last modified 7 years ago

#3464 new defect

Failing variable lookup during array creation

Reported by: Rüdiger Franke Owned by: Marcus Walther
Priority: high Milestone: Future
Component: Cpp Run-time Version:
Keywords: Cc: Niklas Worschech, Marcus Walther

Description (last modified by Rüdiger Franke)

Some models from MSL trunk initialize arrays with expressions like TransformationMatrix:

function ToSpacePhasor
  input Real x[:];
  ...
protected
  parameter Integer m=size(x, 1);
  parameter Modelica.SIunits.Angle phi[m]=MultiPhase.Functions.symmetricOrientation(m);

  parameter Real TransformationMatrix[2, m]=2/m*{+cos(+phi),+sin(+phi)} for k in 1:m};
  ...
end ToSpacePhasor;

The Cpp runtime generates separate createArray functions that don't see the used variables. See e.g. Modelica.Electrical.Machines.Examples.Transformers.TransformerTestbench,

https://test.openmodelica.org/libraries/MSL_trunk_cpp/BuildModelRecursive.html

Shouldn't these functions be removed and the array be created in situ?

Change History (12)

comment:1 by Rüdiger Franke, 9 years ago

Description: modified (diff)

Commit

https://github.com/OpenModelica/OMCompiler/commit/e6cfb0d7575f47ffe80d5524db0bbc722ec3afa8

solves another problem with these examples -- the sizing of the array is wrong and appears not needed there.

comment:2 by Niklas Worschech, 9 years ago

The createArray function is used to separate the array instantiation in sub-functions. Because in some cases the array creation becomes very large and the compiler stops with memory allocation error.

comment:3 by Rüdiger Franke, 9 years ago

How to get forward?

Modelica.Elecrical.Digital.Tables contains some constant arrays that used to be created row by row at simulation time using createArray functions. They should be generated as static C arrays instead -- this might already be the case today, i.e. no createArray functions needed.

For non-constant arrays, like TransformationMatrix above, the separate createArray functions don't work.

Do you have a particular example showing that the separate createArray functions are needed?

in reply to:  3 comment:4 by Niklas Worschech, 9 years ago

Replying to rfranke:

How to get forward?

Modelica.Elecrical.Digital.Tables contains some constant arrays that used to be created row by row at simulation time using createArray functions. They should be generated as static C arrays instead -- this might already be the case today, i.e. no createArray functions needed.

Do you have an example modell for this problem?

Do you have a particular example showing that the separate createArray functions are needed?

We have some special hydraulic models which uses such large arrays but we dont need it at he momement. We can switch to the old array creation. If we have the problem again, I will think about it to solve the problem.

comment:5 by Rüdiger Franke, 9 years ago

The most extreme example I'm aware of is Modelica.Media.Examples.R134a.R134a1.

The constructor of Functions creates seven large arrays on the stack (OMC_LIT17_data to OMC_LIT_24_data) when building the respective OMC_LIT arrays. The *_data could be static.

Really bad are the uses of createArray in the property functions, like dewEnthalpy, bubbleEnthalpy and so on. Each call creates again and again multiple StatArrayDim2<double, 477, 4> by creating and appending 477 StatArrayDim1<double, 4> and then just picks out few elements. This can't be solved in the code generation alone, but at the end the literal arrays should only be created once and the data could be declared statically as one block.
_

comment:6 by Marcus Walther, 9 years ago

I just want to note that we cannot easily convert these variables to member-variables of the Functions-class, because this will lead to race conditions in the parallel code. There is only on instance of the function-class at the moment and if 2 threads will use the same function simultaneously, they will overwrite the temp variables of the other thread during calculation. The same issue occurs if we declare these arrays as "static", of course.

comment:7 by Rüdiger Franke, 9 years ago

The parallel code should not be affected because I'm speaking of constant Modelica arrays whose values are only read. In order to avoid misunderstandings, one could also declare them const in C++.

The arrays are currently created with:

  double arr1_data[] = {1,2,3,4};
  StatArrayDim1<double,4> arr1(arr1_data); 
  double arr2_data[] = {5,6,7,8};
  StatArrayDim1<double,4> arr2(arr2_data);
  StatArrayDim2<double,2,4> arr;
  arr.append(arr1, 1);
  arr.append(arr2, 2);

This should read (at least):

  static double arr_data[] = {1,2,3,4, 5,6,7,8};
  StatArrayDim2<double,2,4> arr;
  assignRowMajorData(arr_data, arr);

One can imagine a further improvement with:

  static double arr_data[] = {1,5, 2,6, 3,7, 4,8};
  StatArrayDim2<double,2,4, true> arr(arr_data);
Last edited 9 years ago by Rüdiger Franke (previous) (diff)

comment:8 by Martin Sjölund, 9 years ago

Milestone: 1.9.41.9.5

Milestone pushed to 1.9.5

comment:9 by Martin Sjölund, 9 years ago

Milestone: 1.9.51.10.0

Milestone renamed

comment:10 by Martin Sjölund, 8 years ago

Milestone: 1.10.01.11.0

Ticket retargeted after milestone closed

comment:11 by Martin Sjölund, 8 years ago

Milestone: 1.11.01.12.0

Milestone moved to 1.12.0 due to 1.11.0 already being released.

comment:12 by Francesco Casella, 7 years ago

Milestone: 1.12.0Future

The milestone of this ticket has been reassigned to "Future".

If you think the issue is still valid and relevant for you, please select milestone 1.13.0 for back-end, code generation and run-time issues, or 2.0.0 for front-end issues.

If you are aware that the problem is no longer present, please select the milestone corresponding to the version of OMC you used to check that, and set the status to "worksforme".

In both cases, a short informative comment would be welcome.

Note: See TracTickets for help on using tickets.