[NF] Implement synchronous features in the new front-end

[Adrian Pop]

Add support for Clock and Modelica 3.3 synch. features.

[Francesco Casella]

Maybe we could split this into

• implement clocked variables
• implement state machines

[Adrian Pop]

WIP: ​https://github.com/OpenModelica/OMCompiler/pull/2664

[Adrian Pop]

Partial support added in 09518b/OMCompiler.

[Francesco Casella]

From the ​Modelica_Synchronous Hudson tests we have 14/96 models currently simulating.

Many models fail with this error:

[OpenModelica/OMCompiler/build/lib/omlibrary/Modelica_Synchronous 0.92.1/BooleanSignals.mo:60:7-60:26:writable]
Error: Type mismatch for positional argument 1 in sample(start=sample.u).
The argument has type:
  Boolean
expected type:
  Real

Other typical issues include

[OpenModelica/OMCompiler/build/lib/omlibrary/Modelica_Synchronous 0.92.1/BooleanSignals.mo:12:7-12:20:writable]
Error: Function parameter interval was not given by the function call,
and does not have a default value.

and

[OpenModelica/OMCompiler/build/lib/omlibrary/Modelica_Synchronous 0.92.1/IntegerSignals.mo:62:7-62:26:writable]
Error: Function argument start=sample1.u in call to sample has variability
discrete which is not a parameter expression.

[Adrian Pop]

Yep, I'm working on that, sample needs overloading for sample(u, clock).

[Adrian Pop]

PR with interval, firstTick and overload of sample is running now:
​https://github.com/OpenModelica/OMCompiler/pull/2665

[Adrian Pop]

Ouch, seems I broke some stuff +54 & -680 :)
​https://libraries.openmodelica.org/branches/history/newInst/2018-09-18%2004:47:02..2018-09-18%2016:00:04.html
I'll see what the problem is.

[Francesco Casella]

Models with clock variables work better, but all other models using sample() with old-fashioned discrete variables got broken.

[OpenModelica/OMCompiler/build/lib/omc/NFModelicaBuiltin.mo:544:5-544:28:writable]
Error: Function Clock not found in scope sample.

[Adrian Pop]

Yes. I wanted to overload sample into one with clock and one without clock (the default one) but that is a bit problematic because if you don't have --std=3.3 on, there is no Clock defined and that is exactly what the error message says.

[Francesco Casella]

How is this solved by the old front-end? Can we use the same strategy?

[Adrian Pop]

Of course. In the current front-end is basically implemented manually. There is a function declared in ModelicaBuiltin.mo but is not used at all as is not overloaded.

I already have a fix for this, will push it in soon.

[Francesco Casella]

OK, thanks!

[Adrian Pop]

PR ​https://github.com/OpenModelica/OMCompiler/pull/2666 should (hopefully) fix the failing 680 models.

[Adrian Pop]

Very strange that none of the tests in our testsuite caught this issue.
I'll try to add one so we don't run into this again.

[Francesco Casella]

Replying to adrpo:

Very strange that none of the tests in our testsuite caught this issue.
I'll try to add one so we don't run into this again.

I guess the testsuite is not using -d=newInst anywhere, so this kind of things happens quite often.

However, in some cases regressions with NF commits are fine, as long as the model was not already simulating with correct results, so I don't think the strict enforcement of the no-regression rule to accept PRs (that we have for normal commits to master) would be reasonable.

I think manual checking of the latest ​report is just fine, until we switch to -d=newInst by default. Then, the standard rules will apply.

In any case, I'm keeping those reports under very close watch :)

[Adrian Pop]

Of course, my bad, this only happened when -d=newInst is on.
PR: ​https://github.com/OpenModelica/OpenModelica-testsuite/pull/1037
should test this now.

[Rüdiger Franke]

The following example would work for firstTick(u). Using firstTick():

model m2
  input Real[10] u;
  Real[size(u, 1)] x1(each start = 1);
equation
  when Clock() then
    x1 = if firstTick() then previous(x1) else previous(x1) + u;
  end when;
end m2;

it produces the error:

[/home/rfranke/OpenModelica/OMCompiler/Compiler/NFFrontEnd/NFCeval.mo: 1447:9-1448:51]:
Internal error NFCeval.evalBuiltinCall: unimplemented case for firstTick

[Adrian Pop]

Did you take the latest sources or the nightly build? The last commit should have fixed this issue.

[Adrian Pop]

Ok. Now I get:

adrpo33@ida-0030 MINGW64 /c/home/adrpo33/dev/OMTesting/nf
$ time ~/dev/OpenModelica/build/bin/omc +d=newInst m2.mo
Error processing file: m2.mo
[C:/home/adrpo33/dev/OMTesting/nf/m2.mo:6:5-6:64:writable] Error: Internal error NFTyping.evaluateCondition failed to evaluate condition `firstTick()`

It seems it badly wants to evaluate the call and conditions to if equations and expressions.
I disabled the ceval of impure functions and your error went away, but it seems this is something else.

[Rüdiger Franke]

I just had updated to the latest version and arrived at the same error. Interestingly it still works for firstTick(u), but not for firstTick(). Independent of the argument passed to firstTick, the result should always be the same Boolean.

[Adrian Pop]

I will make functions with no arguments that are impure have Variability.CONTINUOUOS to prevent evaluation. That is the actual problem, it thinks firstTick() is constant :)

[Adrian Pop]

Works now:

adrpo33@ida-0030 MINGW64 /c/home/adrpo33/dev/OMTesting/nf
$ time ~/dev/OpenModelica/build/bin/omc +d=newInst,-nfScalarize m2.mo

class m2
  input Real[10] u;
  Real[10] x1;
