Opened 8 years ago

Closed 8 years ago

#4093 closed defect (fixed)

Many nightly tests fail since October 21

Reported by: Rüdiger Franke Owned by: Lennart Ochel
Priority: blocker Milestone: 1.11.0
Component: Backend Version:
Keywords: Cc: Patrick Täuber, Volker Waurich, Vitalij Ruge, Niklas Worschech

Description

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.

Change History (19)

comment:1 by Lennart Ochel, 8 years ago

Cc: Patrick Täuber added

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

comment:2 by Lennart Ochel, 8 years ago

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.

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

Cc: Volker Waurich Vitalij Ruge added

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;
  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;
Version 0, edited 8 years ago by Rüdiger Franke (next)

comment:4 by Lennart Ochel, 8 years ago

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.

comment:5 by Lennart Ochel, 8 years ago

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]

comment:6 by Volker Waurich, 8 years ago

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);

comment:7 by Vitalij Ruge, 8 years ago

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.

in reply to:  7 comment:8 by Lennart Ochel, 8 years ago

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.

Last edited 8 years ago by Lennart Ochel (previous) (diff)

comment:9 by Lennart Ochel, 8 years ago

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

in reply to:  3 comment:10 by Volker Waurich, 8 years ago

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?

comment:11 by Rüdiger Franke, 8 years ago

@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.

comment:12 by Volker Waurich, 8 years ago

@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.

comment:13 by Rüdiger Franke, 8 years ago

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?

comment:14 by Volker Waurich, 8 years ago

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?

comment:15 by Rüdiger Franke, 8 years ago

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.

comment:16 by Rüdiger Franke, 8 years ago

Cc: Niklas Worschech added

comment:17 by Rüdiger Franke, 8 years ago

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.

in reply to:  14 comment:18 by Rüdiger Franke, 8 years ago

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!

comment:19 by Rüdiger Franke, 8 years ago

Resolution: fixed
Status: newclosed

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.

Note: See TracTickets for help on using tickets.