Many nightly tests fail since October 21

[Rüdiger Franke]

Have a look at:

​https://test.openmodelica.org/libraries/trend.html

Since October 21 the validation of many MSL and MSL_test examples fails with the Cpp runtime.

Moreover: the translation fails for C and Cpp for

PowerSystems.Examples.Spot.GenerationAC3ph.*Generator*

examples.

[Lennart Ochel]

This is related to the changes to the parameter handling. Probably, there is something wrong with the initialization of parameter in the cpp runtime.

[Lennart Ochel]

Rüdiger, can you have a look at the cpp runtime issue? Patrick and I will have a look at the compilation issues for the PowerSystems models.

[Rüdiger Franke]

Well, I picked the NoiseExamples that have a lot of new validation errors. It turns out that the situation got worse with ​1692f27 (BackendDAE.ASSIGN can hold tuples on lhs).

It is good if duplicate function calls are avoided. It is bad the the generated Cpp code does not call the function anymore at all. This needs to be fixed first because it hides the effects from October 21.

See the following example:

model DIDtuple

  function update
    input Real pre_y1;
    input Real pre_y2;
    input Real u;
    input Real samplePeriod;
    output Real y1;
    output Real y2;
  algorithm
    y1 := pre_y1 + u * samplePeriod;
    y2 := pre_y2 + pre_y1 * samplePeriod + 0.5 * u * samplePeriod^2;
  end update;
  
  input Real u(start = -2);
  Real y1(start = 1), y2(start = 0);
  parameter Real samplePeriod = 0.1;
equation
  when sample(0, samplePeriod) then
    (y1, y2) = update(pre(y1), pre(y2), u, samplePeriod);
  end when;
  annotation(experiment(StopTime=1));
end DIDtuple;

[Lennart Ochel]

Just keep in mind that we have also the backend module wrapFunctionCalls to avoid duplicated function calls. Unfortunately, this Is not enabled by default yet.

[Lennart Ochel]

I just started to debug the model GenerationAC3ph.TurbineGenerator which fails while generating the SimCode structure. The issue is, that an assignment for a final parameter is to be generated, which has neither a binding nor an explicitly declared start value.

generator.c.L_s[1]:PARAM(unit = "H" protected = true final = true )  "L matrix stator dq0" type: Real  [3]

[Volker Waurich]

Thanks rfranke for monitoring the testsuite and the issue with my commit. I will fix this.
@ lochel: I dont think that wrapFunctionCalls can avoid duplicated calls in case of an expanded tuple assignment.
(extObjVar, realVar) := someFunctionCall(someInputs); which gets expanded to:
extObjVar := someFunctionCall(someInputs); and
realVar:= someFunctionCall(someInputs);

[Vitalij Ruge]

I guess I inderduce some bugs. By the try to rewritte the model simplifyComplexFuntion.
Can someone(please) revert it around the day? Or I can do it on weekend.

@lochel Can you add please some examples from the pnlib, that we can catch the bug much earlier.

[Lennart Ochel]

Replying to vitalij:

I guess I inderduce some bugs. By the try to rewritte the model simplifyComplexFuntion.
Can someone(please) revert it around the day? Or I can do it on weekend.

@lochel Can you add please some examples from the pnlib, that we can catch the bug much earlier.

This is the wrong ticket, see ​OMCompiler#1152. Anyway, I have opened ​OMCompiler#1165 to revert the changes.

[Lennart Ochel]

I have fixed the generation of parameter equations with ​OMCompiler#1163.

[Volker Waurich]

Replying to rfranke:

Well, I picked the NoiseExamples that have a lot of new validation errors. It turns out that the situation got worse with ​1692f27 (BackendDAE.ASSIGN can hold tuples on lhs).

It is good if duplicate function calls are avoided. It is bad the the generated Cpp code does not call the function anymore at all. This needs to be fixed first because it hides the effects from October 21.

I fixed the cpp codegen and added this model as a test. Are there any other particular models I can check regarding the cpp codegen?

[Rüdiger Franke]

@vwaurich: unfortunately the nightly tests did not improve -- obviously my example didn't cover all problems.

Instead of a comment // nothing to do we get now a wrong call to DiscreteEvents::pre with array argument. See e.g. Modelica.Blocks.Examples.NoiseExamples.Distributions.

Commit ​76cba8f attempts to correct this second issue ... I'd like to add the simulation of the Distributions example to the testsuite, but the reference results are missing ... so let's wait for the next nightly tests.

[Volker Waurich]

@rfranke: I see. Its in msl 3.2.2. Sorry, I havent found the test of the noise model because I was looking into ModelicaTest. I will care about it, if your commit doesnt do the job.

[Rüdiger Franke]

It became even worse tonight. To summarize:

• typeof(var) returns just CREF instead of T_ARRAY.
• SIMVAR's numArrayElement reports a dimension for a scalar time variable (Modelica.Electrical.PowerConverters.Examples.ACDC.Rectifier1Pulse.Thyristor1Pulse_R).
• SIMVAR's arrayCref is NONE() for all but the first scalar array element (PowerSystems.Examples.Spot.AC1ph_DC.Inverter)
• crefIsScalar returns false for a scalar array element

Let's see if ​4a3540 and ​c60d86 will do it.

Is this thing called MineSweeper or OpenModelica?

[Volker Waurich]

It seems the cpp tests are back in place since: ​ea606a1b
This was actually a bug which did not take effect before. Nice to have it fixed.
Whats the status of the ticket now? Anything else that is broken?

[Rüdiger Franke]

The least remaining thing would be to fix the two operator() methods of RefArrayDim3 that take an index vector as well.

Moreover it should be checked if the row major ordering of RefArray introduces more bugs; e.g. during copy to value array or for value references. Alternatively make RefArray column major as well.

[Rüdiger Franke]

From reading the code I get the impression that the intention was to have RefArray column major, like other Cpp arrays. For instance CodegenCpp.tpl calls SimCodeUtil.getVarIndexListByMapping with iColumnMajor=true and a comment says "to conform to the cpp runtime array addressing".

​PR 1193 makes a naive move into the intended direction: it changes all operator() of RefArray to column major. Moreover it removes listReverse at two places in SimCodeUtil.mo -- no idea if this is correct; but the tests pass and world.y_label of the DoublePendulum example contains the correct entries.

[Rüdiger Franke]

Replying to vwaurich:

This was actually a bug which did not take effect before. Nice to have it fixed.
Whats the status of the ticket now? Anything else that is broken?

I'm afraid the bug took effect because the code generation introduces too many RefArray now. Before October 21 the consecutive memory was detected correctly and simple StatArray were used instead.

Meaning that your fix and my pull request address a new issue that showed up, but not the root cause!

[Rüdiger Franke]

Now the nightly tests run again AND there is no known bug with RefArray anymore. An attempt to test RefArray showed that the generated code is very inefficient, i.e. RefArray causes lot of headache during code generation, but is hardly exploited in the code. The follow-up ticket #4129 was opened.