equation
  when Clock() then
    x1 = if firstTick() then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]};
  end when;
end m2;

[Adrian Pop]

Strange that it does expansion of the else part. I'll have a look on disabling that.

[Adrian Pop]

Most of the stuff implemented in:
​https://github.com/OpenModelica/OMCompiler/pull/2667
Most of Modelica_Synchronous works since this morning:
​https://libraries.openmodelica.org/branches/history/newInst/2018-09-19%2003:24:28..2018-09-19%2006:44:44.html

Fix for impure functions firstTick() is running now:
​https://github.com/OpenModelica/OMCompiler/pull/2668

[Adrian Pop]

Per told me -d=-nfExpandOperations should disable the expansion of else part and it does:

class m2
  input Real[10] u;
  Real[10] x1;
equation
  when Clock() then
    x1 = if firstTick() then previous(x1) else previous(x1) + u;
  end when;
end m2;

I will gather all the extra flags we need to disable expansion and scalarization under -nfScalarize so we don't need to set each flag individually.

[Adrian Pop]

After PR: ​https://github.com/OpenModelica/OMCompiler/pull/2669 only -d=-nfScalarize is needed.

[Adrian Pop]

This looks better now:
​https://libraries.openmodelica.org/branches/newInst/Modelica_Synchronous/Modelica_Synchronous.html
​https://libraries.openmodelica.org/branches/newInst/Modelica_Synchronous_cpp/Modelica_Synchronous_cpp.html
but it seems some models that work in the current front-end fail with the NF in the backend, for example:
​https://libraries.openmodelica.org/branches/newInst/Modelica_Synchronous/files/Modelica_Synchronous_Modelica_Synchronous.Examples.Elementary.ClockSignals.ShiftSample.err
It dies in clockPartitioning. I guess somebody with back-end knowledge should have a look. I will check the differences when flattening, maybe is still a NF issue.

[Rüdiger Franke]

I had a look in the backend. A lot of things work for a simple model. I had to adapt the equation count and moreover disabled detection of strong components, common subexpression elimination, dependency analysis and simcode generation.

The major remaining problem is that all code generation (XML files, variable counts, variable references, start values) assume scalar variables only.

The above example nicely demonstrates the advantage -nfScalarize. With scalarization the array state equation is evaluated n times, i.e. effort of n*n:

class m2
  input Real u[1];
  input Real u[2];
  input Real u[3];
  input Real u[4];
  input Real u[5];
  input Real u[6];
  input Real u[7];
  input Real u[8];
  input Real u[9];
  input Real u[10];
  Real x1[1](start = 1.0);
  Real x1[2](start = 1.0);
  Real x1[3](start = 1.0);
  Real x1[4](start = 1.0);
  Real x1[5](start = 1.0);
  Real x1[6](start = 1.0);
  Real x1[7](start = 1.0);
  Real x1[8](start = 1.0);
  Real x1[9](start = 1.0);
  Real x1[10](start = 1.0);
equation
  when Clock() then
    x1[1] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[1];
    x1[2] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[2];
    x1[3] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[3];
    x1[4] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[4];
    x1[5] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[5];
    x1[6] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[6];
    x1[7] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[7];
    x1[8] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[8];
    x1[9] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[9];
    x1[10] = (if firstTick(u) then previous(x1) else {previous(x1[1]) + u[1], previous(x1[2]) + u[2], previous(x1[3]) + u[3], previous(x1[4]) + u[4], previous(x1[5]) + u[5], previous(x1[6]) + u[6], previous(x1[7]) + u[7], previous(x1[8]) + u[8], previous(x1[9]) + u[9], previous(x1[10]) + u[10]})[10];
  end when;
end m2;

With -nfScalarize it is only evaluated once, i.e. effort of n:

class m2
  input Real[10] u;
  Real[10] x1;
equation
  when Clock() then
    x1 = if firstTick(u) then previous(x1) else previous(x1) + u;
  end when;
end m2;

The start values for x1 are lost though, which is crucial.

[Adrian Pop]

I fixed the start value for the variable. We only handled scalar types but I now added arrays. The each qualifier will be lost as we have no way to represent it yet in the DAE.Var.

// adrpo33@ida-0030 MINGW64 /c/home/adrpo33/dev/OMTesting/nf
// $ time ~/dev/OpenModelica/build/bin/omc -d=newInst,-nfScalarize m2.mo
class m2
  input Real[10] u;
  Real[10] x1(start = 1.0);
equation
  when Clock() then
    x1 = if firstTick(u) then previous(x1) else previous(x1) + u;
  end when;
end m2;

[Adrian Pop]

I guess we should move this discussion to #5110 as it belongs there.

[Rüdiger Franke]

Looks good! The each qualifier is not that important anyway. If a scalar start value passes the frontend, then the backend may implicitly assume each.

[Per Östlund]

I noticed that some of the Clock constructors were a bit borked when checking another ticket, e.g. Clock(1, 2) didn't work. I fixed the definition of Clock() in NFBuiltinFuncs, but then noticed that they weren't even used since NFBuiltinCall.typeClockCall did all the argument matching manually. So I rewrote typeClockCall to use the normal call typing in 7b036fe. The specification is a bit unclear on which arguments are allowed to use named parameters and such things, so I used WWDD (What Would Dymola Do) and allowed all Clock parameters to be both positional and named.

One thing I changed was to remove the check for the Boolean clock that startInterval > 0.0, partly because the specification doesn't mention that restriction and partly because startInterval has a default binding of 0.0 which would violate the check.

[Francesco Casella]

Most of the Modelica_Synchronous models now work with the NF, see ​report.

I would close this ticket, and possibly open other more specific ones on remaining specific issues.